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: add browser warning message #4048

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adamspofford-dfinity
Copy link
Contributor

@adamspofford-dfinity adamspofford-dfinity commented Dec 20, 2024

This adds a hidden div to frontend templates to display a message suggesting switching browsers when the page breaks, to solve the Safari problem in a different way from #4011. When documentation is written on browser breakage this message should be updated to link to it.

@adamspofford-dfinity adamspofford-dfinity marked this pull request as ready for review December 20, 2024 19:37
@adamspofford-dfinity adamspofford-dfinity requested a review from a team as a code owner December 20, 2024 19:37
@krpeacock
Copy link
Contributor

I don't like the implication that Safari is an unsupported browser - that would spook me as a developer

@adamspofford-dfinity
Copy link
Contributor Author

@krpeacock I mean, it's not an implication, it's straightforwardly true. An unadorned 'try switching browsers' would just lead to bug reports about it.

@krpeacock
Copy link
Contributor

It's not nuanced enough. There is no reason that Safari shouldn't work in production, and a developer seeing this message in their starter template would believe that the Internet Computer is incompatible with Safari, which makes the platform a non-starter

Copy link
Contributor

@krpeacock krpeacock left a comment

Choose a reason for hiding this comment

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

Message is too blunt - I don't want us to get in trouble for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants