zulip icon indicating copy to clipboard operation
zulip copied to clipboard

Published 20 hours ago • verified publisher icon zulip

settings: `change email and password` dialog responsive on narrow screens

Open palashb01 opened this issue 2 years ago • 14 comments

  • Added a width for the Change Email and Password dialog at sm_min (576px) and ml_min (425px) to make it more responsive on narrow screens.

Fixes: #24339 CZO: Thread

Screenshots and screen captures:

Change Email:

  • Before: Screenshot from 2023-02-10 00-57-58

  • New Change:

change_email

Change Password:

  • Before: Screenshot from 2023-02-10 00-57-50

  • New Change:

change_password

Self-review checklist
  • [x] Self-reviewed the changes for clarity and maintainability (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

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

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

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

Completed manual review and testing of the following:

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

palashb01 avatar Feb 09 '23 19:02 palashb01

Hello @zulip/server-settings members, this pull request was labeled with the "area: settings (user)" label, so you may want to check it out!

zulipbot avatar Feb 09 '23 19:02 zulipbot

Hey @alya @amanagr , I have made the changes as discussed on CZO, could you please review the PR, Thanks.

palashb01 avatar Feb 09 '23 21:02 palashb01

LGTM

amanagr avatar Feb 14 '23 04:02 amanagr

Works for me in manual testing. @palashb01 While we're here, could you add a commit to change "Forgotten it?" -> "Forgot it?"; I think that'll be better.

alya avatar Feb 24 '23 20:02 alya

@alya , I have made the changes, could you please review the PR, thanks.

Screenshot from 2023-02-25 02-18-06

palashb01 avatar Feb 24 '23 20:02 palashb01

Thanks! The screenshot looks good, though I think we'll want a different prefix on the commit message for the commit you added.

@timabbott , this PR has been reviewed by @amanagr and me -- please take a look.

alya avatar Feb 24 '23 21:02 alya

Thanks! The screenshot looks good, though I think we'll want a different prefix on the commit message for the commit you added.

@timabbott , this PR has been reviewed by @amanagr and me -- please take a look.

Changed the commit message.

palashb01 avatar Feb 24 '23 22:02 palashb01

@palashb01 - For that second commit, you only need to change the string in the template file. The translations will be updated as part of the translation process; see https://zulip.readthedocs.io/en/latest/translating/internationalization.html.

laurynmm avatar Feb 27 '23 13:02 laurynmm

@alya - Also, would we also want to change/update the forgot password text in the modal for getting the user's API key? If so, we'd also need to update this documentation: https://zulip.com/api/api-keys.

API key modal

Screenshot from 2023-02-27 20-12-11

laurynmm avatar Feb 27 '23 19:02 laurynmm

@alya - Also, would we also want to change/update the forgot password text in the modal for getting the user's API key? If so, we'd also need to update this documentation: zulip.com/api/api-keys.

Good idea! Let me write that up as a separate issue, though -- I don't think it's important to do it at the same time.

alya avatar Feb 28 '23 00:02 alya

@palashb01 please post for @laurynmm 's review when you've updated the second commit.

@laurynmm please mark this PR for integration review when it looks good to you.

alya avatar Feb 28 '23 01:02 alya

@alya - Also, would we also want to change/update the forgot password text in the modal for getting the user's API key? If so, we'd also need to update this documentation: zulip.com/api/api-keys.

Good idea! Let me write that up as a separate issue, though -- I don't think it's important to do it at the same time.

Filed #24506 -- let me know if you have suggestions. It's a pretty hidden corner of the UI, so I don't think it's worth starting a #design thread.

alya avatar Feb 28 '23 01:02 alya

Hey @laurynmm , thank you for the reference. I didn't know about the translation process and thought all the files had to be changed. However, I have made the suggested changes. Could you please review the PR again? Thanks.

palashb01 avatar Feb 28 '23 04:02 palashb01

@laurynmm please mark this PR for integration review when it looks good to you.

Looks good and added the integration review label.

laurynmm avatar Feb 28 '23 11:02 laurynmm

The second commit removed a stray line of whitespace in the first commit; I fixed this and marked this to merge once CI passes.

(In the future, please be careful to check that each commit is correct whenever you update a PR).

timabbott avatar Mar 27 '23 16:03 timabbott