-
Notifications
You must be signed in to change notification settings - Fork 179
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
refactor(app): add and use deck map component in interventionmodal #15570
Conversation
e28fb73
to
12b4a70
Compare
}, | ||
] | ||
|
||
const CONSOLE_LOG_ON_SELECT = (location: ModuleLocation): void => { |
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.
nice!
@@ -44,7 +44,7 @@ export function useRunPausedSplash( | |||
showERWizard: boolean | |||
): boolean { | |||
// Don't show the splash when desktop ER wizard is active. | |||
return !(!isOnDevice && showERWizard) | |||
return isOnDevice && !showERWizard |
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.
Thank you!
modulesOnDeck: RunCurrentModulesOnDeck[] | ||
labwareOnDeck: RunCurrentLabwareOnDeck[] | ||
highlightLabwareEventuallyIn: string[] | ||
kind: 'intervention' |
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.
just want to make sure we always want the kind to be intervention?
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.
yeah. in this case we do because this is only picked up by error recovery use cases which always use that style of deck map. the deck config style is only used by the drop tip wiz
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.
code changes look really good! so much cleaner! I posted a question but besides that looks great
cbadbc1
to
13dc511
Compare
We're probably going to use this a lot more now that we'll have a bunch of content components that live in TwoColumn, so let's make StandIn reusable.
add deckmap fixtures and such more deckmap changes
my own? my... precious?
This should be a visually indistinguishable reimplementation.
Two places in error recovery, refactoring as we go. One big change was that there was a bunch of stuff called RecoveryMap as in recovery (deck) map, but recovery also has the Recovery Map, the big data structure that holds how all the steps of error recovery flow together. This is very confusing, so now what used to be RecoveryMap (component, utils) is now DeckMap.
13dc511
to
67a36a8
Compare
Adds DeckMapContent to intervention modal. This is a small wrapper around one of two different kinds of deck map:
Stories for the deckmap are on storybook, where it's rendered side-by-side with the standin since it is only destined to be used in a two-column layout.
Extra fun changes:
BaseDeck
, because otherwise they are completely unreadable once the deckmap gets to small. I think this looks a lot better everywhere, and is actually readable when the deckmap is small, so let's go with it; may want to come back and make these something more specific on desktopRecoveryMap
, a large data structure that is core to the wizard flow and defines how users move between steps and screens. Error recovery also had aRecoveryMap
, which was a component for rendering a deckmap, and auseRecoveryMapUtils
, a hook for getting that component's arguments. Now it uses theDeckMapContent
component above directly, anduseDeckMapUtils
, a hook for getting that component's argumentsReview requests:
Testing, todo:
Closes EXEC-501