zulip icon indicating copy to clipboard operation
zulip copied to clipboard

user_profile: Align privacy icons with stream names in popover.

Open Aditya8840 opened this issue 1 year ago • 14 comments

Description:

This PR resolves the misalignment issue between privacy icons and stream names in the stream list. The icons were not properly aligned with the text, causing a visual discrepancy.

Changes: - Set the display property of the .stream-privacy class to flex.

Fixes: #31521.


UI Changes:

Before: Screenshot 2024-09-05 at 2 36 27 AM

After: Screenshot 2024-09-05 at 2 36 27 AM


Self-review checklist
  • [x] 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).

  • [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.
  • [ ] Strings and tooltips.
  • [ ] End-to-end functionality of buttons, interactions and flows.
  • [ ] Corner cases, error conditions, and easily imagined bugs.

Aditya8840 avatar Sep 04 '24 21:09 Aditya8840

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

zulipbot avatar Sep 04 '24 21:09 zulipbot

Ready for review! 🚀

Aditya8840 avatar Sep 04 '24 21:09 Aditya8840

@zulipbot add "buddy review"

Aditya8840 avatar Sep 05 '24 14:09 Aditya8840

I have made the changes according to your feedback. Please review the updates and let me know if there’s anything else I can improve.

Aditya8840 avatar Sep 06 '24 09:09 Aditya8840

Should I mark this for integration?

Aditya8840 avatar Sep 09 '24 19:09 Aditya8840

Each commit should pass tests.

alya avatar Sep 10 '24 05:09 alya

Done !!

Aditya8840 avatar Sep 10 '24 08:09 Aditya8840

Commit messages look good (though I would reserve the Fixes: for the commit that actually includes the fix), and works and looks good in manual testing.

karlstolley avatar Sep 13 '24 19:09 karlstolley

Done !!

Aditya8840 avatar Sep 14 '24 06:09 Aditya8840

The alignment doesn't look right in the screenshot in the PR description -- channel names are too high. Does the screenshot need to be updated?

alya avatar Sep 16 '24 19:09 alya

Updated !!

Aditya8840 avatar Sep 17 '24 03:09 Aditya8840

The text still looks too high to me in the "after" screenshots.

alya avatar Sep 17 '24 15:09 alya

Thanks for the feedback. I will review the positioning again to ensure it aligns correctly, considering the previous changes.

Aditya8840 avatar Sep 17 '24 17:09 Aditya8840

@zulipbot remove "buddy review"

Aditya8840 avatar Sep 17 '24 18:09 Aditya8840

@Aditya8840 are you still working on this? It's marked as a draft, and I don't see a top-level comment stating what this is waiting on.

timabbott avatar Nov 19 '24 21:11 timabbott

Heads up @Aditya8840, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

zulipbot avatar Dec 04 '24 20:12 zulipbot

Closing in favor of https://github.com/zulip/zulip/pull/33512. @Aditya8840 you might find the final version useful to read, since I caught several mistakes in that PR, some of which are common to this one.

timabbott avatar Mar 05 '25 01:03 timabbott