Feat: Navigation Item component #15
Conversation
There was a problem hiding this comment.
I left couple of comments, most of it don't need change (are just observations)
I didn't go deep into controller logic as I prefer to merge this PR fast and do incremental changes, first thing as we agreed to add more CSS variables.
Apply the changes you may think are needed and fell free to merge.
docs/assets/scripts/figma-button.js
Outdated
| @@ -0,0 +1,24 @@ | |||
| window.addEventListener('load', function (event) { | |||
There was a problem hiding this comment.
I removed figma button as we need server side logic and the portal is static content only
docs/_includes/default.njk
Outdated
| @@ -53,6 +53,7 @@ | |||
| <script src="{{ assetUrl('scripts/code-previews.js') }}" defer></script> | |||
| <script src="{{ assetUrl('scripts/lunr.js') }}" defer></script> | |||
| <script src="{{ assetUrl('scripts/search.js') }}" defer></script> | |||
| <script src="{{ assetUrl('scripts/figma-button.js') }}" defer></script> | |||
There was a problem hiding this comment.
I removed figma button as we need server side logic and the portal is static content only
docs/_includes/default.njk
Outdated
| @@ -80,6 +81,11 @@ | |||
| <sl-icon name="twitter"></sl-icon> | |||
| </a> | |||
|
|
|||
| {# Figma #} | |||
docs/pages/components/mf-nav-item.md
Outdated
|
|
||
| ### Prefix & Suffix | ||
|
|
||
| Add content to the start and end of nav items using the `prefix` and `suffix` slots. |
There was a problem hiding this comment.
Do we need this?
I would just add content before/after using this component
There was a problem hiding this comment.
following the same structure and behavior of Shoelace, adding it within CSS will limit the usage of the Icons lib.
docs/pages/components/mf-nav-item.md
Outdated
| </mf-navigation> | ||
| ``` | ||
|
|
||
| ### Highlight |
There was a problem hiding this comment.
I would use active as it's the most common word use for this
There was a problem hiding this comment.
as discussed highlight have a different logic than active.
|
|
||
| export default css` | ||
| :host { | ||
| --subnav-offset: -2px; |
There was a problem hiding this comment.
this to align the subnav with the parent while mitigating the space from borders.
| cursor: not-allowed; | ||
| } | ||
|
|
||
| .nav-item.nav-item--highlight { |
| line-height: var(--sl-line-height-normal); | ||
| letter-spacing: var(--sl-letter-spacing-normal); | ||
| color: var(--sl-color-neutral-700); | ||
| padding: var(--sl-spacing-2x-small) var(--sl-spacing-2x-small); |
There was a problem hiding this comment.
let's use this for naming convention:
.nav-item {
/* ... */
padding: var(--nav-spacing-inset-stretch, --nav-spacing-inset) var(--nav-spacing-inset-squish, --nav-spacing-inset);
margin-right: var(--nav-spacing-inline);
margin-bottom: var(--nav-spacing-stack);
}
.nav-item:dir(rtl) {
margin-left: var(--nav-spacing-inline);
margin-right: 0px !important;
}
<img width="756" alt="image" src="https://github.com/user-attachments/assets/e6d1aff4-550d-4635-9311-6c4580818692">| @@ -0,0 +1,161 @@ | |||
| import { css } from 'lit'; | |||
There was a problem hiding this comment.
I understand we need to add lot of custom variables at component level, I just added example for nav padding and margin following the space naming conventions I shared before
| describe('<mf-nav-item>', () => { | ||
| it('should render a component', async () => { | ||
| const el = await fixture(html` <mf-nav-item></mf-nav-item> `); | ||
|
|
||
| expect(el).to.exist; | ||
| }); | ||
| }); |
added new Nav Item Component.
modified NAV Component.
Updated autoloader.