Component cleanup: rendering fixes, RadioGroup density, useControllableState adoption#111
Merged
Merged
Conversation
Both rendered blank: the switch thumb used shadow-sm and the dropdown story icons used text-orange/amber/blue-500, all of which propel resets out of the default Tailwind scales. Use shadow-raised-100 and the propel label-icon tokens.
Adds density (comfortable | compact) so consumers set row spacing on the group instead of reaching into it with [&>[role=radiogroup]]:gap-0 from the outside. The popover panels now pass density="compact".
Replaces the hand-rolled isControlled / internal-useState / commit pattern with the shared hook, matching Dropdown.
A standalone Avatar with no magnitude (and not inside an AvatarGroup) fell through to an undefined magnitude, so it rendered with no size class and the person-icon fallback had no size. Default to "md" when neither an explicit magnitude nor a group magnitude is present, and document the precedence.
ScrollArea only demoed vertical and both-axis overflow; add a horizontal-only story so the single horizontal scrollbar is covered on its own. Pull the repeated scaffolding in three story files into a local helper each: a TabsFixture for the shared three-tab set, a ToggleFooter for the display panels' checkbox footer, and an inertAnchor for the breadcrumb menu items. The breadcrumb anchors now all swallow their default navigation, so none of them can tear down the browser test page if a play test later clicks one.
|
📚 Storybook preview: https://pr-111-propel-storybook.vamsi-906.workers.dev |
There was a problem hiding this comment.
Pull request overview
This PR applies a set of small, self-contained component and Storybook cleanups in @plane/propel, focused on fixing token usage/rendering edge cases, improving an API pain point (RadioGroup spacing), and standardizing controlled/uncontrolled state handling via the shared useControllableState hook.
Changes:
- Fixes rendering/token mismatches (Switch thumb shadow token, Dropdown priority icon label tokens, Avatar default magnitude fallback).
- Adds
RadioGroupdensityprop (comfortable|compact) and updates Popover demos to use it instead of external gap overrides. - Refactors several components/stories to reduce duplication and adopt
useControllableStatefor controlled/uncontrolled value bookkeeping.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/propel/src/components/tabs/tabs.stories.tsx | Dedupes repeated story scaffolding via a shared TabsFixture/tab item list. |
| packages/propel/src/components/switch/index.tsx | Replaces ineffective thumb shadow utility with the correct shadow token. |
| packages/propel/src/components/search/index.tsx | Migrates Search and ExpandableSearch to useControllableState. |
| packages/propel/src/components/scroll-area/scroll-area.stories.tsx | Adds a horizontal-only ScrollArea story variant. |
| packages/propel/src/components/radio/radio.stories.tsx | Adds a story demonstrating the new RadioGroup density behavior. |
| packages/propel/src/components/radio/index.tsx | Introduces density prop and internal variants for group spacing. |
| packages/propel/src/components/popover/popover.stories.tsx | Dedupes checkbox footer and switches to RadioGroup density="compact" for flush rows. |
| packages/propel/src/components/nav-item/index.tsx | Replaces custom expanded-state logic with useControllableState. |
| packages/propel/src/components/dropdown/dropdown.stories.tsx | Uses label token utilities for priority icon coloring instead of raw Tailwind colors. |
| packages/propel/src/components/breadcrumb/breadcrumb.stories.tsx | Centralizes inert anchor rendering to prevent Storybook navigation teardown. |
| packages/propel/src/components/avatar/index.tsx | Ensures standalone avatars fall back to md magnitude and simplifies fallback icon sizing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A round of contained component cleanups from a codebase audit plus an independent review. Scoped to self-contained, high-value fixes; the larger node-prop/typography sweeps are deferred.
Fixes
shadow-sm(reset toinitialin propel, so it rendered no shadow); swapped to the realshadow-raised-100token. The dropdown priority-icon colors used rawtext-orange-500/amber-500/blue-500(also reset toinitial) and now use thetext-label-*-icontokens.Avatarwith nomagnitudeand noAvatarGroupfell through to an undefined magnitude, so it rendered with no size class. It now resolves tomd(explicit → group →md), and the precedence is documented.Improvements
densityprop (comfortable|compact) so consumers stop reaching for a cryptic[&>[role=radiogroup]]:gap-0override. The popover display panels now passdensity="compact".Search,ExpandableSearch, andNavItemHeaderreplaced their hand-rolled controlled/uncontrolled bookkeeping with the shareduseControllableStatehook.Stories
ScrollAreastory (only vertical and both-axis were covered).TabsFixture(shared three-tab set), aToggleFooter(display-panel checkbox footer), and aninertAnchorfor breadcrumb menu items. The breadcrumb anchors now all swallow their default navigation, so none can tear down the browser test page if a play test later clicks one.Notes from the independent review (not changed here, with rationale)
vitest@0.1.24+@vitest/browser@4.1.9). This comes from the Vite+ fork pinning vitest through its own catalog while@vitest/browser-playwrightpulls a real vitest peer. It's a toolchain/architecture concern, not safe to pin inside a component PR.act(...)warnings — pre-existing console warnings during the dropdown play tests; they're warnings, not failures, and unrelated to this cleanup.vp checkclean; full story gate green (820 tests).