feat(styles): btp horizontal nav update #4997
Conversation
✅ Deploy Preview for fundamental-styles ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
N1XUS
force-pushed
the
feat/btp-horizontal-nav-update
branch
from
acbae73 to
59edf5e
Compare
N1XUS
changed the title
| @@ -378,7 +400,7 @@ $navListContainerSubmenu: #{$block}__list-container--submenu; | |||
| } | |||
| } | |||
|
|
|||
| .#{$navItemWithExpander} { | |||
| @include _fd-vertical-nav-with-expander() { | |||
There was a problem hiding this comment.
how is this a better approach?
There was a problem hiding this comment.
@InnaAtanasova previous line would apply styling for both horizontal and vertical navs. This mixin allows to apply styling only for a vertical nav. Same with _fd-horizontal-nav-with-expander mixin.
There's something wrong with the naming. The default one is where the whole item serves as an expander. |
|
The underline for selected state is not at the correct position. There's 1-2px space under it. It's more noticeable in focus state: 
|
N1XUS
force-pushed
the
feat/btp-horizontal-nav-update
branch
from
1c5f075 to
04bd9c3
Compare
This is still not fixed. There's 1 px spacing under the selected indicator. |
|
Please fix the underline spacing. The code looks great so I will approve the PR to not block you tomorrow. |
Related Issue
Relates SAP/fundamental-ngx#10670
Description
Screenshots
After:
Please check whether the PR fulfills the following requirements
remfd-*class is used in the filefd-rtl,fd-ellipsis,fd-flex,fd-selected,fd-focus, ect.)fd-reset()mixin is applied to all elementsnormalizeoptionunnormalizeoption[ci visual]so it can trigger chromatic visual regression (e.g.test: run chromatic visual regression [ci visual])