check browser compatibility simply #654
Conversation
ui/public/checkBrowser.js
Outdated
|
|
||
| // This part code is auto generated by `yarn supportedBrowsers` command. | ||
| // Don't edit it and always keep it in the end of this file. It means don't add new content after it. | ||
| ;checkBrowser(/((CPU[ +]OS|iPhone[ +]OS|CPU[ +]iPhone|CPU IPhone OS)[ +]+(9[_\.]3|9[_\.]([4-9]|\d{2,})|([1-9]\d|\d{3,})[_\.]\d+|11[_\.]3|11[_\.]([4-9]|\d{2,})|(1[2-9]|[2-9]\d|\d{3,})[_\.]\d+|12[_\.]0|12[_\.]([1-9]|\d{2,})|12[_\.]4|12[_\.]([5-9]|\d{2,})|(1[3-9]|[2-9]\d|\d{3,})[_\.]\d+|13[_\.]0|13[_\.]([1-9]|\d{2,})|13[_\.]3|13[_\.]([4-9]|\d{2,})|(1[4-9]|[2-9]\d|\d{3,})[_\.]\d+)(?:[_\.]\d+)?)|(CFNetwork\/8.* Darwin\/16\.5\.\d+)|(CFNetwork\/8.* Darwin\/16\.6\.\d+)|(CFNetwork\/8.* Darwin\/16\.7\.\d+)|(SamsungBrowser\/(10\.1|10\.([2-9]|\d{2,})|(1[1-9]|[2-9]\d|\d{3,})\.\d+))|(Edge\/(18(?:\.0)?|18(?:\.([1-9]|\d{2,}))?|(19|[2-9]\d|\d{3,})(?:\.\d+)?))|((Chromium|Chrome)\/(49\.0|49\.([1-9]|\d{2,})|([5-9]\d|\d{3,})\.\d+|79\.0|79\.([1-9]|\d{2,})|([8-9]\d|\d{3,})\.\d+)(?:\.\d+)?([\d.]+$|.*Safari\/(?![\d.]+ Edge\/[\d.]+$)))|(Version\/(12\.1|12\.([2-9]|\d{2,})|(1[3-9]|[2-9]\d|\d{3,})\.\d+|13\.0|13\.([1-9]|\d{2,})|(1[4-9]|[2-9]\d|\d{3,})\.\d+)(?:\.\d+)? Safari\/)|(Trident\/7\.0)|(Firefox\/(68\.0|68\.([1-9]|\d{2,})|(69|[7-9]\d|\d{3,})\.\d+|74\.0|74\.([1-9]|\d{2,})|(7[5-9]|[8-9]\d|\d{3,})\.\d+)\.\d+)|(Firefox\/(68\.0|68\.([1-9]|\d{2,})|(69|[7-9]\d|\d{3,})\.\d+|74\.0|74\.([1-9]|\d{2,})|(7[5-9]|[8-9]\d|\d{3,})\.\d+)(pre|[ab]\d+[a-z]*)?)|(([MS]?IE) (11\.0|11\.([1-9]|\d{2,})|(1[2-9]|[2-9]\d|\d{3,})\.\d+))/) |
There was a problem hiding this comment.
Make the supported browser list fixed, don't change them every time when building anymore. It may be changed by manually running yarn supportedBrowsers.
There was a problem hiding this comment.
Considering that this part will be auto-generated, how about simply use placeholders like __SUPPORTED_BROWSERS__?
There was a problem hiding this comment.
Do you mean ;checkBrowser(__SUPPORTED_BROWSERS__)? But I think the content should be fixed and don't change often, so they should be included in the code repo, right?
Or do you mean var __SUPPORTED_BROWSERS__ = /.../, I tried this for the first time but found it doesn't work. Because we defined it in the last, when we access it before, its value is null.
There was a problem hiding this comment.
Oh, I mean, make this file kind of "template", and plug it into the gulp's standard stream to generate a final script content.
You are right, it is indeed changed not very often. I'm ok with the current implementation now.
| '一些功能在此浏览器上可能无法工作，请使用最新版本的 Chrome/Edge/Firefox/Safari 浏览器。' | ||
| } else { | ||
| text = | ||
| 'Some features may not work in your browser. Please use latest Chrome/Edge/Firefox/Safari browsers.' |
There was a problem hiding this comment.
I think it is enough to use latest to represent the version instead of a specific version value. Because when people want to upgrade or download an app, they just care about the newest version. How do you think about it? @breeswish
ui/package.json
Outdated
| @@ -47,7 +47,8 @@ | |||
| "build": "gulp build", | |||
| "build:library": "gulp build:library", | |||
| "test": "react-app-rewired test", | |||
| "fmt": "prettier --write ." | |||
| "fmt": "prettier --write .", | |||
| "supportedBrowsers": "gulp supportedBrowsers" | |||
There was a problem hiding this comment.
Better to add a verb for it to be aligned with others, like, gen:browserlist
There was a problem hiding this comment.
Good naming, will update it.
ui/gulpfile.esm.js
Outdated
| @@ -28,6 +28,15 @@ task( | |||
| shell.task('yarn react-app-rewired build --library') | |||
| ) | |||
|
|
|||
| task( | |||
There was a problem hiding this comment.
Is it better to use gulp style file process? e.g. input a file, transform, and finally output. We can also use the API to get a list of supported browsers
There was a problem hiding this comment.
Sounds good, let me try it.
|
@breeswish,Thanks for you review. |
A simpler version of PR #634 .
Just show a prompt message for the browser that is not included in the supported browsers list.