feat(propel): add story generator, fix Storybook build, complete stories - #9518
Open
MartaCondeDesign wants to merge 7 commits into
Open
feat(propel): add story generator, fix Storybook build, complete stories#9518MartaCondeDesign wants to merge 7 commits into
MartaCondeDesign wants to merge 7 commits into
Conversation
|
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded Propel Storybook generation, auditing, and coverage scripts. Added AI-focused components and stories. Updated existing component stories and localized selected chart and icon constants. ChangesPropel Storybook and component maintenance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant StoryGenerator
participant PropelComponent
participant Storybook
Developer->>StoryGenerator: Run generate-story for a component
StoryGenerator->>PropelComponent: Parse source and helper definitions
PropelComponent-->>StoryGenerator: Return props, variants, defaults, and component name
StoryGenerator->>Storybook: Write generated story file
Storybook-->>Developer: Render controls and stories
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/propel/scripts/audit.mjs (2)
134-139: 🎯 Functional Correctness | 🔵 TrivialSubstring matching can misclassify migration candidates.
uiWithoutPropeltreats apackages/uicomponent as having a propel equivalent whenever either name is a substring of the other. This can produce false negatives, for example a ui component likeinputwould matchai-inputeven though they are different components, hiding a real migration candidate from the report.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/propel/scripts/audit.mjs` around lines 134 - 139, Replace the bidirectional substring matching in the uiWithoutPropel filter with an exact component-name comparison, using the normalized lowercase names already established by propelNames. Preserve candidates only when no exact Propel name matches the UI component name.
56-65: 🚀 Performance & Scalability | 🔵 TrivialHardcoded-value regex doesn't filter what it intends to.
hardcodedValuesfirst strips\d+px;fromhelperContent, then immediately tests the same (mostly unchanged) content again for/\d+px/. Since most pixel values inside Tailwind class strings or CVA config are not followed by a literal;, this replace step removes almost nothing, so the extra step doesn't reduce false positives as intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/propel/scripts/audit.mjs` around lines 56 - 65, The hardcoded-value detection in the hasHelper branch still matches pixel values after its ineffective filtering step. Update the hardcodedValues expression to apply the intended exclusion to the content actually tested, while preserving hex detection and avoiding false positives from valid Tailwind/CVA pixel classes.packages/propel/scripts/generate-story.mjs (3)
193-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate component-file lookup logic.
findExportedComponentandcomponentUsesChildrenboth callreaddirSync(dir)and run the identical.find(...)predicate to locate the component file. Extract a sharedfindComponentFile(dir)helper and reuse its result to avoid the duplicated file-system scan and predicate.♻️ Proposed refactor
+function findComponentFile(dir) { + const files = readdirSync(dir); + return files.find( + (f) => f.endsWith(".tsx") && !f.includes(".stories.") && !f.includes("helper") && !f.includes("index") + ); +} + function findExportedComponent(dir, fallback) { - const files = readdirSync(dir); - const componentFile = files.find( - (f) => f.endsWith(".tsx") && !f.includes(".stories.") && !f.includes("helper") && !f.includes("index") - ); + const componentFile = findComponentFile(dir); if (!componentFile) return fallback; ... } function componentUsesChildren(dir) { - const files = readdirSync(dir); - const componentFile = files.find( - (f) => f.endsWith(".tsx") && !f.includes(".stories.") && !f.includes("helper") && !f.includes("index") - ); + const componentFile = findComponentFile(dir); if (!componentFile) return false; ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/propel/scripts/generate-story.mjs` around lines 193 - 224, Extract the repeated component-file discovery logic from findExportedComponent and componentUsesChildren into a shared findComponentFile(dir) helper. Have both functions reuse that helper’s result, preserving the existing fallback behavior and children detection semantics.
339-357: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
replaceAllfor multi-hyphen variant labels.
opt.replace("-", " ")only replaces the first hyphen. Current variant names (e.g.error-fill) have a single hyphen so the bug is latent, but a future variant name with two or more hyphens produces a partially-transformed label such as"Error fill-outline".generatePerVariantStories(line 332) already handles this correctly by splitting on every hyphen.function generateAllVariantsRender(variantKey, options, componentName, hasChildren) { const items = options.map((opt) => { - const label = capitalizeFirst(opt.replace("-", " ")); + const label = opt.split("-").map(capitalizeFirst).join(" ");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/propel/scripts/generate-story.mjs` around lines 339 - 357, Update the label construction in generateAllVariantsRender to replace every hyphen in opt, matching the all-hyphen handling already used by generatePerVariantStories, so multi-hyphen variant names produce fully transformed labels.
65-70: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake
stripStringLiteralspreserve string length before reusing match indices.
stripStringLiteralsreplaces non-empty string values with"",'', or``, sostrippedis shorter than `variantsBlock`. This breaks the `stripped has same length` assumption when an earlier variant group contains non-empty class values and a later group value contains a literal `{`; the brace search can extract the wrong grouped options. Use a length-preserving placeholder or an offset table when reusing matched indices.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/propel/scripts/generate-story.mjs` around lines 65 - 70, Update stripStringLiterals so every matched string literal is replaced with a same-length placeholder, preserving stripped’s character offsets relative to variantsBlock. Keep quote delimiters and escaped-content handling intact so later brace searches using matched indices identify the correct variant groups.packages/propel/scripts/check-story-coverage.sh (1)
27-42: 🚀 Performance & Scalability | 🔵 Trivial"New directory" detection can flag pre-existing components.
new_dirsis derived from newly added files underpackages/propel/src/. If this PR only adds a file to an already-existing component directory (for example, a new story file for an old component), that directory is treated as "new" and can fail the--new-onlyCI check even though the component itself isn't new. Consider deriving new directories by comparing the set of top-level directories that exist atbase_shaversusHEAD, rather than from added file paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/propel/scripts/check-story-coverage.sh` around lines 27 - 42, The new_dirs logic under NEW_ONLY incorrectly treats any newly added file as a new component directory. Replace the added-file path derivation with a comparison of top-level directories under packages/propel/src/ at base_sha and HEAD, populating new_dirs only with directories absent from the base tree; retain the existing sorting and deduplication behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/propel/scripts/check-story-coverage.sh`:
- Around line 71-79: Update the coverage calculation near total, covered, and
the “Coverage” output so an empty all_dirs array (total equal to zero) produces
a valid summary without arithmetic division by zero; retain the existing
percentage calculation when total is greater than zero.
In `@packages/propel/src/icons/state/helper.tsx`:
- Around line 7-13: Use the single EIconSize enum exported by
packages/propel/src/icons/state/helper.tsx for the entire icon API. In
packages/propel/src/icons/state/intake-state-group-icon.tsx lines 9-15 and
packages/propel/src/icons/state/state-group-icon.tsx lines 9-15, import
EIconSize from ./helper and remove each local duplicate declaration so their
defaults and size lookups use the shared type; no direct change is needed in
helper.tsx.
---
Nitpick comments:
In `@packages/propel/scripts/audit.mjs`:
- Around line 134-139: Replace the bidirectional substring matching in the
uiWithoutPropel filter with an exact component-name comparison, using the
normalized lowercase names already established by propelNames. Preserve
candidates only when no exact Propel name matches the UI component name.
- Around line 56-65: The hardcoded-value detection in the hasHelper branch still
matches pixel values after its ineffective filtering step. Update the
hardcodedValues expression to apply the intended exclusion to the content
actually tested, while preserving hex detection and avoiding false positives
from valid Tailwind/CVA pixel classes.
In `@packages/propel/scripts/check-story-coverage.sh`:
- Around line 27-42: The new_dirs logic under NEW_ONLY incorrectly treats any
newly added file as a new component directory. Replace the added-file path
derivation with a comparison of top-level directories under packages/propel/src/
at base_sha and HEAD, populating new_dirs only with directories absent from the
base tree; retain the existing sorting and deduplication behavior.
In `@packages/propel/scripts/generate-story.mjs`:
- Around line 193-224: Extract the repeated component-file discovery logic from
findExportedComponent and componentUsesChildren into a shared
findComponentFile(dir) helper. Have both functions reuse that helper’s result,
preserving the existing fallback behavior and children detection semantics.
- Around line 339-357: Update the label construction in
generateAllVariantsRender to replace every hyphen in opt, matching the
all-hyphen handling already used by generatePerVariantStories, so multi-hyphen
variant names produce fully transformed labels.
- Around line 65-70: Update stripStringLiterals so every matched string literal
is replaced with a same-length placeholder, preserving stripped’s character
offsets relative to variantsBlock. Keep quote delimiters and escaped-content
handling intact so later brace searches using matched indices identify the
correct variant groups.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 172623da-295f-48b7-8440-c069b2427159
📒 Files selected for processing (19)
.gitignorepackages/propel/package.jsonpackages/propel/scripts/audit.mjspackages/propel/scripts/check-story-coverage.shpackages/propel/scripts/generate-story.mjspackages/propel/src/ai-command-palette/ai-command-palette.stories.tsxpackages/propel/src/ai-input/ai-input.stories.tsxpackages/propel/src/badge/badge.stories.tsxpackages/propel/src/button/button.stories.tsxpackages/propel/src/charts/area-chart/root.tsxpackages/propel/src/charts/bar-chart/root.tsxpackages/propel/src/charts/line-chart/root.tsxpackages/propel/src/charts/scatter-chart/root.tsxpackages/propel/src/icon-button/icon-button.stories.tsxpackages/propel/src/icons/state/helper.tsxpackages/propel/src/icons/state/intake-state-group-icon.tsxpackages/propel/src/icons/state/state-group-icon.tsxpackages/propel/src/input/input.stories.tsxpackages/propel/src/thinking-indicator/thinking-indicator.stories.tsx
Story generator: - add scripts/generate-story.mjs: auto-generates .stories.tsx from CVA variants and TypeScript props; creates per-variant stories, AllVariants, AllSizes, and argTypes controls; resolves type aliases for select options - add scripts/audit.mjs: reports story coverage across all propel components - add scripts/check-story-coverage.sh: CI helper, fails if coverage drops - add generate-story npm script to packages/propel/package.json - add .gitignore exception for packages/propel/scripts/ Storybook build fixes: - inline EIconSize enum in icons/state (helper, state-group-icon, intake-state-group-icon) to remove @plane/constants dependency that Vite cannot resolve via workspace symlink at Storybook dev time - inline AXIS_LABEL_CLASSNAME in all four chart root files for same reason - rename tick callback props to tickProps in chart files (no-shadow lint) - hardcode PlusIcon in IconButton showcase stories so the React.FC reference is not lost when Storybook serializes story state to the URL Updated stories (badge, button, icon-button, input, ai-input, ai-command-palette, thinking-indicator): full CVA controls, per-variant named stories, AllVariants/AllSizes render stories. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MartaCondeDesign
force-pushed
the
feat/propel-story-generator
branch
from
July 31, 2026 16:31
09c2aaf to
c31a65a
Compare
- Export EIconSize from helper.tsx; import it in state-group-icon.tsx and
intake-state-group-icon.tsx instead of each declaring a duplicate enum
- Guard check-story-coverage.sh against division by zero when total=0
- Fix new_dirs detection to only flag dirs with no files at base_sha
- Add audit-stories npm script to package.json
- Fix stripStringLiterals to preserve string length (index correctness)
- Fix opt.replace("-", " ") → split/map/join to handle multi-hyphen names
- Extract findComponentFile helper to remove duplicated logic
- Fix audit.mjs substring matching → exact match for migration candidates
- Fix hardcoded-value regex by removing ineffective .replace() filter step
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add source files for AI components referenced by existing stories: ThinkingIndicator, AIInput, AITextarea, AICommandPalette, StreamingText. Set up @storybook/addon-vitest with vitest + playwright so every story runs as a render test. Add play() interaction tests to AI component stories: ThinkingIndicator (status/aria-label), AIInput (typing, error state, suggestion), AICommandPalette (search input, AI mode, loading). Add test/test:ui npm scripts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/propel/src/streaming-text/streaming-text.tsx (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the Tailwind v4 overflow-wrap utility.
Replace
break-wordswithwrap-break-word. This component relies on the utility to contain long streamed tokens.Proposed fix
- <span ref={ref} className={cn("break-words whitespace-pre-wrap", className)}> + <span ref={ref} className={cn("wrap-break-word whitespace-pre-wrap", className)}>Based on learnings, Tailwind CSS v4 uses
wrap-break-wordfor overflow-wrap utilities.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/propel/src/streaming-text/streaming-text.tsx` at line 22, Update the class list on the span rendered by the StreamingText component to replace the Tailwind v3 `break-words` utility with Tailwind v4’s `wrap-break-word`, preserving the existing whitespace, custom class, and ref behavior.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/propel/package.json`:
- Line 66: Update the package manifest’s test:ui dependency setup so `@vitest/ui`
is explicitly declared using the workspace catalog: protocol when no existing
workspace manifest already provides it, ensuring vitest --ui runs without
first-run installation.
In `@packages/propel/src/ai-command-palette/ai-command-palette.tsx`:
- Around line 49-79: Update AICommandPalette to honor the declared open prop by
rendering the palette only when open is true, while preserving the existing
default behavior when open is unspecified. Extend the component’s keyboard
handling to detect Escape and call onOpenChange(false), including the input path
associated with the “Esc close” hint.
In `@packages/propel/src/ai-textarea/ai-textarea.tsx`:
- Around line 41-45: Update the command option interaction in the rendered
command option element so onMouseDown only prevents focus loss, while onClick
invokes onSelect(cmd). Preserve pointer behavior and ensure keyboard-generated
clicks select the focused command.
- Around line 106-118: Update handleSelectCommand to validate that the current
line still begins with the slash-command prefix before removing it, returning
without changes when the caret has moved elsewhere. After computing the
replacement value, update the textarea DOM value so uncontrolled defaultValue
usage removes the command visibly, while preserving the existing onChange,
onCommand, and menu-closing behavior for valid selections. Add tests covering
caret movement and uncontrolled defaultValue usage.
---
Nitpick comments:
In `@packages/propel/src/streaming-text/streaming-text.tsx`:
- Line 22: Update the class list on the span rendered by the StreamingText
component to replace the Tailwind v3 `break-words` utility with Tailwind v4’s
`wrap-break-word`, preserving the existing whitespace, custom class, and ref
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0614a660-36ce-49f9-a56d-d4b47361376a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
packages/propel/.storybook/main.tspackages/propel/package.jsonpackages/propel/src/ai-command-palette/ai-command-palette.stories.tsxpackages/propel/src/ai-command-palette/ai-command-palette.tsxpackages/propel/src/ai-command-palette/helper.tsxpackages/propel/src/ai-command-palette/index.tspackages/propel/src/ai-input/ai-input.stories.tsxpackages/propel/src/ai-input/ai-input.tsxpackages/propel/src/ai-input/helper.tsxpackages/propel/src/ai-input/index.tspackages/propel/src/ai-textarea/ai-textarea.tsxpackages/propel/src/ai-textarea/helper.tsxpackages/propel/src/ai-textarea/index.tspackages/propel/src/streaming-text/index.tspackages/propel/src/streaming-text/streaming-text.tsxpackages/propel/src/thinking-indicator/helper.tsxpackages/propel/src/thinking-indicator/index.tspackages/propel/src/thinking-indicator/thinking-indicator.stories.tsxpackages/propel/src/thinking-indicator/thinking-indicator.tsxpackages/propel/vitest.config.tspackages/propel/vitest.shims.d.ts
- AICommandPalette: honor open prop (renders null when false); call onOpenChange(false) on Escape in both search and AI input paths - AITextarea: split onMouseDown/onClick on command buttons so keyboard activation works; validate current line starts with "/" before removing slash-command text; update textarea DOM for uncontrolled usage - StreamingText: replace Tailwind v3 break-words with v4 wrap-break-word - package.json: declare @vitest/ui 4.1.8 so test:ui does not rely on first-run install fallback Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Runs on every PR targeting preview that touches packages/propel/src/. Two steps: - Print full coverage summary (informational, never fails) - Fail if any NEW component directory added in the PR is missing a .stories.tsx file (--new-only mode, uses GITHUB_BASE_SHA) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The job now computes coverage at base and HEAD, posts a table with the before/after percentages as a PR comment (updates the existing comment on re-runs), and includes a collapsible full report. The --new-only step still fails the check if a new component dir has no .stories.tsx. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… to Button Expose both icon props via Storybook select controls with icon mapping so they work interactively on every variant story. Add WithPrependIcon, WithAppendIcon and WithBothIcons gallery stories covering all 8 variants. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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.
Closes #9519
Summary
scripts/generate-story.mjs): auto-generates complete.stories.tsxfiles by reading CVA variants and TypeScript props fromhelper.tsx. Produces per-variant named stories,AllVariants,AllSizes, andargTypeswithselect/boolean/textcontrols. Resolvestype TXxx = "a" | "b"aliases for proper select options. Safe by default — won't overwrite existing stories without--force.scripts/audit.mjs): reports story coverage for all propel components (✅ complete /scripts/check-story-coverage.sh): exits non-zero if coverage drops, suitable as a pre-merge gate.@plane/constantsworkspace symlinks are not resolved by Vite at Storybook dev time. InlinedEIconSize(icons/state) andAXIS_LABEL_CLASSNAME(4 chart roots) directly. Also renamedtickcallbackprops→tickPropsto fixno-shadowlint warnings.badge,button,icon-button,input,ai-input,ai-command-palette,thinking-indicator— all with full CVA controls and AllVariants/AllSizes render stories.Test plan
cd packages/propel && pnpm storybook— Storybook starts with no build errorspnpm generate-story Button --forcere-generates the button story without errorspnpm generate-story NewComponentfor a non-existent component prints a helpful error🤖 Generated with Claude Code
Summary by CodeRabbit