zulip icon indicating copy to clipboard operation
zulip copied to clipboard

left_sidebar: Show tooltip only for truncated channel names.

Open userAdityaa opened this issue 1 year ago • 9 comments

This commit replaces the default HTML title attribute with a Tippy.js tooltip for stream names in the subscription block, ensuring that tooltips are shown only when the name is truncated.

Fixes #31807

Before(Big Channel name) After(Big Channel name)
Screenshot 2024-10-04 at 5 50 45 AM Screenshot 2024-10-03 at 5 31 09 AM
Before(Small Channel name) After(Small Channel name)
Screenshot 2024-10-03 at 5 29 42 AM Screenshot 2024-10-04 at 5 47 08 AM

This tooltip only shows when the stream name is big such that it will get truncated.

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).

  • [ ] Each commit is a coherent idea.
  • [ ] 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.

userAdityaa avatar Oct 03 '24 00:10 userAdityaa

Hello @zulip/design members, this pull request was labeled with the "redesign" label, so you may want to check it out!

zulipbot avatar Oct 03 '24 00:10 zulipbot

Your screenshots should also demonstrate that there's no tooltip for a short channel name. Thanks!

alya avatar Oct 03 '24 22:10 alya

Hey @alya, Ready for review.

userAdityaa avatar Oct 04 '24 00:10 userAdityaa

Please updated your commit message: "streams" are called "channels" now, and you're missing a period.

alya avatar Oct 04 '24 16:10 alya

Ready for review!

userAdityaa avatar Oct 04 '24 18:10 userAdityaa

@sayamsamal can you review this one? I haven't tested.

alya avatar Oct 04 '24 19:10 alya

Also, the prefix of the PR title could be more concise, and it shows the area on which you have worked on. Some thing like left_sidebar or tooltips would be appropriate over here.

sayamsamal avatar Oct 07 '24 13:10 sayamsamal

Hey, How is this test failing just after changing the variable name.

userAdityaa avatar Oct 07 '24 21:10 userAdityaa

@userAdityaa if you take a look at https://github.com/zulip/zulip/actions/runs/11439296839/job/31822666704?pr=32075#step:16:1181, where I ran the Puppeteer tests to confirm the problem you were facing, you'll notice that the error is with the selector: .stream-name[title="Verona"] since you are removing the title attribute from the element.

sayamsamal avatar Oct 21 '24 12:10 sayamsamal

Thanks for the help @sayamsamal.

userAdityaa avatar Oct 21 '24 21:10 userAdityaa

@sayamsamal, Can you please check this once now ?

userAdityaa avatar Oct 29 '24 14:10 userAdityaa

I also don't see any comments replying to https://github.com/zulip/zulip/pull/31851#discussion_r1811103470.

sayamsamal avatar Nov 04 '24 15:11 sayamsamal

I also don't see any comments replying to #31851 (comment).

I forgot to reply to it. Sorry, my mistake.

userAdityaa avatar Nov 05 '24 20:11 userAdityaa

Hey @sayamsamal, I'm confused with the error in the test failure. Can you please help me with this ?

userAdityaa avatar Nov 06 '24 23:11 userAdityaa

There's a known puppeteer test failure in main; see chat.zulip.org for details.

timabbott avatar Nov 07 '24 01:11 timabbott

Other than the one comment, the PR LGTM. I'll mark it for integration once you remove the irrelevant whitespace changes.

sayamsamal avatar Nov 25 '24 11:11 sayamsamal

Closing, as this needs to be reworked to address https://github.com/zulip/zulip/pull/31851#discussion_r1857009686.

timabbott avatar Nov 25 '24 17:11 timabbott