fix(Scroll to Bottom): Feature corrupting visible queue contents #1990
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This fixes an issue where using Scroll to Bottom on the queue page made a bunch of clones of my queued posts appear. This is, obviously, pretty bad. I didn't test what happens if you e.g. delete one, thinking you have it queued twice, but yeah. My guess is this is caused by sortablejs (as I only observe it on queue), likely a race condition when the load-more-posts code gets called at an unexpected time.
Quick test:
Object.groupBy([...document.querySelectorAll('[data-id]')].map(element => element.dataset.id), id => id)Putting our programmatic scroll in an rAF seems to fix this. Fortunately, two doesn't seem to be needed; that would result in an extra render occurring each time we scroll, which would be slower (likely making #1793 worse). One, I would guess, just shifts the scroll and resulting observer callbacks to later in the same synchronous event loop iteration, potentially changing the amount of js code that's run but not really affecting the render loop.
I was going to make this predicated on
${keyToCss('timeline')} > .sortableContainer, but it doesn't seem to really slow down or have ill effects on the feature in general, so perhaps better safe than sorry?Testing steps
I think this one's honestly more of a "can you think of an error-state-causing race condition that could occur with this based on looking at it." I think I can't?