refactor: use requestAnimationFrame to replace readTask
#10432
Conversation
github-actions
bot
added
the
refactor
jcfranco
added
the
skip visual snapshots
| import { whenTransitionDone } from "./dom"; | ||
|
|
||
| /** | ||
| * Exported for testing purposes only | ||
| */ | ||
| export const internalReadTask = readTask; | ||
| export const internalRaf = requestAnimationFrame; |
There was a problem hiding this comment.
I'll try to remove this before installing.
There was a problem hiding this comment.
Yeah seems like this just needs to be cleaned up to just use requestAnimationFrame by itself.
| jest.spyOn(openCloseComponent, "internalReadTask").mockImplementation((task) => task(1337)); | ||
| jest.spyOn(openCloseComponent, "internalRaf").mockImplementation((task) => { | ||
| task(1337); | ||
| return 1337; |
| import { whenTransitionDone } from "./dom"; | ||
|
|
||
| /** | ||
| * Exported for testing purposes only | ||
| */ | ||
| export const internalReadTask = readTask; | ||
| export const internalRaf = requestAnimationFrame; |
There was a problem hiding this comment.
Yeah seems like this just needs to be cleaned up to just use requestAnimationFrame by itself.
jcfranco
deleted the
jcfranco/10348-swap-out-readTask-for-requestAnimationFrame
branch
|
I think this change may have broken our Still working through the details. |
|
@nel11211 Let us know if you are able to narrow it down to this. |
Ok I think I see what's happening. @kumarGayu Is a dynamic click necessary for the function that selects a tab? |
|
@nel11211 Thanks for sharing your findings! If I’m reading this right, this doesn’t seem to be related to this PR. Would you mind moving the conversation to a more appropriate place? |
|
Opened an issue in Analysis repo if we want to move discussion there: https://devtopia.esri.com/WebGIS/arcgis-web-analysis/issues/7297 |
It's not the root cause no, but worth noting for some reason the issue wasn't happening until the change from this PR. |
* origin/dev: (230 commits) chore: release next chore(sort-handle): add messages (#10474) feat(accordion-item): stretch slotted actions to fill its height (#9250) chore: release next feat(dialog, modal, popover, input-date-picker, input-time-picker, sheet): support stacked component sequential closing with escape (#9231) chore: remove commented-out code (#10478) chore: add cssrem VSCode extension recommendation (#10300) docs(accordion-item): fix deprecation tag (#10479) chore: release next feat(stepper-item): update component's active state background color. (#10475) refactor: use `requestAnimationFrame` to replace `readTask` (#10432) chore: release next fix(tip): fix rendering tied to named-slot content (#10470) ci: compile estimate totals per milestone (#10442) chore: release next fix(modal): fix rendering tied to named-slot content (#10469) chore: release next fix(shell-center-row): fix rendering tied to named-slot content (#10451) fix(inline-editable): fix rendering tied to default slot content (#10456) fix(input, input-number, input-text): should not set slotted actions to be disabled (#10458) ...
Related Issue: #10348
Summary
We've been using
readTaskin a few places to ensure proper timing for DOM reads w/o triggering layout thrashing. Looking at the implementation, we might be able to get close or similar behavior by usingrequestAnimationFrame.