Add info about last version supporting IE9 and IE10 and remove obsolete code #5373
Conversation
src/lib/loggers.js
Outdated
| } | ||
| // `this` should always be console, since here we're always | ||
| // applying a method of the console object. | ||
| f.apply(console, args); |
There was a problem hiding this comment.
Could remove this function entirely now, just inline the apply calls. Also the console.trace fallback || console.log in log and warn could be removed
There was a problem hiding this comment.
Good call. Done in bc2b0d1.
test/image/strict-d3.js
Outdated
| @@ -2,6 +2,9 @@ | |||
| * strict-d3: wrap selection.style to prohibit specific incorrect style values | |||
| * that are known to cause problems in IE (at least IE9) | |||
| */ | |||
|
|
|||
| // Do we need this process now that IE9 and IE10 are not supported? | |||
There was a problem hiding this comment.
yes, if you look down below there's plenty here that applies to other browsers too. There may be some parts we could relax but no harm leaving it all for now.
There was a problem hiding this comment.
OK. I removed the comment in 9276d08.
|
Hmm so one last-minute concern I have here is that we dropped support in our build, but anyone building themselves without |
src/lib/index.js
Outdated
| @@ -727,6 +722,8 @@ lib.isIOS = function() { | |||
| return IS_IOS_REGEX.test(window.navigator.userAgent); | |||
| }; | |||
|
|
|||
| // Do we need this process now that IE9 and IE10 are not supported? | |||
There was a problem hiding this comment.
yeah, we could probably get rid of this now - and revert the change here that's its only consumer: 98adfae#diff-9c325f1fe845cf45c705407012f4d4d0be31a9d9fbead70d1f70c5dedde9c634
There was a problem hiding this comment.
Addressed in 42c97e2.
I disagree - I think it's easier to simply say "we don't support IE<11 anymore," otherwise we're going to keep carrying around arcane practices nobody can remember the rationale for... which of course we have plenty of but each instance of this has adds nonzero ongoing cost. And I really don't want to deal with questions like "OK I know you SAID you don't support these browsers but I found this crazy workaround to still build for them and you broke it with something in version 2.13.0." |
|
OK, I see your point, Alex. I guess I'm worried that some people might be happily using IE9/IE10 (ok, "happily") and then in a minor we drop support for them, which is not semver-friendly. Arguably we already did that in 1.49.5 but that's not really a good reason to double down. |
|
Fair enough. How about we split this PR in two: one we merge right now that just documents what already happened but makes no code changes; and a second that we merge in the upcoming v2 that removes IE9/10 cruft. |
|
And we can include a note:
|
I kind of disagree. That could potentially be a blocker for getting #5230 in v1. |
That also collides with #5358. |
|
I think that we can and should merge a PR right away that only:
Let's not add all sorts of additional opportunistic cleanup as part of this otherwise-uncontroversial documentation PR please, and we can discuss all the other v1/v2 stuff tomorrow? |
@plotly/plotly_js