-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[Modal] Fix zoom out on iOS #19548
[Modal] Fix zoom out on iOS #19548
Conversation
Details of bundle changes.Comparing: 62e439b...61a15ab
|
…t styles found in Backdrop and SimpleBackdrop js files.
@TommyJackson85 You can test on mobile using the local dev tool. When you start the documentation,
If you can get a real phone on the same network, you can navigate the URL and test the changes without a deploy on Netlify :). |
oh that is great :) I'll consider it for next time. How do you feel about it now? Will there need to be anymore changes? There is an Argos error. By the way, sorry I forgot to delete that comment :) I wrote it because I just wanted to make an initital commit to create the PR first for testing. |
@TommyJackson85 I think that we are good with Argos. I have approved the change, it's a flaky test. Something is strange, I have seen it fails frequently lately, maybe the logic that wait the font to load doesn't work anymore, no idea. @missbruni Could you make sure it works as expected on https://deploy-preview-19548--material-ui.netlify.com/? Thanks :) |
Well I noticed it worked after I submitted your changes from manually testing. Why was the code put there in the first place (in which you suggested to remove)?
…Sent from my iPhone
On 3 Feb 2020, at 21:56, Olivier Tassinari ***@***.***> wrote:
@TommyJackson85 I think that we are good with Argos. I have approved the change, it's a flaky test. Something is strange, I have seen it fails frequently lately, maybe the logic that waits the font to load doesn't work anymore, no idea.
@missbrun Could you make sure it works as expected on https://deploy-preview-19548--material-ui.netlify.com/? Thanks
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@TommyJackson85 Yes, I confirm, it works :) |
@oliviertassinari and @TommyJackson85 thanks! |
@oliviertassinari I can see that the fix is working here: https://deploy-preview-19548--material-ui.netlify.com/ but not on the latest version 4.9.1, is this fix yet to be released? |
It will be release within a week. |
Attempting to fix this issue.
-test static seems to be failing.
Deployed link of this branch to test on mobile:
https://deploy-preview-19548--material-ui.netlify.com/
Anything else I can do to test it let me know. Before merging, It might good to use this PR to test other instances of this issue if they exist elsewhere and not just dialog fullscreen. Like other pages with similar code:
// Disable scroll capabilities. touchAction: 'none',
I will keep editing / updating this initial post with the significant changes.
Closes #19117