Feat/navbar vertical next flat submenu#2065
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mobile-specific "flat submenu" feature for the navbar-vertical-next component. When the viewport is at or below the small breakpoint (576px), triggering a group now opens a dedicated submenu view with a back button instead of using flyouts or inline expansion. The changes include the new SiNavbarFlatSubmenuComponent, updated layout logic to hide other elements when the submenu is active, and comprehensive E2E and unit tests. Feedback focuses on improving the robustness of label retrieval by targeting specific title elements and ensuring the new component adheres to project standards by using ChangeDetectionStrategy.OnPush and its corresponding import.
panch1739
left a comment
panch1739
left a comment
There was a problem hiding this comment.
@mistrykaran91 Awesomeee!!
Just some notes, in the drill down version, for the title node, we should use text-primary. Right now is using text-secondary:
In the design, the flyout menu is using base-3, could we do the same? I know this is a change from the current implementation, so if is too breaking, we can skip that and do it later:
Thanks!
| readonly flyout = signal(false); | ||
|
|
||
| /** @internal `true` when this group is currently rendered inside the mobile flat submenu. */ | ||
| readonly flatSubmenuActive = computed( |
There was a problem hiding this comment.
I don't like the naming, can we have something shorter e.g. flatMode ?
| styleUrl: './si-navbar-flat-submenu.component.scss', | ||
| changeDetection: ChangeDetectionStrategy.OnPush | ||
| }) | ||
| export class SiNavbarFlatSubmenuComponent implements OnDestroy { |
spliffone
left a comment
spliffone
left a comment
There was a problem hiding this comment.
- When the user press on a group and the flat the next level become active -> the focus is lost but should be on the first item
- When a group is active and the sub items are visible - toggle the collapse expand mode loose the current selection and the user starts from the top navbar - we need to fix it
- I can't see a animation / transition between group click and visible sub items
- Are we still limited to one sub-level or can we build navigation for 3, 4 or more levels?
mistrykaran91
force-pushed
the
feat/add-navbar-vertical-next-flyout-behavior
branch
6 times, most recently
from
23b8b01 to
1c08adc
Compare
mistrykaran91
force-pushed
the
feat/navbar-vertical-next-flat-submenu
branch
from
fbb60eb to
af1ed61
Compare
mistrykaran91
force-pushed
the
feat/navbar-vertical-next-flat-submenu
branch
from
0f34a41 to
3efdc69
Compare
- On mobile viewports (≤576px) with the navbar expanded, triggering a group now opens its sub-items as a flat submenu that replaces the inline list Related #1944
mistrykaran91
force-pushed
the
feat/navbar-vertical-next-flat-submenu
branch
from
b5f1dff to
e26e176
Compare
…ar Vertical Next example
3 participants
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: