Skip to content

feat(LanguageSelector): convert stories to args-based pattern#62

Merged
garrity-miepub merged 3 commits intomainfrom
feature/language-selector-updates
Feb 3, 2026
Merged

feat(LanguageSelector): convert stories to args-based pattern#62
garrity-miepub merged 3 commits intomainfrom
feature/language-selector-updates

Conversation

@garrity-miepub
Copy link
Copy Markdown
Contributor

  • Convert render-based stories to args-based for controls integration
  • Add default args and comprehensive argTypes with descriptions
  • Remove unused wrapper components
  • Add individual Ghost, Minimal, Small, Large stories
  • Replace hardcoded colors with design tokens for dark mode
language-selector.mov

Copilot AI review requested due to automatic review settings February 3, 2026 04:07
@garrity-miepub garrity-miepub marked this pull request as ready for review February 3, 2026 04:07
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Feb 3, 2026

Deploying ui with  Cloudflare Pages  Cloudflare Pages

Latest commit: 51b88f9
Status: ✅  Deploy successful!
Preview URL: https://679f23fd.ui-6d0.pages.dev
Branch Preview URL: https://feature-language-selector-up.ui-6d0.pages.dev

View logs

Copy link
Copy Markdown

Copilot AI left a comment

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 modernizes the LanguageSelector component stories by converting them from render-based stories with wrapper components to an args-based pattern that integrates with Storybook's controls. The changes include adding default args and comprehensive argTypes with descriptions, removing wrapper components, adding individual variant stories (Ghost, Minimal, Small, Large), and replacing hardcoded colors with design tokens for better dark mode support.

Changes:

  • Converted stories from render-based pattern with state management wrappers to args-based pattern
  • Added default args and comprehensive argTypes with descriptions for better Storybook controls integration
  • Replaced hardcoded color classes with design tokens (text-muted-foreground, border-border, bg-card, etc.)

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

- Convert render-based stories to args-based for controls integration
- Add default args and comprehensive argTypes with descriptions
- Remove unused wrapper components
- Add individual Ghost, Minimal, Small, Large stories
- Replace hardcoded colors with design tokens for dark mode
@garrity-miepub garrity-miepub force-pushed the feature/language-selector-updates branch from 36b0d4e to fe4ed88 Compare February 3, 2026 04:10
…stories

Address Copilot review feedback on PR #62:

- Add LanguageSelectorWithState wrapper component that manages internal
  state with React.useState, similar to SelectWithState pattern
- Wrapper syncs with Storybook controls via useEffect when value changes
- Convert AllVariantsComparison to named function with independent state
  for each selector type (dropdown, native, inline)
- Convert InHeader to named function with proper state management

This fixes the issue where controlled components weren't responding to
user interactions - clicking a language option now updates the display
while still allowing Storybook controls to work properly.
Copilot AI review requested due to automatic review settings February 3, 2026 04:19
@garrity-miepub
Copy link
Copy Markdown
Contributor Author

Addressed Copilot Review Comments

Thanks for the thorough review! All three feedback items have been addressed in commit ecab3b2:

1. Default Story State Management (Line 71)

Added LanguageSelectorWithState wrapper component following the same pattern as SelectWithState in Select.stories.tsx. The wrapper:

  • Manages internal state with React.useState
  • Syncs with Storybook controls via useEffect when the value prop changes
  • Forwards the onChange callback for Storybook actions logging

2. AllVariantsComparison State Management (Line 132)

Converted to a named function component AllVariantsComparisonStory that manages independent state for each selector type:

  • dropdownValue for LanguageSelector
  • nativeValue for LanguageSelectorNative
  • inlineValue for LanguageSelectorInline

Each selector now responds to user interactions while staying in sync with Storybook controls.

3. InHeader State Management (Line 151)

Converted to a named function component InHeaderStory with proper state management. The header context demo now correctly updates when users select a language.


Testing: All stories now properly respond to both:

  • User interactions (clicking a language updates the displayed selection)
  • Storybook controls (changing the value control updates the component)

Copy link
Copy Markdown

Copilot AI left a comment

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 1 out of 1 changed files in this pull request and generated 6 comments.


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

… object

Address additional Copilot review feedback on PR #62:

- Update comment to clarify LanguageSelectorWithState is the primary
  Storybook component wrapper (Line 17)
- Fix handleChange to receive Language object and extract .code (Line 51)
- Fix LanguageSelector onChange: (language) => setDropdownValue(language.code) (Line 189)
- Fix LanguageSelectorNative onChange: (language) => setNativeValue(language.code) (Line 197)
- Fix LanguageSelectorInline onChange: (language) => setInlineValue(language.code) (Line 205)
- Fix InHeader LanguageSelector onChange: (language) => setValue(language.code) (Line 231)

The onChange callback receives a Language object (with code, name, flag
properties), not just a string. All handlers now properly extract
language.code from the object to match the component's API.
@garrity-miepub
Copy link
Copy Markdown
Contributor Author

Addressed Additional Copilot Review Comments (Round 2)

All 6 new feedback items have been addressed in commit 51b88f9:

API Signature Mismatch Fixes

The core issue was that onChange receives a Language object (with code, name, flag properties), not just a string. All handlers now properly extract language.code from the object.

Line Issue Fix
17 Comment didn't clarify wrapper purpose Updated to "Story Component Wrapper for State Management (primary Storybook component)"
51 handleChange received string instead of Language Changed to (language: Language) => { setValue(language.code); onChange?.(language); }
189 LanguageSelector onChange passed setter directly Changed to (language) => setDropdownValue(language.code)
197 LanguageSelectorNative onChange passed setter directly Changed to (language) => setNativeValue(language.code)
205 LanguageSelectorInline onChange passed setter directly Changed to (language) => setInlineValue(language.code)
231 InHeader onChange passed setter directly Changed to (language) => setValue(language.code)

This ensures the stories correctly match the component's API contract where onChange provides the full Language object for flexibility (consumers can access code, name, or flag as needed).

@garrity-miepub garrity-miepub merged commit 977b436 into main Feb 3, 2026
10 checks passed
@garrity-miepub garrity-miepub deleted the feature/language-selector-updates branch February 3, 2026 04:32
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