Jarring animation when topmost Q is marked as answered

[jonhoo]

I observed this during a stream, not sure if it's reproducible.

[ghassanachi]

Just tried my hand at this (First time working with svelte).

I believe its related to:

1. https://stackoverflow.com/questions/61730267/reactive-statements-triggered-too-often (Rich Harris weighed in on this)
2. Broken reactivity when dealing with bindings and exported objects sveltejs/svelte#5363

I still don't have a good enough grasp of the Svelte internals to really know what is going on, but I did some debugging/testing and found out that the event prop that was being passed into List and Question was causing a reactive update that was calling loadQuestions even though the event prop was unchanged. To fix this I decided to try putting the event property in a store and explicitly subscribing to the changes which resolved the issue (just using $event in loadQuestions did not work, though I am not sure why).

Good news is that after the changes, not only did it fix this issue but it also resolved #7 and #1.

I'll open a PR for the changes sometime tomorrow (I setup lsp on neovim and the formatter went to town on all of the files I was working on, so too keep the PR on the smaller side I'll reapply the changes without the formatting)

[jonhoo]

Hmm, interesting. We do intentionally re-assign event (to the same value) to trigger a re-load of questions, here for example:

wewerewondering/client/src/List.svelte

Line 58 in b748d1d

interval = setTimeout(() => {event = event;}, next);

So we just need to make sure that we preserve that behavior (or get it some other way).

[ghassanachi]

I've been working on this bug for a little while now (I thought I had a fix, but it wasn't perfect so still working on it)

The reassignment is not an issue with the solution proposed above, since we can use the set function on the store to re-trigger reloads (I've tested this and it is working). The main difference is that it gives us a little more fine grained control on when we want to re-trigger loads. As far as I can tell the issue with the $: notation is that in some instances (dealing with objects) svelte makes assumptions about reassignments even though they may not have happened. Having the value in a store allows us to choose more carefully when we want the updates/reloads to happen.

But as I mentioned the changes did not solve the issue. After a little more digging I'm fairly certain it has to do with calls being made to adjustQuestions (from the polling) while the animation is still rendering. Changing the poll interval to a larger value removes all the of the jitter. With that in mind, one fix that comes to mind would be to debounce the adjustQuestions call, but the problem is that certain actions should take priority over others (toggling a prop vs poll refresh for example). I'm still trying to come up with a solution and I'll update this issue as soon as I think I have a fix

[jonhoo]

That's interesting — that kind of race should be fairly rare given the frequency of updates though 🤔

[ghassanachi]

On the host view the Polling is happening every 3000ms and the animation time is 500ms, so the overlap can happen in the following 2 scenarios:

1. If we send a toggle request during the first 500ms after the poll request we'll overlap with the ongoing animation from the pool
2. If we send a toggle request in the last 500ms before the poll request, the poll request will overlap with our toggle animation

Assuming my assumptions are correct, there is a 1/3 (1000ms/3000ms) chance of animation overlap.

Additionally I think on production (hasn't been happening locally for me), the client is sending a loadQuestions request right after a properly is toggled without respecting the the 3000ms delay. If that's the case then we would be entering condition 2. everytime a property is toggled

[jonhoo]

Ohh, I see, that's helpful. I wonder if the way to do this is with a very very simple debounce. Instead of having the view depend directly on adjustedQuestions, do something like:

rawQuestions -> adjustQuestions -> renderQuestions

where the function that assigns the output of adjustQuestions into renderQuestions does so onTick only if the two differ and there isn't currently an active animation. And that second one I think you could implement just as

animating = true;
setTimeout(500, () -> animating = false)

when a property is changed. May need a bit more trickery (like a clearTimeout call) to handle if two properties are changed in rapid succession. Unless there is a DOM property that already tracks "whether there's an active animation", in which case we could just use that.

I think we could also reduce the animation time a little bit — 500ms feels on the long side.

[ghassanachi]

I actually tested something similar to what you described, it was working well except for the snappiness of toggling a property. This is only an issue in scenario 1. since in the other case it takes priority. But if we reduce the animation time it should be less noticable.

I was really hoping Svelte would provide a on:animationend event similar to the transition events, but no luck so far.

My initial implementation relied on a setTimeout like your suggestion, but I'll check the DOM nodes to see if there is a property (like class) we can monitor instead.

[ghassanachi]

Sidenote: To try and simplify the data flow for setting questions:

rawQuestions -> adjustQuestions -> renderQuestions

I was thinking of using a derived store We could make rawQuestions a store as well and then questions would become:

let prev;
const questions = derived([rawQuestions, localAdjustments, votedFor], ([$rf, $la, $vf], set,) => { 
    const updated = adjustQuestions($rf, $la, $vf);
    if (!deepEqual(updated, prev)) { set(updated) }
    prev = updated;
});

PS: writing code on a phone is tough :)

[jonhoo]

A derived store feels like it makes a lot of sense!