-
-
Notifications
You must be signed in to change notification settings - Fork 334
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(dropdown): fix menu item text breaking issue #1838
Conversation
When the menu item contains too long text, the text doesn't break into the new line when there's no enought space to display in single line and the text overflows the container. This PR fixes the menu item text breaking problem and also changes the container to flex box to render the image, text and the dropdown icon as flex box items to align better.
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.
This unfortunately breaks 2 usecases i found
- Floating Content https://fomantic-ui.com/modules/dropdown.html#floated-content
- Short menu items (dropdown icon for submenu not to the very right anymore if other menu items are longer)
Testcase
Normal
https://jsfiddle.net/lubber/9yu8wr3v/12/
Your PR
https://jsfiddle.net/lubber/9yu8wr3v/14/
Screenshots
Normal
Your PR
@lubber-de I've fixed the floated icon issues. Please review again. I've updated the fiddle link (New link: https://jsfiddle.net/ko2in/3sw01eLz/7/) in my description. There is also a left floated icon which has the right margin |
@lubber-de I found a minor issue with icon alignment and description left-margin and fixed it. Here's the fiddle link (https://jsfiddle.net/ko2in/3sw01eLz/9/). You can compare with the previous version (https://jsfiddle.net/ko2in/3sw01eLz/7/). |
Wow, this looks much better, i am still testing, but it seems you also fixed #1071 by 99%. 👍 For local IE11 testing here is a standalone file |
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.
Basically LGTM 👍
Very good job!
Please change the little specifity as shown and remove the !important and use a doubled .ui
instead (for both right floated
selectors)
margin-left: @floatedDistance !important; | ||
order: 1; | ||
padding-left: @floatedDistance; | ||
margin-left: auto !important; |
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.
Please remove the !important
and add another .ui
to the selectors instead to increase specificity
@lubber-de Please hold for the PR for a moment. I also find some problems with the upward direction menu. I'll look into it and also your issue above and update the PR. |
@lubber-de I did find out the solution for the upward direction menu. But, changing to the flexbox model reflects also some parts which I'm trying to fix. The vertical sidebar menu and accordion inside the menu are also affected by this change. I've fixed almost every issue I've found. The icon issue you mentioned here doesn't relate to my PR. But, the icon positions are also affected too especially the left and right position and their margins which I'm trying to fix now. The margin inside the flexbox is really a challenge though. I'll submit the updated PR this weekend if I didn't find anything else with my changes. |
@ko2in If you still have issues to solve, please convert this PR into Draft |
…and float is no longer applicable
… child elements for their positions and alignments
to the dropdown item text since the icon display next to the text when sub menu opens to the right direction as default.
to shift down to get better alignment along with the text and dropdown icon. In previous version, the remove icon placed a few pixels above from the text and dropdown icon which causes the inappropriate alignment.
doesn't support float in it's child elements. Specify the order of label to display after the text.
the lowest order number should be specified.
incorrect alignment with the elements around when it contains the content which takes more space vertically such as an image. Instead, use `align-item` property on the text container to have everything inside of it in a linear position.
causes the sub-menu positioned incorrectly when it's inside floating dropdown item.
@lubber-de I've fixed the conflicts with the base branch and merge my PR. I've tested with all menu examples and various menu instances. Here's my fiddle. Here is the summary about some additional fixes from my last commit. Remove icon alignmentThe remove icon wasn't aligned in the best position as it was shifted above comparing the text and dropdown icon. Sub-menu position in floating menu itemThe sub-menu of dropdown item was not positioned correctly when the dropdown has Incorrect rounded corners and shadows of simple dropdown with upward directionThe rounded corners of upward simple dropdown menu should be top-left and top-right and the shadow should be displayed above the menu. That is fixed in this commit. |
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.
Please revert the dist folder changes. They should not be part of the PR
These might have received when merging develop branch into my branch. 🤔 |
Try to rebase, because if the develop branch would have been merged there would not be any changes |
@lubber-de I've done rebasing, but I am not able to push to this branch anymore. So, I created another PR. I'm closing this PR. |
Description
When the menu item contains too long text, the text doesn't break into the new line when there's no enough space to display in
single line and the text overflows the container.
Even though the text can be forced to break using
break-word: break-all
, it doesn't look good enough when there is an image representing next to the menu text.This PR fixes the menu item text breaking problem and also changes the container to flexbox to render the image, text, and the dropdown icon as flexbox items to align better.
Since the menu item has changed to the flexbox model and the dropdown icon and direct children elements of a menu item are appearing as flex items and the float property has no effect on flexbox, and so the float rules and variables for the dropdown icon are removed by this PR.
Testcase
Bug: https://jsfiddle.net/ko2in/3sw01eLz/1/
Fixed: https://jsfiddle.net/ko2in/3sw01eLz/9/
Screenshot
Before:

After:

Closes
#1820