fix: ClientRect/DOMRect is not IE11 compatible #855
Conversation
Codecov Report
@@ Coverage Diff @@
## master #855 +/- ##
==========================================
- Coverage 94.5% 94.32% -0.19%
==========================================
Files 85 85
Lines 3496 3505 +9
Branches 532 531 -1
==========================================
+ Hits 3304 3306 +2
- Misses 66 74 +8
+ Partials 126 125 -1
Continue to review full report at Codecov.
|
packages/tab-scroller/index.tsx
Outdated
| computeScrollAreaClientRect: () => { | ||
| if (!this.areaElement.current) { | ||
| // new DOMRect is not IE11 compatible | ||
| const defaultDOMRect = { |
There was a problem hiding this comment.
Could you make this a variable that both computeScrollAreaClientRect and computeScrollContentClientRect reference, instead of defining it twice?
There was a problem hiding this comment.
I created a function getBoundingClientRectOf within tab-scroller.
I considered a shared variable (either within the class or the adapter) but if code happened to modify the returned object, then it would result in these functions no longer returning the expected default object.
packages/chips/Chip.tsx
Outdated
| return this.chipElement.getBoundingClientRect(); | ||
| }, | ||
| getCheckmarkBoundingClientRect: () => { | ||
| const {chipCheckmark} = this.props; | ||
| if (!chipCheckmark) return new ClientRect(); | ||
| if (!chipCheckmark) { |
There was a problem hiding this comment.
@moog16 Should this if also check chipCheckmark.props.getBoundingClientRect?
There was a problem hiding this comment.
Yes, that doesn't hurt to have the check there.
There was a problem hiding this comment.
I won't merge it if you want to update this...you may also want to check unit tests.
There was a problem hiding this comment.
Added new commit.
Couldn't run tests due to fsevents failing to build on Node v12 on Mac.
I'll try to see about switching to nvm tomorrow and running the tests.
packages/tab-scroller/index.tsx
Outdated
| computeHorizontalScrollbarHeight: () => | ||
| computeHorizontalScrollbarHeight(document), | ||
| }; | ||
| } | ||
|
|
||
| getBoundingClientRectOf = (element: HTMLElement) => { |
|
@moog16 The new commit fixed the unit tests. Let me know if you want me to rebase these changes down to a single commit. |
Fixes #853.
@moog16 I copied the usage from
rippleandtab-barso that it'd be easier to do a find & replace in the future.Let me know if there's anything else I need to do.