fix(frontend/settings): sliders lag on mobile phone

[saihaj]

Summary of PR

Add debounce to settings onChange

Tests for unexpected behavior

Sliders on mobile work more smoothly and same experience on desktop

Time spent on PR

30 minutes

Linked issues

Fix #282

Reviewers

@Harjot1Singh @bhajneet

[saihaj]

Note: for display lines slider I don't think it can get more better than this because it depends on the step size (1 in this case). But for font size sliders it should be pretty smooth.

@bhajneet please test this to make sure that it is better than before.

[bhajneet]

If I'm being honest, when I get something for review, I expect there to be little to test/check. This experience is not the same as before in terms of visual responsiveness of the projector. Sure, maybe the slider is a little smoother. But we have sacrificed user input => visual update on projector response times. I don't see how that would make the user feel as though things are "faster". What we were aiming for was reducing the lag to slide on mobile without sacrificing anything on desktop.

[bhajneet]

@saihaj how much time did you take to test this overall and what specific things did you test for?

[bhajneet]

Having just tested again, I can confirm that slider smoothness isn't affected (still bad on mobile). Negatively, the response time for display to update has gotten worse. This PR has no perceivable benefit from my testing/review.

[saihaj]

I tested out on my desktop and for sure if we want to look smooth there is a little delay than before I can notice it because I played around with sliders in both versions. That is why I wanted someone else to test this and see If there is delay that is quite noticeable. I tired to keep the delay small enough and still making sure that it is not too noticeable.

[bhajneet]

If I were to read just the code, it would look like the only thing being throttled is the sending of the setting. So naturally there will be lag introduced to how long it takes for the setting to be applied to the presenter. That part I can understand.

However, there is no change in performance for dragging of sliders with or without this change.

So overall, I'm only seeing the downside. Perhaps you noticed an improvement in slider smoothness, but having played with the option all the way up to 500ms, I don't see any smoothness on mobile for font size. Perhaps there is simply too many steps for font size, perhaps the slider on mobile can be made wider by placing it below so it has more room to slide each step, I mean there are various options but unfortunately I don't see that this current implementation is an improvement

[saihaj]

@Harjot1Singh do you know if it is possible to modify sliders value function such that it feels smooth to slide and it is faster like before on projector?

Because with the 100ms delay right now we are slowing everting down. If we just do to our sliders.

Is it possible that we debounce all the websocket connections ? Then desktop user won’t be affected and mobile users will have that 100ms delay.

[Harjot1Singh]

The real delay actually comes from using controlled components (so, the value prop). Every time the setting changes, it re-renders the setting component itself. This is inline with React practice, however, in the cases where it is causing a negative effect, or that the re-rendering is too heavy (often the case for components that fast update, like sliders), you have 3 options:

1) Use defaultValue and flip the setting components over to uncontrolled components. This means that state changes do not reprender the component, and the value is actually just emitted, and not set. The downside is that any programmatic value changes or value changes sent via the network for this device (triggered by another user) will not be reflected in settings if settings is open at the same time (but the presenter should still update). There are a few ways around this, but this would be the tradeoff for now.

2) Reducing the possible number of updates by discretising/setting a stepper.

3) Reducing the number of updates sent via throttle. But now that I’ve thought through it, @bhajneet‘s experience makes sense, because the slider will appear to be less reactive, and since the slider is a controlled component, it’ll update with values at 100ms when you are dragging, which is not what we want.

The best solution is most likely throttled onChange + defaultValue (1 & 3)

[saihaj]

What should be implemented? @bhajneet @Harjot1Singh

[saihaj]

Converting this to draft since #282 is on hold

[bhajneet]

Closing this as there isn't any work committed for a draft pr