Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

settings: Add a saving-saved indicator to the 'SETTINGS/TOPICS' UI. #26337

Merged
merged 1 commit into from Aug 6, 2023

Conversation

prakhar1144
Copy link
Member

#26016 (comment)

I think the settings "topics" UI is missing a loading...saved indicator interaction to show you that your changes are saved; right now you get no feedback at all that your change works. But that feels like something that'd be OK as an immediate follow-up.


This commit adds a 'saving...' - 'saved' indicator to the 'SETTINGS/TOPICS' UI.

This improves the UX by reflecting that the changes are saved.


Screenshots and screen captures:

screen-capture.webm
Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

This commit adds a 'saving...' - 'saved' indicator to the
'SETTINGS/TOPICS' UI.

This improves the UX by reflecting that the changes are saved.
@zulipbot zulipbot added size: M buddy review GSoC buddy review needed. mentor review GSoC mentor review needed. labels Jul 24, 2023
Copy link
Member

@abhijeetbodas2001 abhijeetbodas2001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the followup! Tested this out manually -- works great.
One question I have is, while we are waiting for the success response (and when we are showing the loading indicator), should we disable the dropdown selector? For example, see the below video, which seems like a weird experience, if you try to change the policy a second time before the first request is completed. (I introduced artificial latency by putting time.sleep(5) in the server code). It might be possible that this edge case is too rare and not worth working on, so I'm not sure.
Screencast from 2023-07-24 22-54-00.webm

@abhijeetbodas2001
Copy link
Member

Actually we have the same "problem" (if we can call it a problem that is) in other settings panes as well. So probably this edge case is rare enough that we can ignore it :)

@prakhar1144
Copy link
Member Author

@abhijeetbodas2001
I think since we never received any reports of such a weird experience, there are very, very rare chances.
But even if we plan to fix that, I guess doing it as a separate mini-project would be better. (Fixing it at all other places)

@prakhar1144

This comment was marked as off-topic.

@zulipbot zulipbot added the integration review Added by maintainers when a PR may be ready for integration. label Jul 28, 2023
@timabbott timabbott merged commit 3d5d434 into zulip:main Aug 6, 2023
8 checks passed
@timabbott
Copy link
Sponsor Member

This is great, though I'm a bit worried that this looks a bit misaligned vertically when next to the ? button; maybe a good topic to bring up in #frontend and figure out if there's something cleaner we could be doing.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buddy review GSoC buddy review needed. integration review Added by maintainers when a PR may be ready for integration. mentor review GSoC mentor review needed. size: M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants