primer
🐛 [BUG] - SubNav current page and dropdown toggle overflow on narrow viewports
Describe the bug
On narrow viewports, the current page and dropdown toggle chevron overflow when the current page and heading are sufficiently long.
Reproduction steps
- Drop down to a narrow viewport and set the heading and current page titles to be sufficiently long.
- Observe that the current page overflows and the dropdown chevron isn't visible
Expected behavior
On a slightly wider viewport you can see how it should display
We might have to consider reflowing these elements where necessary, maybe this kind of thing
@joshfarrant can you share the px value you used for the narrow viewport please? Was it arbitrary of mapping to our narrow Primer Brand breakpoint?
@rezrah the specific px value doesn't matter for this bug. If you view the mobile version of the SubNav and set any sufficiently long string as the current page, then the string will overflow
It looks like we use this version of the SubNav on viewports 63.25rem (I believe that's medium) or below
and set any sufficiently long string as the current page
I'm wondering what the use-case is for this? ☝ We discourage long strings (like full sentences) in this component, and haven't designed the component to support high char counts (like your example above) beyond the site title. Reflow doesn't really work here, we've looked at that before but could open another convo with Site about it if needed.
Is there a related ship where the SubNav.Link items are being used this way? E.g. Is it Security page?
You can see examples of our recommendations here, or in Storybook, but we're admittedly missing some interface guidelines to make this more obvious. Let's get those added when new docs ship.
Btw...in the Security example you've shared above, some slight tweaks to content can make this fit at our 320px (minimum) breakpoint👇
Yes it's currently used in this way on https://github.com/security/plans
As using the component with longer strings is quite easily done (despite not being recommended), I wonder if having the text wrap to the following line would be a more graceful fail state than the current behaviour, however I'm happy to close this if we consider this not to be valid usage of the component and just rely on improved documentation instead.
-
Wrapping was discussed the during design phase but doesn't easily work for multiple reasons. A notable one being that we'll end up with a problematic and duplicative open state. 👇
-
We could probably tighten up the spacing a little on narrow viewports, but that won't entirely mitigate the issue in all cases.
-
Could also drop the separator at same time, but looping in @jesussandreas for second opinion as this was an important part of this patterns presentation.
-
I'd be curious why we need the GitHub qualifier before Security 🤔. We don't do that in other places (besides Copilot) and it feels quite redundant. Wonder if an 'easy' fix is using more concise content. A similar example is that we use ampersands instead of
andfor style guiding and char counts limiting. -
Another option is to remove
GitHubonly at narrow viewports, but keep it at wider ones. While one could argue it's an inequitable experience across breakpoints, I'd counter it's really not adding communicating anything of significant value to the user 🤷 (back to previous point)
cc. @danielguillan for FR second opinion. Also @raytalks for visibility on this convo thread (and specifically discussions around content on custom pages)
@rezrah I agree that we need to keep these links concise, so I will work with our other teams to keep an eye on the character lengths. And I agree we should replace "and" with ampersands in these as well.
I am following up with Wayne and Bobby on whether we can remove "GitHub" from "GitHub Security" in the SubNav. Seems like it may have been done for SEO reasons, but I agree that it does not add much except for the Copilot SubNav scenario.
