Skip to content

Refine component consistency and story tests#115

Open
lifeiscontent wants to merge 14 commits into
mainfrom
component-consistency-pass
Open

Refine component consistency and story tests#115
lifeiscontent wants to merge 14 commits into
mainfrom
component-consistency-pass

Conversation

@lifeiscontent

Copy link
Copy Markdown
Collaborator

Summary

  • Add a shared internal NodeSlot component and reuse it across repeated inline node wrappers while preserving each caller's existing ARIA semantics.
  • Normalize touched Storybook play functions to the direct canvas fixture and keep the a11y/interaction assertions intact.
  • Align touched component/story docs with the inlineStartNode/inlineEndNode naming pattern.

Validation

  • vp check --fix
  • vp test run
  • vp run -F @plane/propel test run --reporter verbose

Storybook previews

Copilot AI review requested due to automatic review settings June 17, 2026 10:02
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR standardizes “node slot” rendering across multiple Propel components by introducing a shared internal NodeSlot component, and it normalizes the touched Storybook interaction tests to use the canvas fixture while aligning docs to the inlineStartNode/inlineEndNode naming.

Changes:

  • Add NodeSlot (span wrapper + shared nodeSlotClass) and replace repeated inline wrappers across components.
  • Update Storybook play functions to use ({ canvas }) instead of within(canvasElement).
  • Rename/align touched docs and comments to the inlineStartNode/inlineEndNode terminology.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/propel/src/internal/node-slot.tsx Introduces NodeSlot component and shared nodeSlotClass for consistent node wrapping/sizing.
packages/propel/src/internal/node-slot.ts Removes the previous class-only helper module in favor of the new TSX implementation.
packages/propel/src/components/button/index.tsx Reuses NodeSlot for inline-start/end nodes in Button.
packages/propel/src/components/icon-button/index.tsx Reuses NodeSlot for decorative icon children in IconButton.
packages/propel/src/components/pill/index.tsx Replaces the local PillNode wrapper with NodeSlot and aligns docs.
packages/propel/src/components/dropdown/index.tsx Replaces inline nodeSlotClass spans with NodeSlot for item/subtrigger slots.
packages/propel/src/components/nav-item/index.tsx Replaces nodeSlotClass usage with NodeSlot and updates inline-start/end terminology.
packages/propel/src/components/search/search.stories.tsx Updates play functions to use the canvas fixture and removes within import.
packages/propel/src/components/progress/progress.stories.tsx Updates play functions to use canvas and removes within import.
packages/propel/src/components/pill/pill.stories.tsx Updates play functions to use canvas, removes within, and aligns story docs wording.
packages/propel/src/components/nav-item/nav-item.stories.tsx Updates play functions to use canvas, removes within, and aligns story docs wording.
packages/propel/src/components/nav-item/nav-item-header.stories.tsx Updates play functions to use canvas, removes within, and aligns story docs wording.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/propel/src/internal/node-slot.tsx
@lifeiscontent

Copy link
Copy Markdown
Collaborator Author

Updated this PR with a Storybook coverage pass and the Field API cleanup.\n\nValidation:\n- vp check --fix: pass\n- Storybook focused tests for new input/tooltip stories: pass\n- vp run -F @plane/propel test run --coverage --reporter verbose: 116 files passed, 860 tests passed\n\nCoverage after this pass: 97.95% statements, 84.71% branches, 97.6% functions, 98.71% lines. Input is now 100% statements/functions/lines and 91.89% branches.\n\nAPI cleanup:\n- Removed the exported Field Object.assign compound pattern.\n- Added named child exports FieldLabel, FieldControl, FieldDescription, and FieldError.\n- Updated the comment composer recipe so copyable story code uses Field/FieldControl plus native label semantics instead of Field.*.\n\nPreview links:\n- Banner Optional Content Semantics: http://localhost:6006/?path=/story/components-banner--optional-content-semantics\n- Progress Circular Zero Max: http://localhost:6006/?path=/story/components-progress--circular-zero-max\n- Search Labelled By Semantics: http://localhost:6006/?path=/story/components-search--labelled-by-semantics\n- Search Disabled Hides Clear: http://localhost:6006/?path=/story/components-search--disabled-hides-clear\n- Search Expandable Type And Clear: http://localhost:6006/?path=/story/components-search--expandable-type-and-clear\n- Dropdown Label And Footer Semantics: http://localhost:6006/?path=/story/components-dropdown--label-and-footer-semantics\n- Input Field Composition: http://localhost:6006/?path=/story/components-input--field-composition\n- Input Native Aria Label: http://localhost:6006/?path=/story/components-input--native-aria-label\n- Input Field Error Composition: http://localhost:6006/?path=/story/components-input--field-error-composition\n- Tooltip Shared Provider Timing: http://localhost:6006/?path=/story/components-tooltip--shared-provider-timing\n- Comment Composer Default: http://localhost:6006/?path=/story/patterns-comment-composer--default

Copilot AI review requested due to automatic review settings June 17, 2026 11:14
@lifeiscontent

Copy link
Copy Markdown
Collaborator Author

Added the VariantProps consistency cleanup.\n\nChanges:\n- Input tone now derives from boxVariants.\n- Pill magnitude now derives from labelPillSize.\n- Progress variant now derives from rootVariants.\n- Toolbar density now derives from toolbarVariants.\n- Tabs variant now derives from rootVariants.\n\nLeft non-CVA unions alone where they are behavioral or sourced elsewhere: TablePinned/TableHeadSort/PageToken/ScrollAreaOrientation, AvatarTone palette, AvatarGroupMagnitude, and IconButton aliases from Button.\n\nValidation:\n- vp check --fix: pass\n- Focused Storybook tests for Input/Pill/Progress/Toolbar/Tabs: pass\n- vp run -F @plane/propel test run --reporter verbose: 116 files passed, 860 tests passed\n\nPreview links:\n- Input Field Error Composition: http://localhost:6006/?path=/story/components-input--field-error-composition\n- Pill Button: http://localhost:6006/?path=/story/components-pill--button\n- Progress Circular Zero Max: http://localhost:6006/?path=/story/components-progress--circular-zero-max\n- Toolbar Default: http://localhost:6006/?path=/story/components-toolbar--default\n- Tabs Contained: http://localhost:6006/?path=/story/components-tabs--contained

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comment thread packages/propel/src/components/input/index.tsx Outdated
Comment thread packages/propel/src/components/input/input.stories.tsx Outdated
@lifeiscontent

Copy link
Copy Markdown
Collaborator Author

Base UI render/useRender audit update:

  • Reviewed current Base UI render usages. The existing usages in Input, Tabs, Toolbar, Tooltip, Table, Breadcrumb/Dropdown/Popover stories are composition-oriented and should stay as render rather than being refactored to useRender.
  • Refactored NavItem to use Base UI's documented useRender({ defaultTagName, render, props }) shape with mergeProps, while preserving the important a11y/polymorphism behavior: default rows are native button type="button", but render={<a /> } does not receive a button-only type attribute.
  • Added an AsLink Storybook assertion for that anchor behavior.

Validation:

  • vp check --fix passed.
  • Focused Storybook tests with a11y passed for components-navitem--default, components-navitem--as-link, and components-navitem--keyboard-activation across theme instances.

Preview:

Copilot AI review requested due to automatic review settings June 17, 2026 11:34
@lifeiscontent

Copy link
Copy Markdown
Collaborator Author

Base UI primitive migration update:

  • Moved Button, IconButton, PillButton, and IconPill onto Base UI Button so disabled/loading semantics are owned by the primitive instead of our local helper.
  • Removed packages/propel/src/internal/loading-button.ts.
  • Reworked NavItemHeader into a Base UI Collapsible composition: NavItemGroup owns open state, NavItemHeader is the trigger, and NavItemPanel is the controlled panel. This fixed the trigger/panel aria relationship and removed the local controllable state path.
  • Added IconButton loading interaction coverage and updated NavItemHeader stories to always pair triggers with a real panel.

Validation:

  • vp check --fix passed.
  • Focused Storybook/a11y tests passed for Button loading/keyboard behavior, IconButton loading behavior, Pill button/toggle behavior, and NavItemHeader Collapsible behavior.
  • Full Storybook/a11y suite passed.

Preview:

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Comment thread packages/propel/src/components/button/index.tsx Outdated
Comment thread packages/propel/src/components/icon-button/index.tsx Outdated
Comment thread packages/propel/src/components/pill/index.tsx Outdated
Comment thread packages/propel/src/components/pill/index.tsx Outdated
Comment thread packages/propel/src/components/input/index.tsx Outdated
Copilot AI review requested due to automatic review settings June 17, 2026 18:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 279 out of 279 changed files in this pull request and generated 1 comment.

Comment thread packages/propel/.storybook/preview.tsx
Split the package into three tiers, each published as its own subpath:

- base/   Base UI extensions (e.g. the textarea Field.Control adaptation)
- ui/     every Base UI component styled with Plane tokens, flattened so
          each part renders a single element (the composable primitives)
- components/  ready-made compositions of ui parts for the 90% case, with
          a 1:1 mapping to ui (drop to ui/<x> for low-level control)

Highlights:
- vite subpathEntries + package exports now cover base/ui/components/hooks
- renamed the masked `dropdown` -> `menu` to match Base UI naming
- every ui part is a single element; decoration/composition lives in components
- no className/style exposed; render passes through; nodes use inlineStartNode/
  inlineEndNode (dropped leadingIcon/trailingIcon)
- all cva variants live in ui (or base); components never call *Variants()
- each component has a UI/<Name> story (composes atomic parts) and a
  Components/<Name> story (uses the ready-made); stories never cross tiers

vp check (typecheck/lint/format), vp pack (attw + publint), and the
Storybook story suite all pass.
Copilot AI review requested due to automatic review settings June 18, 2026 13:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

0.2.x ships real vitest 4.1.9 as a direct dependency instead of the
old @voidzero-dev/vite-plus-test fork. Our catalog still aliased
vitest to that fork (the 0.1.x shape), and the `vitest: "catalog:"`
override then redirected 0.2.0's real vitest onto the binless 0.1.24
fork, so `vp test` failed with "Could not find vitest bin".

Point the catalog at vitest 4.1.9 directly, matching what a fresh
vite+ scaffold generates. check, build (tsdown/attw/publint) and the
Storybook browser test suite all pass.
Claude Code worktrees live at .claude/worktrees/<name> -- full second
checkouts of the repo nested inside vite's workspace root. Vitest
doesn't honor .gitignore, so a repo-root `vp test` would crawl those
duplicate trees. Drop **/.claude/** from test discovery (on top of the
built-in excludes) and from the vite server's file watcher.
Copilot AI review requested due to automatic review settings June 18, 2026 14:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

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