fix: mobile modal button visible #138
Merged
Conversation
This file contains 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
✅ Deploy Preview for storybook-bloom-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
ColinBuyck
commented
Dec 1, 2023
| @@ -49,14 +49,22 @@ | |||
| width: auto; | |||
| } | |||
|
|
|||
| max-height: 100vh; | |||
There was a problem hiding this comment.
already set on lines 37 + 38 by the variables
ColinBuyck
commented
Dec 1, 2023
| @@ -254,30 +254,6 @@ export const ScrollableModalMinimalContent = () => { | |||
| ) | |||
| } | |||
|
|
|||
| export const TransparentOverlayModal = () => ( | |||
There was a problem hiding this comment.
Talking with Emily, we determined this story was not representative or realistic so it's removed here
cade-exygy
approved these changes
Dec 1, 2023
ludtkemorgan
approved these changes
Dec 4, 2023
|
🎉 This PR is included in version 12.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
16 tasks
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.
Pull Request Template
#133
Description
This PR updates the modal and overlay components so that the actions in the footer will be visible on mobile devices whether the bottom search bar is up or not. This fix is made possible by using dvh which is supported by over 90% of users (source).
In the event that dvh is not supported, this PR includes a fallback solution which adds static padding which will keep full height modals actionable.
Why does this work?
Setting the max height to 100dvh or dynamic view height prevents the modal buttons being blocked since it adjusts based on whether browser ux pieces are expanded or retracted (ie. search bar).
The fallback approach came to be since without dvh, we have to manually move the full-screen modal up to sit higher than browser ux pieces. This is achieved by the padding but then this causes non-fullscreeen modals to show a large footer. However with background-clip and removing the border and shadow, we can create that shift upward without detracting from the modals not taking up the full screen.
There is one edge case that wasn't able to be resolved based on the time-sensitivity of this fix and how rare this case is. The screen recroding below would only come up if a user had a mobile browser that didn't support dvh and was on a modal with overflow content (rather than scrollable). Also the screen recording doesn't account for the majority of the grayed out space being covered by the mobile browser's search or tool bar.
https://github.com/bloom-housing/ui-components/assets/53269332/0d6e6fb6-f5b4-4b9c-9aaa-72d98770dff8
How Can This Be Tested/Reviewed?
To test this directly, symlink this UIC to bloom and then run it locally. Access the public site on your mobile device and click into a listing with multiple images. Test Coliseum should work if you just seeded. Open the multi-photo modal and ensure the close button is visible at the bottom of your different browsers.
Also ensure the modal and image card stories are still functional on Storybook on both large and small devices.
A small note that in my testing, I noticed that this doesn't work for the multiple image card story if accessing storybook on your phone and then clicking full screen (button below). I think this is because of how storybook canvas represents a full screen and believe that local instance is support enough of this fix.
Checklist:
Reviewer Notes:
Steps to review a PR:
On merge, squash commits.