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

Nav items overflowing navbar on medium viewports #1271

Closed
3 tasks done
fvsch opened this issue Sep 1, 2022 · 5 comments
Closed
3 tasks done

Nav items overflowing navbar on medium viewports #1271

fvsch opened this issue Sep 1, 2022 · 5 comments
Labels
theme Related to the theme

Comments

@fvsch
Copy link

fvsch commented Sep 1, 2022

Describe the bug

When using the default theme, the navbar items are hidden away in a menu on screen widths lower than 768px.

At 768px and slightly larger sizes, the navbar may have to render on one line:

  • The logo and site title
  • A search box (when using the algolia options)
  • Several nav items
  • And the "extra navigation" button

When the nav items don't fit, their text may wrap, and their extreme line-height makes that result particularly bad:

Screenshot 2022-09-01 at 23 57 42

Reproduction

  1. Visit reproduction on StackBlitz: https://stackblitz.com/edit/vitepress-theme-issue-navbarmenulink?file=index.md
  2. Resize the preview area to be roughtly 800px wide

Expected behavior

There are two levels of fixes:

  1. Mitigate the breakage when nav items do wrap.

    • Nav links should not use the header height as their line-height (that's a bad way to do vertical centering of text, for the reason demonstrated in this issue).
    • Instead use something like min-height: var(--vp-nav-height-mobile); line-height: normal; (plus probably a tad of padding-block).
  2. If the nav items still overflow their container, what should happen? Should they be clipped (overflow: hidden on the container), or should the container height grow (i.e. the container should be using min-height and not height)? Clipping might be better to conserve the design, but note that it's a WCAG violation (loss of content).

  3. Prevent this situation altogether:

    • One option is to use the mobile layout earlier, e.g. for widths lower than 1000px instead of widths lower than 769px. (This might not be enough for some accessibility use cases, like text-only zoom.)
    • Another one is to implement a type of dynamic overflow menu, where all items that don't fit in the navbar are moved to the overflow menu. This requires runtime JS, probably using IntersectionObserver.

System Info

System:
    OS: macOS 12.5.1
    CPU: (8) arm64 Apple M1
    Memory: 98.88 MB / 8.00 GB
    Shell: 3.5.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 16.17.0 - /opt/homebrew/opt/node@16/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.12.1 - ~/.npm-packages/bin/npm
  Browsers:
    Chrome: 105.0.5195.52
    Firefox Nightly: 106.0a1
    Safari: 15.6.1
  npmPackages:
    vitepress: ^1.0.0-alpha.13 => 1.0.0-alpha.13

Additional context

I can probably do a PR for the line-height part at least.

Validations

  • Follow our Code of Conduct
  • Read the docs.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
@fvsch fvsch added the bug: pending triage Maybe a bug, waiting for confirmation label Sep 1, 2022
@ambodu
Copy link

ambodu commented Sep 5, 2022

@fvsch
Copy link
Author

fvsch commented Sep 5, 2022

i think this problem can be solved by this way: white-space: nowrap

That prevents text wrapping, so you don't get the "nav item becauses twice the height of the navbar" problem, but then the nav items will overflow the navbar and make other items go out of view much quicker.

I think my fix in #1273 is a better approach.

@brc-dd brc-dd added bug theme Related to the theme and removed bug: pending triage Maybe a bug, waiting for confirmation labels Sep 5, 2022
@ambodu
Copy link

ambodu commented Sep 6, 2022

@fvsch hey guys, i got you, #1273 is a better approach.😀

@kiaking
Copy link
Member

kiaking commented Oct 31, 2022

Thanks for the report and also the PR, but I really think the right approach to this is to "change the title & menu structures".

There's no way we can fit an unlimited number of menus. So, if it gets over-wrapped, users should consider grouping them into a dropdown menu or shortening the labels.

It is all about balance. For example, if you have lots of social links, you will get less space for menu items too.

Sometimes, the content has to adjust to the design. Or, even better, create own custom nav bar that can display the long texts 👍

@kiaking kiaking closed this as not planned Won't fix, can't repro, duplicate, stale Oct 31, 2022
@fvsch
Copy link
Author

fvsch commented Nov 24, 2022

I disagree. IMO designs should be implemented to be resilient to not-so-extreme cases, in line with the spirit of responsive design (see also: defensive CSS. Oftentimes that's necessary to meet WCAG AA requirements too.

The current design breaks easily with just ~3 items when those items are not short single word labels, and it currently breaks for two localizations of the Vite docs:

Spanish (the "…" menu is pushed to the right, outside of the viewport):

Screenshot of the header of the Vitest docs in the Spanish localization, at a width of 768px

Chinese (showing the line-height issue again, with label wrappings to two lines, with huge gaps between lines):

Screenshot of the header of the Vitest docs in the Chinese localization, at a width of 768px

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
theme Related to the theme
Projects
None yet
Development

No branches or pull requests

4 participants