adobe
fix(colorarea): remove layout thrashing and cache selectors
Remove layout invalidation from ColorArea and cache selectors
Description
This change removes layout invalidation/recalculation from the ColorArea when adjusting the current color, either with the mouse or with keyboard input. The change also caches the selectors for the queries so they don't need to be recalculated every time they're accessed.
Motivation and context
The layout thrashing had some negative performance impact, most noticeable in Safari on Intel based Macs.
Related issue(s)
- fixes https://github.com/adobe/spectrum-web-components/issues/5541
- resolves [SWC-927]
Screenshots (if appropriate)
Before:
After:
Author's checklist
- [ ] I have read the CONTRIBUTING and PULL_REQUESTS documents.
- [ ] I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
- [ ] I have added automated tests to cover my changes.
- [ ] I have included a well-written changeset if my change needs to be published.
- [ ] I have included updated documentation if my change required it.
Reviewer's checklist
- [ ] Includes a Github Issue with appropriate flag or Jira ticket number without a link
- [ ] Includes thoughtfully written changeset if changes suggested include
patch,minor, ormajorfeatures - [ ] Automated tests cover all use cases and follow best practices for writing
- [ ] Validated on all supported browsers
- [ ] All VRTs are approved before the author can update Golden Hash
Manual review test cases
-
[ ] Descriptive Test Statement
- Go here
- Do this action
- Expect this result
-
[ ] Descriptive Test Statement
- Go here
- Do this action
- Expect this result
Device review
- [ ] Did it pass in Desktop?
- [ ] Did it pass in (emulated) Mobile?
- [ ] Did it pass in (emulated) iPad?
🦋 Changeset detected
Latest commit: 36544173c7ad600fb79e38ed3e13da4514854a6c
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 84 packages
| Name | Type |
|---|---|
| @spectrum-web-components/color-area | Patch |
| @spectrum-web-components/bundle | Patch |
| documentation | Patch |
| @spectrum-web-components/eslint-plugin | Patch |
| @spectrum-web-components/accordion | Patch |
| @spectrum-web-components/action-bar | Patch |
| @spectrum-web-components/action-button | Patch |
| @spectrum-web-components/action-group | Patch |
| @spectrum-web-components/action-menu | Patch |
| @spectrum-web-components/alert-banner | Patch |
| @spectrum-web-components/alert-dialog | Patch |
| @spectrum-web-components/asset | Patch |
| @spectrum-web-components/avatar | Patch |
| @spectrum-web-components/badge | Patch |
| @spectrum-web-components/breadcrumbs | Patch |
| @spectrum-web-components/button-group | Patch |
| @spectrum-web-components/button | Patch |
| @spectrum-web-components/card | Patch |
| @spectrum-web-components/checkbox | Patch |
| @spectrum-web-components/clear-button | Patch |
| @spectrum-web-components/close-button | Patch |
| @spectrum-web-components/coachmark | Patch |
| @spectrum-web-components/color-field | Patch |
| @spectrum-web-components/color-handle | Patch |
| @spectrum-web-components/color-loupe | Patch |
| @spectrum-web-components/color-slider | Patch |
| @spectrum-web-components/color-wheel | Patch |
| @spectrum-web-components/combobox | Patch |
| @spectrum-web-components/contextual-help | Patch |
| @spectrum-web-components/dialog | Patch |
| @spectrum-web-components/divider | Patch |
| @spectrum-web-components/dropzone | Patch |
| @spectrum-web-components/field-group | Patch |
| @spectrum-web-components/field-label | Patch |
| @spectrum-web-components/help-text | Patch |
| @spectrum-web-components/icon | Patch |
| @spectrum-web-components/icons-ui | Patch |
| @spectrum-web-components/icons-workflow | Patch |
| @spectrum-web-components/icons | Patch |
| @spectrum-web-components/iconset | Patch |
| @spectrum-web-components/illustrated-message | Patch |
| @spectrum-web-components/infield-button | Patch |
| @spectrum-web-components/link | Patch |
| @spectrum-web-components/menu | Patch |
| @spectrum-web-components/meter | Patch |
| @spectrum-web-components/modal | Patch |
| @spectrum-web-components/number-field | Patch |
| @spectrum-web-components/overlay | Patch |
| @spectrum-web-components/picker-button | Patch |
| @spectrum-web-components/picker | Patch |
| @spectrum-web-components/popover | Patch |
| @spectrum-web-components/progress-bar | Patch |
| @spectrum-web-components/progress-circle | Patch |
| @spectrum-web-components/radio | Patch |
| @spectrum-web-components/search | Patch |
| @spectrum-web-components/sidenav | Patch |
| @spectrum-web-components/slider | Patch |
| @spectrum-web-components/split-view | Patch |
| @spectrum-web-components/status-light | Patch |
| @spectrum-web-components/swatch | Patch |
| @spectrum-web-components/switch | Patch |
| @spectrum-web-components/table | Patch |
| @spectrum-web-components/tabs | Patch |
| @spectrum-web-components/tags | Patch |
| @spectrum-web-components/textfield | Patch |
| @spectrum-web-components/thumbnail | Patch |
| @spectrum-web-components/toast | Patch |
| @spectrum-web-components/tooltip | Patch |
| @spectrum-web-components/top-nav | Patch |
| @spectrum-web-components/tray | Patch |
| @spectrum-web-components/underlay | Patch |
| @spectrum-web-components/custom-vars-viewer | Patch |
| @spectrum-web-components/story-decorator | Patch |
| @spectrum-web-components/vrt-compare | Patch |
| @spectrum-web-components/base | Patch |
| @spectrum-web-components/grid | Patch |
| @spectrum-web-components/opacity-checkerboard | Patch |
| @spectrum-web-components/reactive-controllers | Patch |
| @spectrum-web-components/shared | Patch |
| @spectrum-web-components/styles | Patch |
| @spectrum-web-components/theme | Patch |
| @spectrum-web-components/truncated | Patch |
| example-project-rollup | Patch |
| example-project-webpack | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
![changeset-bot[bot] avatar](https://avatars.githubusercontent.com/in/28616?v=4 "Published by a pub.dev verified publisher"){kind=small}
Branch preview
Review the following VRT differences
When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
- Spectrum | Light | Medium | LTR
- Spectrum | Dark | Large | RTL
- Express | Light | Medium | LTR
- Express | Dark | Large | RTL
- Spectrum-two | Light | Medium | LTR
- Spectrum-two | Dark | Large | RTL
- High Contrast Mode | Medium | LTR
If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Tachometer results
Chrome
color-area permalink
basic-test
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 610 kB | 128.46ms - 130.52ms | - | faster ✔ 4% - 6% 5.58ms - 8.22ms |
| branch | 587 kB | 135.56ms - 137.21ms | slower ❌ 4% - 6% 5.58ms - 8.22ms |
- |
Firefox
color-area permalink
basic-test
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 610 kB | 200.30ms - 207.62ms | - | faster ✔ 1% - 7% 2.71ms - 13.93ms |
| branch | 587 kB | 208.03ms - 216.53ms | slower ❌ 1% - 7% 2.71ms - 13.93ms |
- |
@tjgupta can you please include detailed testing instructions? Also should this cause a visual change at all, I'm seeing a large number of VRT failures?
caveat: please don't update the golden hash until a second reviewer gives a thumbs up if visual changes are expected
Thanks for your contribution @tjgupta . Can you please add a script to set x and y rapidly like below.
const colorArea = document.querySelector('sp-color-area');
for (let i = 0; i <= 1; i += 0.01) {
colorArea.x = i;
colorArea.y = 1 - i;
}
Let's observe for layout (reflow/paint) spikes.
📚 Branch Preview
🔍 Visual Regression Test Results
When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
- Spectrum | Light | Medium | LTR
- Spectrum | Dark | Large | RTL
- Express | Light | Medium | LTR
- Express | Dark | Large | RTL
- Spectrum-two | Light | Medium | LTR
- Spectrum-two | Dark | Large | RTL
- High Contrast Mode | Medium | LTR
Deployed to Azure Blob Storage: pr-5550
If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.
I'm having quite a lot of difficulty understanding, running, and fixing the tests, as well as a bit of difficulty understanding all the cases that this component handles (in my opinion it probably has too large of an API surface area and too many places storing the same state). I'm going to close this PR for now due to time constraints, but feel free to reopen if you're able to fix this performance issue.