Feat: Introduce NavigationItem subcomponent and showcase Header with Navigation and Dropdown
#1840
Conversation
crishpeen
requested review from
literat,
pavelklibani,
curdaj,
a team and
adamkudrna
as code owners
✅ Deploy Preview for spirit-design-system-storybook ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for spirit-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| align-items: center; | ||
| } | ||
|
|
||
| .NavigationItem--stretchChildren, |
There was a problem hiding this comment.
It's just the AlignmentYExtended dictionary alone, isn't it? 🙂 Now it looks a bit strange to me like this, but I think we will be able to get rid of this modifier once we abstract the alignment classes (and people will be able to use also other alignments here if needed).
| $link-typography: tokens.$body-small-semibold; | ||
| $link-gap: tokens.$space-600; | ||
| $link-spacing: tokens.$space-600; | ||
| $link-color-state-default: tokens.$component-header-item-state-default; | ||
| $link-color-state-hover: tokens.$component-header-item-state-hover; |
There was a problem hiding this comment.
Shouldn't these all start with action instead of link?
| It centers its children vertically if the children are not NavigationAction components or | ||
| it does not have the `.NavigationItem--stretchChildren` class. |
There was a problem hiding this comment.
I would turn this around so it's more comprehensible (hopefully): If there is a NavigationAction inside or the XY modifier is present, it stretches its content vertically. In all other cases, the content is vertically centered.
| display: flex; | ||
| align-items: stretch; |
There was a problem hiding this comment.
Is it needed? It works without it for me.
| @@ -29,7 +29,9 @@ export type NavigationActionProps<E extends ElementType> = { | |||
| elementType?: E; | |||
| } & NavigationActionBaseProps; | |||
|
|
|||
| export interface SpiritNavigationItemProps extends ChildrenProps, StyleProps {} | |||
| export interface SpiritNavigationItemProps extends ChildrenProps, StyleProps { | |||
| isStretchingChildren?: boolean; | |||
There was a problem hiding this comment.
🕺 🙈
hasChildrenStretched / hasStretchedChildren Also not great…
| @@ -43,11 +43,27 @@ import { NavigationItem } from '@lmc-eu/spirit-web-react'; | |||
| <NavigationItem>{/* Navigation actions go here */}</NavigationItem>; | |||
| ``` | |||
|
|
|||
| If the children are not `NavigationAction` components or the `NavigationItem` does not have the | |||
| `isStretchingChildren` class, it will centre its children vertically. | |||
There was a problem hiding this comment.
| `isStretchingChildren` class, it will centre its children vertically. | |
| `isStretchingChildren` prop, it will centre its children vertically. |
Otherwise better than in web 🙂.
| <NavigationItem> | ||
| <ButtonLink href="#" color="secondary"> | ||
| Sign up | ||
| Log out |
Description
Additional context
Issue reference