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

[Account] Prevent account info overflowing on kebab menu #4555

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

bharatkashyap
Copy link
Member

@bharatkashyap bharatkashyap commented Dec 24, 2024

Screenshot 2025-01-08 at 7 14 25 PM

@bharatkashyap bharatkashyap added the bug 🐛 Something doesn't work label Dec 24, 2024
Copy link
Member

@apedroferreira apedroferreira left a comment

Choose a reason for hiding this comment

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

Added just one comment, I guess at least having an ellipsis even when there's no options button should be doable for consistency?

variant="body2"
fontWeight="bolder"
// Only truncate when a longer text causes the text to overflow over the MoreIconButton
maxWidth={handleClick ? 180 : 'unset'}
Copy link
Member

@apedroferreira apedroferreira Jan 6, 2025

Choose a reason for hiding this comment

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

Looks good, but is there no way to make it overflow with the available width without us having to set a manual value?
I feel like it should be possible but haven't tried it.

Also what is the behavior if there is no options button? Does it truncate with an ellipsis after the sidebar width? I guess that's what it should do in every case... (just tested and looks like it doesn't so I guess it could)

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea to only truncate when there is an options button is to allow the popover to have the complete information, so something like:

Screen.Recording.2025-01-07.at.3.53.23.PM.mov

We could truncate in all cases, but then do you suggest including a Tooltip when the text is inside the popover; something like:

Screen.Recording.2025-01-07.at.4.00.53.PM.mov

Copy link
Member

@apedroferreira apedroferreira Jan 7, 2025

Choose a reason for hiding this comment

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

I meant that if you don't pass a handleClick to AccountPreview and so there's no options button to show a popover, this is what it looks like right now:

Screenshot 2025-01-07 at 10 44 02

So maybe all these overflows should ellipsize automatically? Probably not a huge deal though, it was just something I noticed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah got it! Yes the ellipsis should happen in this case as well, good catch

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 8, 2025
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 8, 2025
@mui-bot
Copy link

mui-bot commented Jan 8, 2025

Netlify deploy preview

https://deploy-preview-4555--mui-toolpad-docs.netlify.app/

Generated by 🚫 dangerJS against 854677a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Larger text hides the options button in AccountPreview
3 participants