Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implement OnyxNavItem #874

Closed
13 tasks done
Tracked by #1010
MajaZarkova opened this issue Apr 11, 2024 · 8 comments
Closed
13 tasks done
Tracked by #1010

Implement OnyxNavItem #874

MajaZarkova opened this issue Apr 11, 2024 · 8 comments
Assignees
@MajaZarkova
Labels
dev Requires technical expertise
Milestone
Navigation bars

Comments

@MajaZarkova
Copy link
Contributor

MajaZarkova commented Apr 11, 2024 •
edited by jannick-ux
Loading

Open Questions/To-dos

  • When we implemented the OnyxBadge we set the dot as out of scope. We don't have a decision whether the dot should be a separate component or part of OnyxBadge. The dot should be used inside OnyxNavItem, do we set it as out of scope here as well?

Implement the dot as a slot, so the user have a possibility to use a dot or a badge

Relates to #841

Why?

This component is needed and will be used inside the navigationBar. This is just a support component

Design

Figma Link

Acceptance criteria

  • Supports additional badge alongside the text
  • Supports two modes:
    • Opens the linked page on click
    • Opens a flyout with multiple pages on hover

Definition of Done

  • covered by component tests
  • screenshot tests are updated
  • follow-up tickets were created if necessary
  • updated version + documentation is deployed
  • Storybook can show the feature
  • Set task to In Approval

Approval

Out of scope

Reference implementations

  • Suggestion to use the OnyxListbox as the flyout mentioned before
  • CoreUI Reference

Applicable ARIA Pattern

ARIA Pattern
@MajaZarkova MajaZarkova added this to onyx Apr 11, 2024
@MajaZarkova MajaZarkova converted this from a draft issue Apr 11, 2024
@MajaZarkova MajaZarkova moved this from New to Backlog in onyx Apr 11, 2024
@MajaZarkova MajaZarkova self-assigned this Apr 11, 2024
@mj-hof mj-hof added this to the Navigation bars milestone Apr 11, 2024
@mj-hof mj-hof added the dev Requires technical expertise label Apr 11, 2024
@MajaZarkova MajaZarkova mentioned this issue Apr 11, 2024
Closed
18 tasks
@mj-hof mj-hof moved this from Backlog to Ready in onyx Apr 17, 2024
@MajaZarkova MajaZarkova self-assigned this Apr 18, 2024
@MajaZarkova MajaZarkova moved this from Ready to In Progress in onyx Apr 18, 2024
@MajaZarkova
Copy link
Contributor Author

MajaZarkova commented Apr 19, 2024 •
edited
Loading

Notes for Monday daily

  • Is onyx marking the parent active when a child is active? -> Yes
  • Can parent be also clicklable if it has children? -> No, it should only be hoverable
  • Can a navItem be disabled? -> No
  • If all the children are disabled is the parent disabled? -> No
  • Do we have special treatment for external links, eg. arrow icon (future questions) -> Jannick will align Figma (add arrow icon)

@MajaZarkova
Copy link
Contributor Author

#447

This was referenced Apr 22, 2024
Open
Merged
Closed
larsrickert pushed a commit that referenced this issue Apr 24, 2024
@MajaZarkova
feat(OnyxNavItem): Implement basic OnyxNavItem (#992)
afe16cf 
Relates to #874 

Implement basic OnyxNavItem behavior.
@MajaZarkova MajaZarkova moved this from In Progress to In Approval in onyx Apr 24, 2024
@jannick-ux
Copy link
Collaborator

Before approving: please check thehover state of the selected flyout-item. Currently it has the same optics like a non-selected-item. But it should look different.
https://www.figma.com/file/YfEUBOHk4J4nYrk04geswG/onyx---Design-System-Figma-Library?type=design&node-id=1124-18839&mode=design&t=VeOiYlOSoYy7HVE5-4

The goal looks like this: same background color like the non-hovered selected item but with primary colored text.

@larsrickert
Copy link
Collaborator

@MajaZarkova Ideally when pressing tab, it should directly focus the listbox instead if having to press tab two times.
I need the same behavior for my user fly out and I think @JoCa96 already implemented this in his combox PR

@MajaZarkova
Copy link
Contributor Author

@larsrickert , good point. I'll implement it.
@jannick-ux , thank you for finding the bug.

I'll create a PR fixing both issues :)

@MajaZarkova MajaZarkova mentioned this issue Apr 25, 2024
Merged
2 tasks
@MajaZarkova MajaZarkova moved this from In Approval to In Progress in onyx Apr 25, 2024
@larsrickert
Copy link
Collaborator

@MajaZarkova Currently the options are always visible / taking up height which does not work as intended when used inside the nav bar because the whole nav bar adjusts its height then.

We propably should position it absolute to the parent

Image

MajaZarkova added a commit that referenced this issue May 7, 2024
@MajaZarkova @github-actions
fix(OnyxListbox): Fix hover colors on OnyxListbox and OnyxNavItem (#1019
93ad5a9 
)

Relates to #874 

Fix hover colors on OnyxListbox and OnyxNavItem

## Checklist

- [x] If a new component is added, at least one [Playwright screenshot
test](https://github.com/SchwarzIT/onyx/actions/workflows/playwright-screenshots.yml)
is added
- [x] A changeset is added with `npx changeset add` if your changes
should be released as npm package (because they affect the library
usage)

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@MajaZarkova MajaZarkova moved this from In Progress to In Approval in onyx May 7, 2024
@MajaZarkova
Copy link
Contributor Author

@larsrickert , follow up bug ticket is created -> #1043

@jannick-ux
Copy link
Collaborator

Please have a look on the text styles of the component in default and in active state.
Default state uses "paragraph-default", active state uses "h3". So in the end, when the item is active, the font gets more weight. Rest looks nice to me

@MajaZarkova MajaZarkova mentioned this issue May 7, 2024
Merged
2 tasks
MajaZarkova added a commit that referenced this issue May 7, 2024
@MajaZarkova @github-actions
fix(OnyxNavItem): Fix OnyxNavItem font-weight (#1044)
1eb0528 
Relates to #874 

Fix OnyxNavItem font-weight for active state

## Checklist

- [x] If a new component is added, at least one [Playwright screenshot
test](https://github.com/SchwarzIT/onyx/actions/workflows/playwright-screenshots.yml)
is added
- [x] A changeset is added with `npx changeset add` if your changes
should be released as npm package (because they affect the library
usage)

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-project-automation github-project-automation bot moved this from In Approval to Done in onyx May 7, 2024
@JoCa96 JoCa96 mentioned this issue May 8, 2024
Closed
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MajaZarkova MajaZarkova

Labels
dev Requires technical expertise
Projects
onyx
Status: Done
Milestone
Navigation bars
Development

No branches or pull requests
4 participants
@MajaZarkova @larsrickert @mj-hof @jannick-ux