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.

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

Implement mobile nav bar #1073

Closed
14 tasks done
larsrickert opened this issue May 13, 2024 · 4 comments
Closed
14 tasks done

Implement mobile nav bar #1073

larsrickert opened this issue May 13, 2024 · 4 comments
Assignees
Labels
dev Requires technical expertise

Comments

@larsrickert
Copy link
Collaborator

larsrickert commented May 13, 2024

Open Questions/To-dos

  • @mj-hof in Figma the context area is collapsed in a 3 dot menu but they are still some icons (notification/search) displayed in the nav bar itself. Does this make sense? The context menu content is up to the project so can not really differentiate here, should we just move the whole context are to the 3 dot menu?
    • create new slot (something like "contextAreaMobile") that will remain in the nav bar even on mobile
    • content of contentArea slot will move to the 3 dot menu on mobile
  • @mj-hof Is the logo not visible at all on mobile? Answer:
    • also show logo (so we can just reuse the OnyxNavAppArea component) if its given by the project

Depends on

Why?

The nav bar should be responsive and adapt on mobile breakpoints.

Design

Figma link.

Acceptance criteria

  • the design is implemented
  • when reaching xs breakpoint or smaller, the nav bar should switch to the mobile layout
  • when burger menu is open: app area (app name + logo is shown instead of active page name)
  • if a page is active and burger menu is closed, the active page name is displayed, otherwise the app name
  • the context area collapses into a "3 dot menu"
  • the nav items collapse into a burger menu

Definition of Done

  • covered by component tests
  • screenshot tests are updated
  • updated version + documentation is deployed
  • Storybook can show the feature
  • Storybook code snippet of new/changed examples are checked that they are generated correctly
  • Set task to In Approval

Approval

Implementation details

  • if possible prefer a CSS implementation
  • use container queries instead of media queries so the component behaves correctly also when e.g. displayed inside Storybook (and use our container query utils)
  • prevent cloning DOM nodes

Applicable ARIA Pattern

Menu button

@larsrickert larsrickert added this to onyx May 13, 2024
@larsrickert larsrickert converted this from a draft issue May 13, 2024
@larsrickert larsrickert added the dev Requires technical expertise label May 13, 2024
@larsrickert larsrickert added this to the Navigation bars milestone May 13, 2024
@larsrickert larsrickert moved this from New to Backlog in onyx May 13, 2024
@larsrickert larsrickert moved this from Backlog to New in onyx May 13, 2024
larsrickert added a commit that referenced this issue May 14, 2024
Relates to #867

- Add basic navigation bar component without mobile behaviour
- add navigation bar to alpha test app

Note on the "2xs" screenshots: For now we accept the "broken" visuals
until we implement the mobile behaviour of the nav bar in #1073
@larsrickert larsrickert moved this from New to Backlog in onyx May 29, 2024
@mj-hof
Copy link
Collaborator

mj-hof commented May 29, 2024

Reconsider the behaviour of the "App icon + App name" etc with @jannick-ux and the agreement of UXUI department

@mj-hof mj-hof moved this from Backlog to New in onyx May 29, 2024
@mj-hof mj-hof moved this from New to Ready in onyx May 29, 2024
@larsrickert larsrickert self-assigned this Jun 4, 2024
@larsrickert larsrickert moved this from Ready to In Progress in onyx Jun 4, 2024
larsrickert added a commit that referenced this issue Jun 14, 2024
Relates to #1073

- implement basic mobile nav bar
- fix selected styles for nested OnyxNavItem options
larsrickert added a commit that referenced this issue Jun 14, 2024
Relates to #1073 

Fix error when using the nav bar with Nuxt / server side rendering
larsrickert added a commit that referenced this issue Jun 21, 2024
Relates to #1073

Support showing the nav items inside the `OnyxNavBar` when in mobile
mode
larsrickert added a commit that referenced this issue Jun 26, 2024
Relates to #1073

- implement support for mobile context area inside the nav bar
@larsrickert larsrickert moved this from In Progress to In Approval in onyx Jun 26, 2024
@BoppLi
Copy link
Contributor

BoppLi commented Jul 1, 2024

Approval feedback @larsrickert

  1. could you create a follow up ticket for smoother open/close animations (and link it here), if it doesn't exist already?
  2. when testing it in the alpha app, I noticed that the mobile menu does not close after navigating to another page. is this to be done on "project side", if so, could you add this to the alpha app's nav bar? or is this a feature we should take care of, e.g. with an mobileOpen prop?
  3. in the Storybook link you created above, the example does not fire any events when clicking on nav items. is this intentional / out of scope?

@larsrickert
Copy link
Collaborator Author

Approval feedback @larsrickert

  1. I think that should be covered by Concept animations #54 in general
  2. Since the navigation is done via the Vue router onyx can not handle this automatically. We would propably need to create a property for the open state of current route
  3. Thats intentional, the click events are emitted by the nav buttons, not the nav bar itself so its not logged in Storybook. You need to add a click handler to the OnyxNavButton inside the project

@BoppLi
Copy link
Contributor

BoppLi commented Jul 1, 2024

  1. could you add that property and connect it in the alpha app? imho the UX is not the best when it just stays open

@github-project-automation github-project-automation bot moved this from In Approval to Done in onyx Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Requires technical expertise
Projects
Status: Done
Development

No branches or pull requests

3 participants