Skip to content

Fix: component design QA pass across Propel design system #220

Open
codingwolf-at wants to merge 29 commits into
mainfrom
fix/component-design-fixes
Open

Fix: component design QA pass across Propel design system #220
codingwolf-at wants to merge 29 commits into
mainfrom
fix/component-design-fixes

Conversation

@codingwolf-at

@codingwolf-at codingwolf-at commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

Batch of visual and behavioral fixes across several Propel components, found during a design QA review. Covers color/token corrections, sizing and spacing consistency, animation/transition polish, and a few small behavior bugs — mostly scoped to individual components with no breaking API changes.

What changed

  • Visual consistency — aligned colors, border radii, row heights, and spacing to match their reference components (e.g. Context Menu ↔ Menu, Pagination arrows ↔ page numbers, Breadcrumb tokens).
  • Behavior fixes — corrected a few state-handling bugs (e.g. stale selection state, incorrect border on valid/checked fields, description text not hiding under errors).
  • Motion polish — tightened and unified transition durations across toggle controls (Checkbox/Radio/Switch) and smoothed a couple of abrupt UI transitions (Button loading state, Table scroll shadow).
  • Component documentation — clarified a few story examples and doc comments where the demonstrated use case or intended behavior wasn't obvious.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Improvement (change that would cause existing functionality to not work as expected)
  • Code refactoring
  • Performance improvements
  • Documentation update

Screenshots and Media (if applicable)

NA

Test Scenarios

  • Visually spot-check the affected components in Storybook against their Figma reference
  • Confirm no regressions in existing interaction tests
  • Run vp check / vp test on the full package

References

NA

…olGroup for consistent layout

- Added FieldItemControlGroup to CheckboxField, CheckboxGroupFieldOption, RadioGroupFieldOption, and SwitchField components to enhance layout consistency.
- Updated field item content variants to align control heights with label text for improved visual alignment.
…avior to handle error state

- Modified the description prop in AutocompleteField, ComboboxField, and SelectField components to clarify that it is replaced by the error prop when an error is set.
- Adjusted rendering logic to ensure description is only displayed when there is no error present.
…iant styles

- Updated the behavior of editable cells to ensure the selected tint is maintained only while the menu is open.
- Refined the table variant styles to incorporate directional shadows based on scroll state, improving visual feedback for pinned headers and cells.
- Adjusted the `onOpenChange` logic for better clarity and conciseness in handling selected email state.
…rows

- Updated the corner radius for pagination items and arrow buttons to `radius/sm` for consistency in design.
- Enhanced comments to clarify the design intent regarding the shared corner radius for selected numbers and arrows.
…clarity

- Added assertions to verify that the disabled tab's label color differs from the resting state, ensuring proper visual feedback.
- Updated the tabUnderlineLabelVariants to reflect the disabled state correctly, ensuring the disabled tone is applied based on the parent tab's state.
- Improved comments to clarify the relationship between the tab's disabled state and its visual representation.
…ailing control

- Adjusted the right padding of the autocomplete input group to ensure the trailing button appears flush with the input field, enhancing visual balance.
- Updated comments to clarify the impact of the `:has()` selector on padding behavior when a button is present.
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

📚 Storybook preview: https://pr-220-propel-storybook.vamsi-906.workers.dev

…or clarity

- Changed the border-radius of the pill component from fully rounded to medium (md) to align with design specifications.
- Updated comments to reflect the new border-radius and clarify the design intent.
…nsistency

- Updated the width for the xl variant of the otp-field input from 11 to 10 to align with design standards.
- Modified the control group comments to reflect the new height for the xl size, changing it from 44px to 40px for improved clarity and consistency across components.
…rove comments

- Updated button label variants to include transition effects for smoother loading states.
- Modified control chrome variants to apply a unified transition effect for better visual consistency.
- Enhanced spinner component with new animations for a smoother reveal and spin effect, improving user experience.
- Added detailed comments to clarify the purpose and behavior of the new animations.
…size specifications

- Changed the secondary link variant from `text-secondary hover:text-primary` to `text-tertiary hover:text-primary` for clearer hover visibility.
- Adjusted text size specifications for the lg and xl variants to ensure distinct font sizes at each magnitude, enhancing visual hierarchy.
…nctionality

- Added a new DrawerHeader component that includes a built-in corner close button for consistent user experience across drawers.
- Updated drawer stories to reflect the new DrawerHeader implementation, removing manual close button instances.
- Adjusted comments and exports in related files to accommodate the new component structure.
- Refactored the PanelRow component to standardize the layout for various controls, ensuring consistent height across Radio and Checkbox elements.
- Updated the ToggleFooter to utilize the new PanelRow for better visual alignment and user interaction.
- Improved comments to clarify the purpose of alignment adjustments in the popover stories, ensuring panels extend inward to avoid overflow.
…tency

- Changed icon and disclosure indicator tint from `tertiary` to `secondary` for better visual alignment.
- Updated breadcrumb link and page variants to ensure consistent height across elements, fixing the height to `h-6` (24px) for uniformity.
- Enhanced comments to clarify design specifications and behavior of breadcrumb components.
…stent label sizing

- Updated the SelectField component to use FieldLabel instead of SelectLabel, ensuring the label size matches other form fields.
- Enhanced comments to explain the rationale behind the change, focusing on visual consistency across form elements.
…ance comments

- Updated transition durations for Checkbox, Radio, and Switch components to a consistent 100ms for improved responsiveness.
- Enhanced comments to clarify the rationale behind the changes, ensuring a cohesive user experience across toggle controls.
- Added CheckboxField to the Checkbox stories for better integration and documentation of form behavior.
…height consistency

- Replaced `Dot` with `Circle` for selection indicators in the context menu stories to enhance visual clarity.
- Adjusted item height from `h-8` to `h-[34px]` for better alignment with dropdown menu rows, ensuring a consistent user experience.
- Updated comments to clarify the rationale behind the height adjustments and styling changes.
@codingwolf-at codingwolf-at changed the title Fix/component design fixes Fix: component design QA pass across Propel design system Jul 7, 2026
…r test assertions

The checkbox danger-border selector gained a not-data-checked/not-data-indeterminate
guard in an earlier commit, changing its compiled class string. Two sibling
interaction tests still pinned the old literal class via toHaveClass, breaking CI.
Replaced with computed-style checks against the live --border-danger-strong token.
…omments

- Adjusted the `MenuItemIndicator` to render at the inline-start position, ahead of any leading icon, for better alignment with design specifications.
- Enhanced comments in the `MenuItem` and `MenuItemIndicator` components to clarify the rendering logic and selected state handling.
- Updated related stories to reflect the new positioning of the check indicator, ensuring consistency across menu items.
… improved documentation

- Introduced new components: `PreviewCardIcon`, `PreviewCardMeta`, `PreviewCardPropertyGroup`, and `PreviewCardTitleRow` to enrich the preview card's anatomy.
- Updated `PreviewCardContent` to remove the backdrop, clarifying its non-modal nature and enhancing the hover-triggered experience.
- Enhanced storybook examples to demonstrate the new components and their usage in various scenarios, including rich anatomy interactions.
- Improved comments across files to provide clearer explanations of component behavior and design rationale.
…ontextual icons

- Introduced `maxVisible` prop to limit the number of visible chips, collapsing excess into a "+N more" count.
- Added `startIcon` and `endIcon` props for contextual icons on chips, improving visual representation.
- Created `ComboboxChipOverflow` component to display the overflow count when chips exceed the visible limit.
- Updated `ComboboxChips` to handle new layout options and improved rendering logic for better user experience.
- Enhanced storybook examples to demonstrate new features and interactions, including overflow behavior.
@codingwolf-at codingwolf-at changed the title Fix: component design QA pass across Propel design system Fix: component design QA pass across Propel design system [WIP - No not merge] Jul 7, 2026
…ture

- Updated `MenuItem` to render a leading single-select check indicator conditionally, enhancing alignment with design specifications.
- Refined comments in `MenuItem` for clarity on rendering logic and selected state handling.
- Modified `SelectField` to use `FieldLabel` for better consistency in label sizing across field components, improving overall UI coherence.
<Radio value={value} />
<span className="min-w-0 flex-1 truncate">{label}</span>
{control}
<span className="min-w-0 flex-1 truncate">{children}</span>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codingwolf-at this is intentionally named label because its designed for only a text label

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, reverted it back to label


/* spinner reveal: a loader slot growing from zero to its glyph width while fading in, so the
control it sits in (button, pill) widens smoothly into its loading state instead of snapping */
--animate-spinner-reveal: spinnerReveal 0.2s ease-out forwards;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't express intent reveal how?

export const contextMenuItemVariants = cva(
cx(
"flex h-8 cursor-default items-center gap-2 rounded-md px-2 text-13 outline-none select-none [--node-size:1rem]",
"flex h-[34px] cursor-default items-center gap-2 rounded-md px-2 text-13 outline-none select-none [--node-size:1rem]",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can do h-8.5

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}: PreviewCardContentProps) {
return (
<BasePreviewCard.Portal>
<BasePreviewCard.Backdrop render={<Backdrop />} />

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure this is required, e.g. if I click anywhere on the screen it should close the preview (not sure if it still does this)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still closes fine, the preview card is hover based and outside-click/Esc dismissal is handled by the Root's useDismiss, not the backdrop. Base UI's PreviewCard.Backdrop is just a decorative <div> with pointer-events: none and no dismiss handler, so removing it changes no behavior.

Comment on lines +41 to +47
startIcon?: (item: Value) => React.ReactNode;
/**
* A contextual element rendered after each chip's label, before the remove control — e.g. an
* `<Icon icon={…} />` status glyph, sized to the chip's `--node-size`. Decorative — the chip's
* label carries its name.
*/
endIcon?: (item: Value) => React.ReactNode;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the value mapping already existed, but the fact its leaking into doing something like this, makes me think we should rethink this API. e.g. leave filtering logic to consumers, so they can still build their own item templates

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — startIcon/endIcon are the start of an unbounded list of (value) => node props, with the chip template owned by the component. The low-level parts (ComboboxChip, ComboboxChipRemove, ComboboxChipOverflow) are already public, so a consumer can hand-build a chip template today. Which direction do you want for the ready-made?

  1. Keep it minimal — drop startIcon/endIcon, point custom chips at the low-level parts.
  2. Template APIchildren={(value) => <ComboboxChip>…</ComboboxChip>} so consumers own the whole chip; drop the per-slot mapping props.
  3. Leave as-is for now (WIP) and do the redesign in a follow-up.

…hance documentation

- Updated the `ParentCheckbox` story to `SelectAll`, reflecting its role as a select-all control.
- Improved comments for clarity on the select-all functionality and its interaction with child checkboxes.
- Adjusted interaction tests to align with the new naming and functionality, ensuring accurate behavior representation.
- Refined nested checkbox group documentation to clarify independent category management.
managementValue.length > 0 && managementValue.length !== MANAGEMENT_PERMISSIONS.length
}
label="Manage content"
aria-controls={MANAGE_CONTENT_PERMISSIONS.join(" ")}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is incredibly confusing values and ids should not be used interchangibly.

* affects "Manage content". Both groups are uncontrolled (`defaultValue`), so Base UI owns every
* parent/child state with no manual syncing. "User permissions" is a plain heading, not a control.
*/
export const NestedParentCheckbox: Story = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want a convention for field ids:

form-name-field-value

@codingwolf-at codingwolf-at changed the title Fix: component design QA pass across Propel design system [WIP - No not merge] Fix: component design QA pass across Propel design system Jul 7, 2026
…rity

- Modified the `PanelRow` component to accept a `label` prop instead of `children`, enhancing the clarity of its intended use.
- Updated related components to align with the new `PanelRow` structure, ensuring consistent usage across the popover component.
…onents

- Updated height from `h-[34px]` to `h-8.5` in `Popover`, `ContextMenu`, and `Menu` components for consistency.
- Ensured that all related styles align with the new height standard, improving visual coherence across the UI.
…r consistency

- Renamed spinner animation from `--animate-spinner-reveal` to `--animate-spinner-expand` for clarity.
- Updated related CSS keyframes to follow a consistent naming pattern, changing `slideInFromBottom` to `slide-in-from-bottom`, `fadeIn` to `fade-in`, and others accordingly.
- Improved comments in the spinner and animations files to reflect the changes and enhance understanding of the animation behaviors.
…umentation

- Removed explicit `id` attributes from child checkboxes, allowing the base UI to derive input IDs automatically.
- Updated the `SelectAll` and `NestedParentCheckbox` stories to reflect the new checkbox structure, improving clarity and consistency.
- Enhanced comments to better explain the relationship between parent and child checkboxes, ensuring accurate understanding of their functionality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants