nusmodifications / nusmods

🏫 Official course planning platform for National University of Singapore.
https://nusmods.com
MIT License
588 stars 319 forks source link

Display warning to users with unsupported browser #686

Closed ZhangYiJiang closed 6 years ago

ZhangYiJiang commented 6 years ago

Our browser support matrix is

Desktop

Mobile

For users on browsers that are not supported, we should show some sort of warning when NUSMods is first launched that they may experience issues. The warning should be dismissible and that preference persisted. It can be useful to include upgrade paths for common unsupported browsers (eg. Chrome/Firefox on Windows 7 and older OS X)

For best effect it may be included outside the normal React app, since browser issues may prevent the app from running at all. May also want to design two different levels of warnings, one for browsers with known issues (old Safari, IE) and ones which are just slightly outdated (non-recent version of evergreen browsers).

ZhangYiJiang commented 6 years ago

Example of error from unsupported browser (Chrome Mobile 34): https://sentry.io/nusmods/v3/issues/432780391/

ZhangYiJiang commented 6 years ago

Another one - Safari 8: https://sentry.io/nusmods/v3/issues/432781077/

ZhangYiJiang commented 6 years ago

IE11 on Windows 7: https://sentry.io/nusmods/v3/issues/432782926/

liaujianjie commented 6 years ago

Can I take up this issue? The most straightforward way to do this is probably by checking against the user agent with static javascript. The alternative would be to detect features instead of browsers.

There's a good library that'll make this significantly easier. It's 20kb when minified.

ZhangYiJiang commented 6 years ago

I'm actually already working on this using Bowser, which is slightly smaller because it is more limited in scope and has a nice check method that makes it quite easy to check browser versions. I'm still working on the UI, so if you want to do that I can hand the branch over to you.

ZhangYiJiang commented 6 years ago

@liaujianjie I have pushed my unfinished code up at https://github.com/nusmodifications/nusmods/tree/browser-warning - comment here if you want to take over.

liaujianjie commented 6 years ago

@ZhangYiJiang hey thanks, I'd love to take over.

liaujianjie commented 6 years ago

How's this implementation UI-wise? I'm not sure if it is a strict requirement to adhere to the MD specs for this feature.

screen shot 2018-02-04 at 4 49 33 pm

I intend to store a flag in localStorage when a user presses "Don't show again".

ZhangYiJiang commented 6 years ago

I'm not sure what you mean by MD specs, but that looks good. Be careful about using CSS / JS features not present in older browsers. localStorage should be fine because it has support since IE8.