Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add public ViewManagerModel.viewSelectionObserver #3824

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

amcclain
Copy link
Member

  • For use by apps to mask components during view select/rebuild.

Hoist P/R Checklist

Pull request authors: Review and check off the below. Items that do not apply can also be
checked off to indicate they have been considered. If unclear if a step is relevant, please leave
unchecked and note in comments.

  • Caught up with develop branch as of last change.
  • Added CHANGELOG entry, or determined not required.
  • Reviewed for breaking changes, added breaking-change label + CHANGELOG if so.
  • Updated doc comments / prop-types, or determined not required.
  • Reviewed and tested on Mobile, or determined not required.
  • Created Toolbox branch / PR, or determined not required.

If your change is still a WIP, please use the "Create draft pull request" option in the split
button below to indicate it is not ready yet for a final review.

Pull request reviewers: when merging this P/R, please consider using a squash commit to
collapse multiple intermediate commits into a single commit representing the overall feature
change. This helps keep the commit log clean and easy to scan across releases. PRs containing a
single commit should be rebased when possible.

@amcclain amcclain requested a review from ghsolomon November 13, 2024 18:15
@amcclain
Copy link
Member Author

Testing out our new stuff in an actual app that manages a fair amount of state via viewManager - on switch there's a noticeable lag/pause while the components are all given their new state. Exposing this task observer allowed me to put a mask in place, which I think works well to let the user know what's happening.

+ For use by apps to mask components during view select/rebuild.
@amcclain amcclain force-pushed the viewSelectionObserver branch from ba2e4fe to ee58044 Compare November 15, 2024 19:12

this.setValue(this.selectedView?.value ?? ({} as T));
// Introduce minimal wait and link to viewSelectionObserver to allow apps to mask.
await wait(100)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ghsolomon rightly pointed out that this random 100 value is unfortunate. I re-tested, and I found that I was unable to get the mask to reliably render in my app when swapping views unless I left this "high" value in place. await wait() and await wait(1) did not do it.

(The app test case involve some heavyweight re-filtering of a large recordset + dashboard layout changes, but nothing that extreme - definitely a "real world" case.)

We discussed how ideally we could use some utility that perhaps keyed off of animation frames or otherwise was able to better detect that rendering had "settled down" - for want of a more technical term...

@amcclain amcclain merged commit 2fa1ae0 into develop Nov 15, 2024
2 checks passed
@amcclain amcclain deleted the viewSelectionObserver branch November 15, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant