-
Notifications
You must be signed in to change notification settings - Fork 991
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
Fixes #36350 - Add line breaks to long bookmarks #9758
Conversation
Issues: #36350 |
Could we do it a bit differently? I mean, if there are multiple long names then they will just take a lot of vertical space. So nothing is changed, but the direction of wasted space. Maybe we could try using ellipsis here? Just crop the line after few characters and put the rest into tooltip. |
I think the ellipsis was used before we started with any bookmark changes and the user feedback was, that they need to see the whole bookmark name. Seeing just the beginning does not help too much. I think this is a good compromise, it's still readable (not extremely long line) and contains the whole name. Hoes does it look like, when there's 100 bookmarks like this (I think the reporting user had around 100 of them), is it scrollable? If so, I'm 👍 |
I agree, that's why I suggested tooltip with the rest (or the full name even). But since users want to deal with scrolling, who am I to be against that 😶 |
> | ||
<EllipisWithTooltip>{name}</EllipisWithTooltip> | ||
const bookmarksList = ({ bookmarks, onBookmarkClick }) => { | ||
const hasLongerName = bookmarks.some(bookmark => bookmark.name.length > 90); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you do this check also for errorItem
?
</EllipisWithTooltip> | ||
<DropdownItem | ||
ouiaId="error-dropdown-item" | ||
className={`bookmarks-dropdown-item ${errors > 90 ? 'expanded' : ''}`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be errors.length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please change the class name expanded
to something more specific to the bookmarks? just in case there would be another rule from somewhere for expanded
classnames
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good, Thanks!
The dropdown was stretching with long names and it was overflowing the page too much:
Now the dropdown shrinks and the words break if it is too long so that it is nicely visible:
This is a quick resolution that could be redone in the future.