-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fixed an issue where a crash would occur when applying an external update to list content while a live reorder event was occurring #513
Conversation
…date to list content while a live reorder event was occurring.
override func viewWillAppear(_ animated: Bool) { | ||
super.viewWillAppear(animated) | ||
|
||
Timer.scheduledTimer(withTimeInterval: 0.1, repeats: true) { [weak self] _ in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Remove this or add a start/stop button to the nav bar
@@ -11,7 +11,7 @@ import Foundation | |||
/// A queue used to synchronized and serialize changes made to the backing collection view, | |||
/// to work around either bugs or confusing behavior. | |||
/// | |||
/// ## Handling Re-ordering (`isQueuingForReorderEvent`) | |||
/// ## Handling Re-ordering (`isQueuingToApplyReorderEvent`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Update this comment
/// Set by consumers to enable and disable queueing during a reorder event. | ||
var isQueuingForReorderEvent : Bool = false { | ||
/// Set by consumers to enable and disable queueing when a reorder event is being applied. | ||
var isQueuingToApplyReorderEvent : Bool = false { | ||
didSet { | ||
self.runIfNeeded() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Double check we don't need this when a re-order event is abandoned by the user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for digging in on this, Kyle! Much appreciated.
Testing POS integration here: https://github.com/squareup/ios-register/pull/95745 And then will merge for this week's release if it's good. |
Ugh, either this or the first responder fix seems to have caused some sort of bug:
|
(Side note, I hate those damn |
This fixes another edge case in the reordering APIs in collection view wherein if the following occurs:
To work around this, we extend
ListChangesQueue
, to also queue changes if a live reorder event is occurring.The
hasUncommittedUpdates
API docs have some hints to this, albeit seemingly not quite correct:https://block.atlassian.net/browse/AP-26351
Checklist
Please do the following before merging:
Main
section.