feat: Edge on Chromium and FF Mobile support

mikemaccana / outdated-browser-rework

Detects outdated browsers and advises users to upgrade to a new version. Handles mobile devices!

MIT License

226 stars 63 forks source link

feat: Edge on Chromium and FF Mobile support #97

Closed benmccallum closed 4 years ago

benmccallum commented 4 years ago

Fixes #96
- Essentially means past v18 (last legacy Edge version), it'll use the major version straight from the useragent (as Chrome would be handled) so you can now start putting 79, 80 etc. in your Edge config value. Versions source: https://en.wikipedia.org/wiki/Microsoft_Edge#Release_history
Fixes #98
- Handles Firefox Mobile as Mobile Safari, since it's the same thing. See comments on #98
Unit tests
- Abstracts out browser evaluation to a testable exported function
- Adds tests for cases above of "is browser out-of-date", "is browser unsupported" and "is android but not chrome"

benmccallum commented 4 years ago

Hi @mikemaccana, do you think we could get this out? Solves two noteworthy issues. I've built the source, bumped the version number. Literally just needs a merge and an npm publish AFAIK.

mikemaccana commented 4 years ago

Thanks @benmccallum! If you add some tests I'll merge.

benmccallum commented 4 years ago

Should I add some jest tests?

benmccallum commented 4 years ago

Hey @mikemaccana,

I've written some jest tests to cover the enhancements I've made (edge mapping before vs after they got on Chromium and FF mobile being handled as Safari Mobile stuff).

In doing this, I had to abstract out the browser evaluation part of index.js so I could easily just test what I wanted/needed to.

I've also written tests to cover "is browser unsupported" cases and "is android but not chrome" cases. The only stuff not covered now is isPropertySupported, but that can wait for another day/person.

I've also tested the behaviour in the index.html, by toggling a bunch of settings and checking what should come up or not.

LGTM!

benmccallum commented 4 years ago

@mikemaccana quick bump. Sorry to hassle. Couple of nice improvements here we'd like to get out.

benmccallum commented 4 years ago

bump again

virtualize commented 4 years ago

I also like to see this merged. What's stopping you @mikemaccana?

jfbloom22 commented 4 years ago

The changes in this PR are working well for me. Right now I have Edge: 80, configured. My goal is to consider Edge before the switch to Chromium outdated. However, after a lot of searching I was not able to figure out what version of Edge they switched to Chromium. Anyone happen to know?

benmccallum commented 4 years ago

@jfbloom22 the wikipedia article for Edge has this info I believe in a big table.

benmccallum commented 4 years ago

I put a fair bit of effort into this @mikemaccana, and folks seem to want it. Is it possible to ask again for it to be merged so we can get a proper release out?

mikemaccana commented 4 years ago

Sorry I've missed this, I've had so much spam from people who didn't write PRs I've started to ignore GitHub comments. I'll merge it soon!

On Tue, 8 Sep 2020, 07:59 Ben McCallum, notifications@github.com wrote:

I put a fair bit of effort into this @mikemaccana https://github.com/mikemaccana, and folks seem to want it. Is it possible to ask again for it to be merged so we can get a proper release out?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mikemaccana/outdated-browser-rework/pull/97#issuecomment-688661175, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABKEMTNDFBD6T3A7E3LNYTSEXI5JANCNFSM4K7SA4VA .

mikemaccana commented 4 years ago

Merged in v3!