Feature/business hours editor updates #47
Merged
Conversation
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
- Add meta.args with defaults for disabled, showDescription, use24Hour, weekStartsOn, addHoursLabel - Remove hardcoded showDescription from wrapper - Clean up redundant args in individual stories
…nal props
- Hide 'value', 'onChange', 'className' controls from Storybook docs
- Clicking 'Set object' on value control caused 'schedule.find is not a function'
error because Storybook defaults to {} (object) instead of [] (array)
- These props are managed by the interactive wrapper component
- Hide 'use24Hour' control - prop exists in interface but is not implemented
- Component uses native <input type='time'> which uses browser locale
- Prop is destructured as _use24Hour (intentionally unused)
- Can be implemented in future if 24-hour format support is needed
- Add default args for remaining controls (disabled, showDescription, weekStartsOn, addHoursLabel)
- Ensures boolean controls start with actual values instead of undefined
- Add useEffect to sync schedule state when props.value changes from Storybook controls
There was a problem hiding this comment.
Pull request overview
Updates the BusinessHoursEditor Storybook stories to better reflect how the component is meant to be used (via an interactive wrapper) and to prevent problematic controls from causing runtime errors.
Changes:
- Hides wrapper-managed props (
value,onChange,className) and the unimplementeduse24Hourprop from Storybook docs. - Adds default
argsfor remaining controls so toggles/text start with concrete values. - Adds a
useEffectin the wrapper to sync internal schedule state whenprops.valuechanges.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/components/BusinessHoursEditor/BusinessHoursEditor.stories.tsx
Outdated
Show resolved
Hide resolved
Address Copilot review feedback: spread props was overriding controlled state. Now destructure value and onChange from props before spreading restProps, ensuring the wrapper's controlled state is always used.
Deploying ui with
|
| Latest commit: |
81cf750
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ac685672.ui-6d0.pages.dev |
| Branch Preview URL: | https://feature-business-hours-edito.ui-6d0.pages.dev |
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.
Hide 'value', 'onChange', 'className' controls from Storybook docs
error because Storybook defaults to {} (object) instead of [] (array)
Hide 'use24Hour' control - prop exists in interface but is not implemented
Add default args for remaining controls (disabled, showDescription, weekStartsOn, addHoursLabel)
Add useEffect to sync schedule state when props.value changes from Storybook controls
business-hour-editor-update.mov