Conversation
- Fixed jest setup to work with lodash-es using babel
…ion of useGetBoundingClientRect and introduce layoutAdapter for improved layout measurement
🦋 Changeset detectedLatest commit: ddd7217 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
predikament
added
enhancement
|
Hey @xavi160, one concern here. Several internal calls (smartNavigate, setFocus, updateLayout) became async but aren't awaited by their callers (key event handler, addFocusable, recursive calls in smartNavigate). |
…oid unnecessary layout updates
xavi160
dismissed stale reviews from guilleccc, asgvard, predikament, SMishra25, and AdriVelazquezNorigin
via
0ce0ba9
| private async tick(): Promise<void> { | ||
| await this.currentTask?.(); | ||
| if (this.nextPriorityTasks.length > 0) { | ||
| this.currentTask = () => | ||
| Promise.all(this.nextPriorityTasks.map((task) => task())); | ||
| this.nextPriorityTasks = []; | ||
| await this.tick(); | ||
| } else { | ||
| this.currentTask = this.nextTask; | ||
| this.nextTask = undefined; | ||
| await this.tick(); | ||
| } | ||
| } |
There was a problem hiding this comment.
If this.currentTask becomes undefined then we'll probably end up in an infinite loop.
I think we need to wrap await this.tick(); on line 25 with an if (this.currentTask) guard.
| (component) => | ||
| component.parentFocusKey === parentFocusKey && | ||
| component.focusable && | ||
| component.layoutUpdatedAt > threshold |
There was a problem hiding this comment.
This should probably be:
| component.layoutUpdatedAt > threshold | |
| component.layoutUpdatedAt <= threshold |
Otherwise we are doing it backwards, updating layout for fresh elements and skipping the stale.
|
|
||
| ```typescript | ||
| setFocus(focusKey: string, focusDetails?: FocusDetails): void | ||
| setFocus(focusKey: string, focusDetails?: FocusDetails): Promise<void> |
There was a problem hiding this comment.
setFocus is still sync.
| this.schedulePriority(async () => fn.bind(context)(...args)); | ||
| } | ||
|
|
||
| schedule(task: Task) { |
There was a problem hiding this comment.
My bad, I'll fix it
7 participants
Motivation
In other platforms (like
react-native) node measurement is async . This PR does the prep work to support that.Changes
layoutAdapteroptiondefaultLayoutAdapterandgetBoundingClientRectAdapteruseGetBoundingClientRectoption