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

fix(modal): ensure modal renders correctly on mobile safari #5776

Merged
merged 10 commits into from
Apr 3, 2020

Conversation

tw15egan
Copy link
Member

@tw15egan tw15egan commented Apr 1, 2020

Closes #5755

Fixes a bug in regards to mobile Safari and the way they render the bottom menu bar (see link for more info). Uses browser-specific media query to fix the issue (MDN)

Changelog

New

  • iOS-specific styles so that modal footer appears when using modal on iPhone

Testing / Reviewing

Test out Modal on an iPhone, or use the simulator via Xcode

@tw15egan tw15egan requested a review from a team as a code owner April 1, 2020 17:14
@ghost ghost requested review from abbeyhrt and emyarod April 1, 2020 17:14
@tw15egan tw15egan requested review from a team and laurenmrice and removed request for a team April 1, 2020 17:14
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

this resolves the issue (tested on mobile safari with a local build)

image

looks like the original position: relative is just carried over from 4dcd149. just wondering, instead of adding a specific media query could we make position: fixed; top: 0 the default behavior change the position back to its initial value at larger breakpoints?

@tw15egan
Copy link
Member Author

tw15egan commented Apr 1, 2020

@emyarod Yeah let's just do that instead

@netlify
Copy link

netlify bot commented Apr 1, 2020

Deploy preview for carbon-elements ready!

Built with commit 6c2e01c

https://deploy-preview-5776--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Apr 1, 2020

Deploy preview for carbon-components-react ready!

Built with commit 6c2e01c

https://deploy-preview-5776--carbon-components-react.netlify.com

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

looks good to me pending design review

image

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - I think the change would affect some demonstration sites where modal is demonstrated in a non-<iframe>, non-full-window container, but I guess they have to override the style to make it work. Thanks @tw15egan!

@tw15egan tw15egan merged commit daf9c7d into carbon-design-system:master Apr 3, 2020
@tw15egan tw15egan deleted the safari-modal-fix branch April 28, 2021 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Modal] Modal actions not visible in IOS safari
4 participants