fix(app): set max height for desktop modal shell #14915
Conversation
We special-cased the size of the modal shell to accommodate a large body for the 96-channel pipette attachment BeforeBeginning step to avoid scrollable content. The size was set to 70% of the screen, but that resulted in a lot of modal whitespace when the app was resized to larger than default dimensions. Here, I add a max height of 31rem to accommodate the content of the largest pipette wizard flow tile content without growing arbitrarily large with a resized desktop app. I also ensure minimum padding between tile content and footer buttons, and fix padding on the ChoosePipette tile.
| @@ -105,7 +105,7 @@ const ModalArea = styled.div< | |||
| >` | |||
| position: ${POSITION_RELATIVE}; | |||
| overflow-y: ${OVERFLOW_AUTO}; | |||
| max-height: 100%; | |||
| max-height: ${({ isFullPage }) => (isFullPage ? '100%' : '31rem')}; | |||
There was a problem hiding this comment.
doesn't this change affect every component that uses LegacyModalShell since the default is now 31rem instead of 100%?
There was a problem hiding this comment.
Agree with @jerader. I think LegacyModal is due for a bit of a re-think, but I'm all for avoiding changing the base component if we can help it. If we are special-casing the attach flows due to the 96 graphic, let's pass in custom CSS.
There was a problem hiding this comment.
For the time being, I'll leave alone the max height here til we can give some more thought to standardizing our modal styling.
| <Flex | ||
| flexDirection={DIRECTION_ROW} | ||
| gridGap={SPACING.spacing24} | ||
| paddingBottom={SPACING.spacing24} |
There was a problem hiding this comment.
See other comment. I don't think we want to change base components if we can help it.
There was a problem hiding this comment.
Fair point. I will leave this alone.
closes RQA-2279
Overview
We special-cased the size of the modal shell to accommodate a large body for the 96-channel pipette attachment BeforeBeginning step to avoid scrollable content. The size was set to 70% of the screen, but that resulted in a lot of modal whitespace when the app was resized to larger than default dimensions. Here, I add a max height of 31rem to accommodate the content of the largest pipette wizard flow tile content without growing arbitrarily large with a resized desktop app. I also ensure minimum padding between tile content and footer buttons, and fix padding on the ChoosePipette tile.
Test Plan
I also smoketested various wizard flows that use LegacyModalShell.
Changelog
Review requests
frontend friends
Risk assessment
low