fomantic / Fomantic-UI

Fomantic-UI is the official community fork of Semantic-UI
https://fomantic-ui.com
MIT License
3.58k stars 333 forks source link

[Sticky] Sticky menu lost rounded corners #426

Open ko2in opened 5 years ago

ko2in commented 5 years ago

Bug Report

The sticky menu lost rounded corners when it's fixing in the page.

Steps to reproduce

  1. Add vertical menu in the page
  2. Invoke the sticky plugin to the menu
  3. Scroll the page until the menu is positioning fixed in the page

Expected result

The menu should keep it's rounded corners as before.

Actual result

The menu lost it's rounded corners.

Testcase

http://jsfiddle.net/mrf2jznq/

Version

2.7.1

lubber-de commented 5 years ago

:thinking: Not as easy as it looks. Removing the

.ui.menu.fixed,
.ui.menu.fixed .item:first-child,
.ui.menu.fixed .item:last-child {
    border-radius: 0!important;
}

only fixes it in this special case, but the border-radius:0 has to be kept for usual fixed menu behavior (without using the sticky module). I think we need to add another class to the sticky element at the moment when it gets fixed by the sticky module to have an indication for CSS to handle this, so it becomes something like

.ui.menu.fixed,
.ui.menu.fixed:not(.stickynow) .item:first-child,
.ui.menu.fixed:not(.stickynow) .item:last-child {
    border-radius: 0!important;
}

But this would also mean we have to check every other possible sticky element in css aswell

ko2in commented 5 years ago

On this documentation page, the sticky element has the class name sticky. I can see in my chrome inspector. But, why the sticky menu in my fiddle doesn't have that class name?

Otherwise, this issue would be solved as you proposed above, without any extra class, but just sticky class.

.ui.menu.fixed:not(.sticky),
.ui.menu.fixed:not(.sticky) .item:first-child,
.ui.menu.fixed:not(.sticky) .item:last-child {
    border-radius: 0!important;
}
lubber-de commented 5 years ago

The sticky from the docs page is set manually, not by the framework at runtime, so you cannot rely on that. I still think we have to add that class at runtime to accomplish this.