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

Fix AI Context menu text wrapping causing overlap #21438

Merged
merged 5 commits into from
Dec 3, 2024

Conversation

huacnlee
Copy link
Contributor

@huacnlee huacnlee commented Dec 3, 2024

Closes #20678 #21394

Before After
SCR-20241203-jreb SCR-20241203-jwhe

Release Notes:

  • Fixed AI Context menu text wrapping causing overlap.

Also cc #21409 @WeetHet @osiewicz to use Label, this PR has been fixed Label to ensure whitespace_nowrap when use single_line.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 3, 2024
@huacnlee
Copy link
Contributor Author

huacnlee commented Dec 3, 2024

image

@danilo-leal
Copy link
Contributor

danilo-leal commented Dec 3, 2024

Oh wow, I literally just opened a PR to solve the same issue like 30 minutes later without having known about yours 😄 That's crazy. Yours is much better!

I wonder about the Label's single_line property, though. I feel like, particularly given you're adding the tooltips to see the whole description, text_ellipsis would be just fine (instead of hard forcing it to a single line). I'd potentially consider leaving that addition/debate to a separate PR and keep this one simple.

@huacnlee
Copy link
Contributor Author

huacnlee commented Dec 3, 2024

Ok, I have removed the tooltip change.

@danilo-leal
Copy link
Contributor

danilo-leal commented Dec 3, 2024

The tooltip's great! It's a good solution to see the whole description if it's been cut by the overflow. I was referring to the need of having the single_line property in the Label component. That may be useful, I think, but not necessarily for this scenario. And maybe we could have its addition on a separate PR. I might've not communicated super well, apologies.

@huacnlee
Copy link
Contributor Author

huacnlee commented Dec 3, 2024

In this case if we not enable whitespace_nowrap for single_line, the issue will not to be fix

@danilo-leal
Copy link
Contributor

I pushed a little commit there adding text_ellipsis. That fixes the overflowing! And then, the tooltip helps have access to the description even if the text's overflown.

Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Thank you! 🙌

@danilo-leal danilo-leal merged commit a8c7e61 into zed-industries:main Dec 3, 2024
12 checks passed
@huacnlee huacnlee deleted the fix-label-single-line branch December 3, 2024 05:30
maxdeviant added a commit that referenced this pull request Dec 3, 2024
Release Notes:

- N/A

---

Split from #21438, this change for make sure the `single_line` mode
Label will not be wrap.

---------

Co-authored-by: Marshall Bowers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assistant slash command menu: descriptions can overflow
2 participants