Make legend toggle work in IE11 #2741
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2741 +/- ##
==========================================
- Coverage 82.85% 82.82% -0.03%
==========================================
Files 60 60
Lines 4776 4791 +15
==========================================
+ Hits 3957 3968 +11
- Misses 819 823 +4
Continue to review full report at Codecov.
|
|
@GDFaber Thank you for looking into this. The change looks almost good, but we'd like to minimize the effect of change. So can you add the conditional like the below instead of just changing to 'block'? |
|
Hi @kt3k, sure, I'll add IE detection as stated above. I have a question about that: I found a check for IE9 in clip.js/ChartInternal.prototype.getClipPath() as well. Should I add a detection function to util.js (as a part of this PR) and let getClipPath() use it (this would be a new PR then)? or is it ok to just check locally and leave clip.js as it is? |
That sounds good to me. |
|
src/util.js
Outdated
| */ | ||
| export var getIEVersion = function() { | ||
| // https://stackoverflow.com/questions/19999388/check-if-user-is-using-ie | ||
| const agent = window.navigator.userAgent; |
There was a problem hiding this comment.
How about taking agent as function paramter? If we do so, we can write the tests like the below:
expect(getIEVersion('Mozilla/5.0 (Windows NT 6.3; Trident/7.0; rv:11.0) like Gecko ')).toBe(11)
There was a problem hiding this comment.
Sure, let's do this. I'm going to modify getIEVersion() to take an optional parameter so usage is kept simple and it can be tested as well.
| @@ -10,7 +11,7 @@ Chart.prototype.show = function (targetIds, options) { | |||
| targets = $$.svg.selectAll($$.selectorTargets(targetIds)); | |||
|
|
|||
| targets.transition() | |||
| .style('display', 'initial', 'important') | |||
| .style('display', isIE() ? 'block' : 'initial', 'important') | |||
display: initialbydisplay: blockin Chart.prototype.show() to stop hidden data items from disappearing in IE11 when they should be shown again (as described in IE 11 legend toggle #2528).