-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(trip form): add callback for detecting when all submodes are disabled #28
Conversation
BREAKING CHANGE: removes mode subsettings panel Co-authored-by: Daniel Heppner <[email protected]> Co-authored-by: Amy Corson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Thanks for the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a Storybook action so we can see the new onAllSubmodesDisabled
callback being fired. See the other comments too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the mock buttons intended to be used outside Storybook? Renaming this file will result in the contents to ship in the binary packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was renamed because the build on the new version of storybook was failing. When .story is in the filename, a story is expected and there is no story in this file. These mocks are not used anywhere else besides storybook.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized there is a __mocks__
folder for this package, could you move mockButtons.tsx
into that folder?
packages/trip-form/src/MetroModeSelector/AdvancedModeSettingsButton.story.tsx
Show resolved
Hide resolved
@@ -111,7 +120,8 @@ export default { | |||
argTypes: { | |||
fillModeIcons: { control: "boolean" }, | |||
onSetModeSettingValue: { action: "set mode setting value" }, | |||
onToggleModeButton: { action: "toggle button" } | |||
onToggleModeButton: { action: "toggle button" }, | |||
onAllSubmodesDisabled: { action: "all submodes disabled" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see "all submodes disabled" appearing in the Storybook action pane, did I miss something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see "all submodes disabled" appearing in the Storybook action pane, did I miss something?
I pulled a previous commit, sorry, ignore that comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to go!
This PR adds a callback to detect when all the submodes under a mode button have been disabled. This will allow us to disable the mode button when all its submodes have been turned off.