Commit 89fea23
authored
Fix safari scrolling issue (#7395)
### WHY are these changes introduced?
Scrolling and interacting with pages using index table may be slugging
on (newish versions?) of safari.
### Screenies
||Before|After|
|-|-|-|
|Timeline|<img width="356" alt="Screen Shot 2022-10-13 at 11 51 05 AM"
src="https://user-images.githubusercontent.com/24610840/195645166-7835c563-f756-46eb-a16e-6784b998defb.png">|<img
width="427" alt="Screen Shot 2022-10-13 at 11 57 29 AM"
src="https://user-images.githubusercontent.com/24610840/195646407-cff19381-90ea-4ac8-a5d8-499e861ea247.png">|
|Layers|<img width="693" alt="Screen Shot 2022-10-13 at 10 42 45 AM"
src="https://user-images.githubusercontent.com/24610840/195645603-0fa69e9f-f2d2-465b-8bfc-165b926f636c.png">|<img
width="322" alt="Screen Shot 2022-10-13 at 10 49 27 AM"
src="https://user-images.githubusercontent.com/24610840/195645994-4c81a595-5db1-4ed7-acc6-87a1ff967b38.png">|
|Layer count|<img width="149" alt="Screen Shot 2022-10-13 at 10 43 46
AM"
src="https://user-images.githubusercontent.com/24610840/195645673-85f481b3-31c7-4c0b-92db-612c35d99cbe.png">|<img
width="324" alt="Screen Shot 2022-10-13 at 10 48 54 AM"
src="https://user-images.githubusercontent.com/24610840/195645856-8494841f-f4fb-4b36-a094-67cb41ef04bf.png">|
### WHAT is this pull request doing?
I'm removing the `filter` property that's causing a new layer to be
create for each row. Painting each layer is taking a considerable amount
of time. But why not use the GPU? Sure, we could. But taking advantage
of the GPU would cause a significant bump in memory usage. I tested out
`will-change: filter` and noticed a resting memory usage of ~1gb
occasionally going higher (or lower).
<img width="336" alt="Screen Shot 2022-10-13 at 10 44 26 AM"
src="https://user-images.githubusercontent.com/24610840/195647680-cf3b9594-86be-4ea0-9cae-fc58e8199abf.png">
#### Why are we running into the issue?
`box-shadow` seems to have performance issue with scrolling. I haven't
located a recent thread but found many older posts on slack overflow of
similar issues and [webkit bug
reports](https://bugs.webkit.org/show_bug.cgi?id=22102).
But aren't we using drop-shadow not box shadow? Yes! However, upon
looking at the
[spec](https://drafts.fxtf.org/filter-effects/#funcdef-filter-drop-shadow)
it seems the values are interpreted as box-shadow with an additional
length property.
### Notes
There's still a few drop-shadows in the index table component. Since
we're using collapsing borders for the table they're needed.
### How to 🎩
Use the snapshot in web!
### 🎩 checklist
- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide1 parent c21ea83 commit 89fea23
File tree
2 files changed
+6
-1
lines changed- .changeset
- polaris-react/src/components/IndexTable
2 files changed
+6
-1
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
120 | 120 | | |
121 | 121 | | |
122 | 122 | | |
123 | | - | |
| 123 | + | |
124 | 124 | | |
125 | 125 | | |
126 | 126 | | |
| |||
0 commit comments