zulip
zulip copied to clipboard
left_sidebar: Show tooltip only for truncated channel names.
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) |
|---|---|
| Before(Small Channel name) | After(Small Channel name) |
|---|---|
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.
Hello @zulip/design members, this pull request was labeled with the "redesign" label, so you may want to check it out!
Your screenshots should also demonstrate that there's no tooltip for a short channel name. Thanks!
Hey @alya, Ready for review.
Please updated your commit message: "streams" are called "channels" now, and you're missing a period.
Ready for review!
@sayamsamal can you review this one? I haven't tested.
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.
Hey, How is this test failing just after changing the variable name.
@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.
Thanks for the help @sayamsamal.
@sayamsamal, Can you please check this once now ?
I also don't see any comments replying to https://github.com/zulip/zulip/pull/31851#discussion_r1811103470.
I also don't see any comments replying to #31851 (comment).
I forgot to reply to it. Sorry, my mistake.
Hey @sayamsamal, I'm confused with the error in the test failure. Can you please help me with this ?
There's a known puppeteer test failure in main; see chat.zulip.org for details.
Other than the one comment, the PR LGTM. I'll mark it for integration once you remove the irrelevant whitespace changes.
Closing, as this needs to be reworked to address https://github.com/zulip/zulip/pull/31851#discussion_r1857009686.