Skip to content
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): split InterventionModal to molecule #15174

Merged
merged 3 commits into from
May 17, 2024

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented May 13, 2024

The implementation of the intervention modal (i.e., manual move labware) in the desktop app was sort of manually open-coded in the intervention modal organism. We're going to need modals styled in this way elsewhere, so split it out into an app molecule with some overridable styles.

Also, make its background wrapper position: absolute instead of position: fixed, for the same reasons as #15166 - when this modal is hung off of the modal portal rather than the top portal, it should allow interaction with the navbar and the breadcrumbs. This should not affect the modal's use in the actual InterventionRequired organism, since in the organism the modal is hung off of the top portal.

Closes RSQ-6

Testing

  • The new storybook component should render
  • Move labware and pause modals on desktop should still render as in figma the shipping app
  • Move labware and pause modals on desktop should continue to (a) visually overlap and (b) inhibit interaction with the navbar and breadcrumbs

the shipping app has these modals confined to the route component area, while figma has them overlapping everything with a small padding to the viewport boundary. this pr doesn't change that. the navbar and route breadcrumbs are still non interactive.

The implementation of the intervention modal (i.e., manual move labware)
in the desktop app was sort of manually open-coded in the intervention
modal organism. We're going to need modals styled in this way elsewhere,
so split it out into an app molecule with some overridable styles.

Also, make its background wrapper position: absolute instead of
position: fixed, for the same reasons as #15166 - when this modal is
hung off of the modal portal rather than the top portal, it should allow
interaction with the navbar and the breadcrumbs. This should not affect
the modal's use in the actual InterventionRequired organism, since in
the organism the modal is hung off of the top portal.

Closes RSQ-6
@sfoster1 sfoster1 requested a review from a team as a code owner May 13, 2024 20:23
@sfoster1 sfoster1 requested review from b-cooper and mjhuff and removed request for a team May 13, 2024 20:23
Copy link
Contributor

@mjhuff mjhuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the refactor. We'll definitely put this to good use soon!

@sfoster1 sfoster1 merged commit f4e87f9 into edge May 17, 2024
20 checks passed
@sfoster1 sfoster1 deleted the rsq-6-intervention-modal-molecule branch May 17, 2024 14:15
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
The implementation of the intervention modal (i.e., manual move labware)
in the desktop app was sort of manually open-coded in the intervention
modal organism. We're going to need modals styled in this way elsewhere,
so split it out into an app molecule with some overridable styles.

Also, make its background wrapper position: absolute instead of
position: fixed, for the same reasons as #15166 - when this modal is
hung off of the modal portal rather than the top portal, it should allow
interaction with the navbar and the breadcrumbs. This should not affect
the modal's use in the actual InterventionRequired organism, since in
the organism the modal is hung off of the top portal.

Closes RSQ-6

## Testing
- [x] The new storybook component should render
- [x] Move labware and pause modals on desktop should still render as in
~figma~ the shipping app
- [x] Move labware and pause modals on desktop should continue to ~(a)
visually overlap and (b)~ inhibit interaction with the navbar and
breadcrumbs

the shipping app has these modals confined to the route component area,
while figma has them overlapping everything with a small padding to the
viewport boundary. this pr doesn't change that. the navbar and route
breadcrumbs are still non interactive.
Carlos-fernandez pushed a commit that referenced this pull request Jun 3, 2024
The implementation of the intervention modal (i.e., manual move labware)
in the desktop app was sort of manually open-coded in the intervention
modal organism. We're going to need modals styled in this way elsewhere,
so split it out into an app molecule with some overridable styles.

Also, make its background wrapper position: absolute instead of
position: fixed, for the same reasons as #15166 - when this modal is
hung off of the modal portal rather than the top portal, it should allow
interaction with the navbar and the breadcrumbs. This should not affect
the modal's use in the actual InterventionRequired organism, since in
the organism the modal is hung off of the top portal.

Closes RSQ-6

## Testing
- [x] The new storybook component should render
- [x] Move labware and pause modals on desktop should still render as in
~figma~ the shipping app
- [x] Move labware and pause modals on desktop should continue to ~(a)
visually overlap and (b)~ inhibit interaction with the navbar and
breadcrumbs

the shipping app has these modals confined to the route component area,
while figma has them overlapping everything with a small padding to the
viewport boundary. this pr doesn't change that. the navbar and route
breadcrumbs are still non interactive.
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.

None yet

2 participants