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
Add a screen that tells users that their browser is unsupported #13137
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we trigger any analytics event or any other mechanism to get data on unsupported browsers? I'm not familiar with how that works though...
/** | ||
* Build overlay for Landscape mode mobile | ||
*/ | ||
buildUnsupportedBrowserOverlay_() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also detach the amp-story-page
components from the DOM to make sure there's no sound / video playing?
Also, not very important since we're not playing the story, but as a good practice I think we should always interact with the DOM from vsync tasks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, for vsync.
Removing the elements from the DOM causes all kinds of errors since there are a number of race conditions introduced by doing that. Every AMP element manages its own lifecycle, so the other elements get messed up by being removed from the DOM when they weren't expecting it.
Added an expected error to track when users hit this case. |
LGTM |
…roject#13137) * Add unsupported browser screen. * Remove testing condition. * Add expected error and prevent layoutCallback * Fix line length * Assume browser is supported during tests. * Remove testing condition.
…roject#13137) * Add unsupported browser screen. * Remove testing condition. * Add expected error and prevent layoutCallback * Fix line length * Assume browser is supported during tests. * Remove testing condition.
Fixes #11853