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

Notify changes synchronously if possible #60

Merged
merged 1 commit into from Sep 21, 2022
Merged

Conversation

helios175
Copy link
Collaborator

We don't want to use the mainScope.launch { calculateNotifications ; then update } path if the updates are synchronous (for instance, when only extra item is updated, or when going from empty to not-empty).

This PR changes:

  • Update.generateDataChangesLambdas is not suspend anymore. It won't just produce a list of notifications for the UI thread, but an UpdateWork comprising async work and notifications.
  • Recycler.update will receive that and decide to do the sync path (no async work) or the coroutine path (async work).
  • As both paths need to assign the new recycler data, notify onReady and assign the adapter that is encapsulated in applyNotifications and called from both places.
  • The async path still checks that currentUpdate is not changed, and call onCancelled if it changed (some other call to update invalidated us).

@helios175 helios175 changed the title Notify changes synchornously if possible Notify changes synchronously if possible Sep 14, 2022
Copy link
Member

@pyricau pyricau left a comment

Choose a reason for hiding this comment

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

Ship it!

@pyricau
Copy link
Member

pyricau commented Sep 21, 2022

(one might wonder why no tests were updated in this change, but I'm going to pretend like this thought didn't occur to me)

@helios175 helios175 marked this pull request as ready for review September 21, 2022 22:37
@helios175 helios175 merged commit dc9f816 into main Sep 21, 2022
@helios175 helios175 deleted the helios/syncifpossible branch September 21, 2022 22:38
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.

None yet

2 participants