-
Notifications
You must be signed in to change notification settings - Fork 12
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
menu: improve menu generation management #335
Conversation
690f757
to
62613ed
Compare
projects/rero/ng-core/src/lib/menu/menu-widget/menu-widget.component.html
Outdated
Show resolved
Hide resolved
projects/rero/ng-core/src/lib/menu/menu-widget/menu-widget.component.html
Outdated
Show resolved
Hide resolved
projects/rero/ng-core/src/lib/menu/menu-widget/menu-widget.component.html
Outdated
Show resolved
Hide resolved
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.
Commit message proposition:
menu: improve menu generation management
Improves the menu generation management by adding some specifications on
menu entries:
* Allows to set disabled state on any menu entry.
* Allows to separate entries with a divider.
* Removes some CSS indentation from the menu component.
Removes the 'PrefixSuffixComponent' and replaces it with a simple
template.
62613ed
to
9e8882d
Compare
<li class="nav-item dropdown" dropdown placement="bottom right"> | ||
<a class="nav-link dropdown-toggle" | ||
dropdownToggle | ||
[routerLink]="" |
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.
Why empty routerLink
?
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.
Without this, the "a" tag isn't recognize as a valid link by browser and the cursor stay as pointer (not the little hand).
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.
and with href="#"
?
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.
I think simply adding href
alone works.
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.
Commit message approved.
<li class="nav-item dropdown" dropdown placement="bottom right"> | ||
<a class="nav-link dropdown-toggle" | ||
dropdownToggle | ||
[routerLink]="" |
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.
I think simply adding href
alone works.
Improves the menu generation management by adding some specifications on menu entries: * Allows to set disabled state on any menu entry. * Allows to separate entries with a divider. * Removes some CSS indentation from the menu component. Removes the 'PrefixSuffixComponent' and replaces it with a simple template. Co-Authored-by: Renaud Michotte <renaud.michotte@gmail.com>
9e8882d
to
5ea96fe
Compare
Improves the menu generation management. Adding some specifications on
menu entries :
Removes the 'PrefixSuffixComponent' and replaces it with a simple
template.
Co-Authored-by: Renaud Michotte renaud.michotte@gmail.com
How to test?
npm run serve
Code review check list