-
Notifications
You must be signed in to change notification settings - Fork 716
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consolidate browser compatibility data, drop IE11 support #11685
Consolidate browser compatibility data, drop IE11 support #11685
Conversation
Build Artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes seem okay, but the browser requirements code is hard to follow. Added a comment about what in particular tripped me up, but happy to chat further about what might help
Have updated to try to make the parsing logic more self explanatory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for breaking this down - I find it much easier to comprehend. Per slack discussion, we'll go forward with this and check in with @MisRob re: TextTruncator and address in a follow up issue as appropriate
Follow up will happen in KDS where KTextTruncator has been implemented. |
Docs build failure is unrelated, merging. |
Summary
References
Fixes #5336
Reviewer guidance
The only other place I could find IE11 specific code was in the TextTruncatorCSS component, but I was not entirely sure how to remove the IE11 specific code. Advice from @MisRob would be appreciated, or we can file a follow up issue for this cleanup.
Testing checklist
PR process
Reviewer checklist
yarn
andpip
)