Auto save changes in sharing modal

[MichaelBuessemeyer]

This PR automatically saves settings / content of a modal. Therefore it is no longer required to explicitly hit "Save". As already stated in #5074, this makes sense for the sharing modal. Another modal is not mentioned, and I could not find another one where it makes sense to me to save the current input automatically.

The original issue #5074 mentioned checking all modals to whether auto-saving is useful in that case. On a rough look, I could not find a modal that would benefit from auto-saving (search the codebase for all files having modal in their name).
If you can think of another modal, just tell me and I'll add it here.

URL of deployed dev instance (used for testing):

• https://___.webknossos.xyz

Steps to test:

• Open an explorative tracing
• Open the sharing modal via the drop-down menu in the navbar
• Update the sharing settings as you want (switch from private to public and set some teams and more)
• The changes in the modal should automatically be send to the backend and therefore automatically updated.

Issues:

• fixes #5074

---

(Please delete unneeded items, merge only when none are left open)

• Updated (unreleased) changelog
• Ready for review

[MichaelBuessemeyer]

I am unsure how to communicate to the user, that the settings were automatically updated 🤔. I tried a spinner and disabling the "ok" button but the update is just too fast for the spinner to be visible long enough.
My 2nd thought was to put a check mark ✅ or so behind or in front of the setting once it is updated 🤔. When the call failed another icon with a tooltip could be rendered 🤔.

[philippotto]

The code looks good, but I noticed a couple of things during testing:

• When the annotation is set to "internal", opening the modal will also immediately show a toast (only when opening the modal for the first time, as the modal is rendered lazily and won't be destroyed when closing it).
• There can only be one instance of the success toast. When switching fast between the settings, no new toast will appear (--> therefore, one could wonder if it was really saved). I suggest to decrease the time for how long the toast will be there and ensure somehow that the toast can be there multiple times (maybe assign a random key to the toast?).
• Switching to "Private" won't show a toast (I think the toast comes from updating the teams, but this isn't done when setting to "Private").
• There is a race condition when changing the visibility very fast. I somehow managed to get an error that the teams cannot be updated. Probably, the frontend thought that the annotation is "public" and tried to update the teams, but the back-end still had the annotation set to "private".

[MichaelBuessemeyer]

HI @philippotto,

I am currently working on your feedback.

There is a race condition when changing the visibility very fast. I somehow managed to get an error that the teams cannot be updated. Probably, the frontend thought that the annotation is "public" and tried to update the teams, but the back-end still had the annotation set to "private".

What would you suggest to circumvent this race condition? freeze / disable the modal until the changes are applied in the backend? I think this would only be necessary for the visibility setting and not for the team setting, correct? Changing selected teams should not lead to this race condition. Therefore I would suggest disabling the visibility setting while a backend update call is still in process 🤔.

What do you think?

[philippotto]

What would you suggest to circumvent this race condition? freeze / disable the modal until the changes are applied in the backend?

Yes, I think that's fair.

I think this would only be necessary for the visibility setting and not for the team setting, correct?

Could very well be, but I think it's easier to just always block the view to avoid any problems if these dependencies change in the future. In most cases, the blocking shouldn't even be noticeable to the user.

[MichaelBuessemeyer]

Could very well be, but I think it's easier to just always block the view to avoid any problems if these dependencies change in the future. In most cases, the blocking shouldn't even be noticeable to the user.

It is only slightly noticeable 👍.

[philippotto]

awesome, works great 👍 however, the share modal changed on the master branch (a new boolean setting was added), which needs to be incorporated now :)

[MichaelBuessemeyer]

@philippotto I just applied your feedback 🌮.

Thanks for informing me that another setting was added to the sharing modal on the master. This made it easier to understand how to resolve the merge conflicts.
Note that the current update to this pr also fixes a bug: Previously the old value of the visibility and the shared teams setting was used when sending updates about changes to the backend server. This was actually a but and I found this one by setting some settings, reloading, and then opening the sharing modal again. Prior to the fix, the settings were not what I left before reloading. But now this bug should be resolved and the settings should be correctly persisted. I checked this myself but please do so too to avoid merging bugs 🙈.

This pr is ready for your second review 🏁

[philippotto]

Note that the current update to this pr also fixes a bug: Previously the old value of the visibility and the shared teams setting was used when sending updates about changes to the backend server. This was actually a but and I found this one by setting some settings, reloading, and then opening the sharing modal again. Prior to the fix, the settings were not what I left before reloading. But now this bug should be resolved and the settings should be correctly persisted. I checked this myself but please do so too to avoid merging bugs .

Thanks for fixing this! Was the bug introduced recently (e.g, with the allow-contributors-to-modify change) or has it been like this for a while? Could you point to the line changes where you fixed this?

[MichaelBuessemeyer]

Thanks for fixing this! Was the bug introduced recently (e.g, with the allow-contributors-to-modify change) or has it been like this for a while? Could you point to the line changes where you fixed this?

Oh, maybe I misformualted my previous message. The truth is: I introduced this bug with this pr and noticed it only pretty late 😅. That's why I wanted to make sure that this bug does not make it to production by asking you to also specifically checking the fix.

[philippotto]

Nice! Seems to work great :) I noticed two minor things which should be changed:

• When a save requests fails (e.g., because the client is offline), the dialog is disabled completely, no error is shown and closing + reopening the dialog does not change anything. Effectively, the user has to reload the page to change the settings. An error message should be shown and maybe offer a retry possibility?
• the "Cancel" button is superfluous (and potentially irritating) now -> can be removed.

[MichaelBuessemeyer]

When a save requests fails (e.g., because the client is offline), the dialog is disabled completely, no error is shown and closing + reopening the dialog does not change anything. Effectively, the user has to reload the page to change the settings. An error message should be shown and maybe offer a retry possibility?

Thanks for finding this. I now ensure that upon a failed update request an error is shown to the user, the UI resets its state prior to the failure so that it matches what's currently saved in the backend and it is always reenabled. For additional information, the error is logged to the console.

the "Cancel" button is superfluous (and potentially irritating) now -> can be removed.

👍 Done 🍡

[philippotto]

Excellent 💯