fix(dropdown): fix some bugs introduce by the previous commit (#1473)

[ko2in]

Description

This PR fixes the first level of simple dropdown inside the right menu was cropped and out of the screen when the parent menu item has short text, and the dropdown doesn't specify the class name left.

To avoid this, all dropdown inside the right menu will be forced to appear leftward by default. The dropdown can also appear leftward or rightward by having left or right class name for it's nested dropdown menus.

This PR also fix the dropdown inside the buttons which are positioned incorrectly.

Testcase

Broken

https://jsfiddle.net/18jogrs9/
https://jsfiddle.net/usfh3tpw/1/

Fixed

https://jsfiddle.net/ymo0f9st/

Closes

#1502
#1514
#1520

[ko2in]

I updated the left and right margin of sub menu .ui.dropdown .menu .menu within one rule, since the sub menu are absolute positioned element, and it's safe to declare the negative margin for both left and right together for leftward or rightward direction without affecting on the neighbor element.

Previously in dropdown.variables: @subMenuMargin: 0 0 0 @subMenuDistanceAway;. And now: @subMenuMargin: 0 @subMenuDistanceAway;.

I don't think it's necessary to specify individually for leftward and rightward.

And I also would like to suggest removing the declaration important from the following from menu.less.

/* Left Floated */
.ui.menu:not(.vertical) .left.item,
.ui.menu:not(.vertical) .left.menu {
  display: flex;
  margin-right: auto !important;  
}
/* Right Floated */
.ui.menu:not(.vertical) .right.item,
.ui.menu:not(.vertical) .right.menu {
  display: flex;
  margin-left: auto !important;
}

This especially effects on the simple dropdown in the right menu, that need to specify to appear rightward direction with the class name right menu, and the negative margin for the left side is overruled by the above declarations, and which gives the slightly different position for the rightward dropdown in the right menu.

With !important

Without !important

I'm not sure how much it can effect and break the backward compatibility. By testing with this fiddle (https://jsfiddle.net/ymo0f9st/1/) doesn't break anything and works as expected.

[lubber-de]

LGTM😃 Thank you for the quick fix! 👍
Although your suggested removement for !important seems to work fine (in your jsfiddle)

• this selector is also used for items.
• There are also other margin settings using !important in stackable.menu for a left/right/center item in menu.less, so it might has impact there.

So, either you leave the !importants in (for now / this PR) or you check all other !important settings for any right/left/center/ menu margin within menu.less and remove (and test) them as well

I basically would love to see the !importants getting removed, but there are lots of situations to be tested, so we might postpone the !important removement into a later PR to get this one getting merged asap

That said, i approve this PR for now as it is 🙂

[ko2in]

@lubber-de Thanks for the details. Then, I'll test with the other modules around and if it doesn't break anything, I'll send a PR for this.

[hakeem86]

The previous version did not fix the left submenu on iOS, worked fine on other os/browsers.

Is this fix tested on iOS?

[lubber-de]

Is this fix tested on iOS?

The above mentioned fixed jsfiddle runs fine as expected on iPad Pro, iOS 13, Safari

[ceisele-r]

@ko2in is your fix going to be released, soon? The menu out of screen bug prevents us from updating to the latest version.

[lubber-de]

@ceisele-r v2.8.6 (which includes this fix) is supposed to be released tomorrow 🙂
The current master branch was already updated last night

[ceisele-r]

Great, thanks @lubber-de

[lubber-de]

@ceisele-r 2.8.6 has been released just now 🎉 We hear your voices 😄

[ceisele-r]

@lubber-de thank you very much!