search

visualize-admin / visualization-tool

The tool for visualizing Swiss Open Government Data. Project ownership: Federal Office for the Environment FOEN
https://visualize.admin.ch
BSD 3-Clause "New" or "Revised" License
29 stars 3 forks source link

feat: Add Save button to annotations panel #1488

Closed bprusinowski closed 2 months ago

bprusinowski commented 2 months ago

Closes #1427

This PR adds a "Save" button to the annotations panel. As we save the changes dynamically anyway, it acts as a "close panel" button.

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
visualization-tool ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 6, 2024 11:14am
bprusinowski commented 2 months ago

@KerstinFaye I see that in the current design file we also have a Cancel button, contrary to what's inside the #1427 issue 😱

As we have a "live mode editing" across the whole application, where changes are visible on a chart as soon as we make them (except for some data loading related actions, like updating multi-filters where we need to confirm the selection to avoid constant data changes and loading), I wonder if we should change this paradigm for annotations – because if I understand correctly, in the current design, user should be able to update title and description without immediate feedback on a chart, but only applied after clicking "Save", and discarded when clicking "Cancel")? Or maybe they should be immediately visible, but discarded when going back to main menu or clicking "Cancel"?

This is a personal preference, but I find it more user-friendly to have this live saving as we do currently, just wanted to check with you if we should change this (and if so, should we also apply it in other parts of the application)? If you think it's a good idea to change this, maybe then we could also have a small tag above, like "Pending changes", so we know if we actually made a change? I think it might be a bit confusing now, as we can save the changes immediately after entering the panel, without actually updating the title or description yet 👀

Let me know what you think about this :)

ptbrowne commented 2 months ago

Code looks good but I am wondering about the behavior here. Here I wonder, there was no modification to how changing a field changes the chart, so Bartosz if I am not mistaken, here in this PR, even if the user does not click "Save", it will have been saved already right ? I completely agree with Bartosz, the addition of a Save button feels like a change of paradigm. What is exactly the intention there ?

bprusinowski commented 2 months ago

@ptbrowne yes, it's a "fake" Save button now, as I didn't want to completely change the behavior before discussing this 👍

KerstinFaye commented 2 months ago

@bprusinowski Thanks a lot for the feedback. I agree, it's definitely better to have the live feedback. I reviewed the implemented version and I think the solution to have the save button but still the live feedback is best.

KerstinFaye commented 2 months ago

@ptbrowne I think this ideas is even based on your feedback that you gave ones in a document that you did for UI improvements. I thought it was a good idea to give the user the opportunity to confirm their action to approve the title input. But if you are all agreeing that it's a change of paradigm then we can also leave it like we had. (--> https://github.com/visualize-admin/visualization-tool/issues/1332). I just thought that "save" as a text might be more clear than "ok".

bprusinowski commented 2 months ago

Thanks for the review @KerstinFaye! From my side it would be fine to leave the current implementation from this PR, let me know what you think @ptbrowne 👍

ptbrowne commented 2 months ago

I think this ideas is even based on your feedback that you gave ones in a document that you did for UI

Maybe, I have tried to think more about it, and I think my feedback was to answer to the need of the user "after inputting a title, what do I do now ". I think we want to

I think this PR solution with the "save" button solves (2) but could be confusing to the user, and somehow does not respect (1). So maybe a good solution would be to have the text of the button only "OK".

KerstinFaye commented 2 months ago

I think this PR solution with the "save" button solves (2) but could be confusing to the user, and somehow does not respect (1). So maybe a good solution would be to have the text of the button only "OK".

That works. Let's use "ok".

ptbrowne commented 2 months ago

Thanks 👍