feat(url_hash_binding) added setting to rate limit url state updates

[chrisj]

I think this feature would be better with the one to save to session storage on beforeunload (history.replaceState does not work in beforeunload on chrome). If the user sets a long rate limit, they can easily lose recent changes with a reload.

The blur event works pretty well but mousemove works better in preventing this issue where only part of the url is selected.

Unfortunately both of them fail to update if I use the shortcut to select the url (ctrl+L/cmd+L). I'll see if there is anything can be done. It does trigger the blur event but I can't get it to change the url.

Update: I added the keydown event to update the url which works with the select url shortcut.

A value of 0 for rate limit currently means unrestricted. We may want to add a checkbox to disable it completely, otherwise a very high value is effective for the performance issues that have been experienced.

Before merging this, it would be good to get some thorough testing to make sure there are no regressions but it would be good to first verify the state changes are satisfactory.

[seankmartin]

I think this might trigger the blur event as well so not sure if both listening for the blur event and listening for the ctrl+l/cmd+l press are needed? I see from your PR message though that this seems to be needed, since the URL doesn't update despite the event triggering. Maybe can add a comment in the code about that?

[chrisj]

Thanks, I added more detail there. I think we can probably remove the blur event, I need to test it on more browsers

[seankmartin]

Hey @chrisj this seems a really nice feature, thanks!

The main curiosity / comment I have is should this be a setting that is configurable as part of the frontend and stored in the JSON state, or should it instead be like how the viewer configuration is working and be something you could change on the viewer or at build time, but not something exposed in the UI or state. I guess you might have some use cases in mind where controlling this from the frontend and saving it in the state is very helpful. But I wonder if maybe this is something that the application using neuroglancer or the neuroglancer deployment wants to control instead of the user?

I was also wondering about disabling the hash updates completely outside of the times flush() is called. This is just piggybacking on your suggestion to have a checkbox to turn off the automatic time based hash updates completely. I like this idea, and I think supporting it would be clearer than setting the update rate to a very high value to achieve a similar goal.

As a complete aside, I just wanted to mention that the mouseleave on the document is pretty handy also for frames around neuroglancer, and that's very nice!

[chrisj]

@seankmartin I think this is something we sometimes want users to decide, especially because sometimes there are users that don't have control/knowledge to create their own deployment but they are important users and very engaged. They might have a specific workflow that needs particular performance modifications.

I don't think there is an easy answer. I would like performance settings to be able to be split off from the regular state, but since it can be workflow dependent (huge states), sometimes it does make sense. I think it is easy for users to load a state without realizing it has non default performance settings. Maybe having a color indicator could be useful though it might cause more problems than solve.

[seankmartin]

Thanks Chris for the context, could there be a hybrid system similar to what @fcollman is doing in #831 to combine viewer attributes with state information?

For really large states, if it causes performance issues updating the hash rapidly, then maybe neuroglancer could automatically change the hash update rate based on some heuristic about the state size. Then it wouldn't be required for the user to know about this setting and manually set a higher rate limit update rate to avoid these kind of problems. So the application would control that rate limit update by default, but since it should support users being able to update that rate in the frontend manually then they could still do it and override the automatic setting by neuroglancer.

[chrisj]

Closing in favor of auto rate limiting