-
Notifications
You must be signed in to change notification settings - Fork 331
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
Reduce the height of wrapped navbar #2117
base: main
Are you sure you want to change the base?
Conversation
Navbar items have been forced the header height. This means if they get wrapped because the window is not wide enough, the navbar will double its height. This costs a lot of vertical space, which is particularly annoying as screens with limited width typically also have limited height. This PR changes the behavior to not force the height. Additionally we set a small vertical margin so that it is ensured that items have a bit of space around them. This approach does not change the visible appearance of the navbar if everything fits in one line. But it reduces the vertical size of the navbar for wrapped content.
xref to #1784 which has somewhat related goals |
Thanks for the pointer I was not aware of this. I suppose we agree the current situation is not ideal. We can reduce the nav height in two ways: (1) reduce vertical spacing for wrapped content (2) rearrange/collapse items.
I’m afraid (and the stalled discussion in #1784 supports this) that we don’t do anything if we strive for the perfect solution. Therefore, I want to advertise this PR as a minimal step forward. It does not touch the arrangement and thus makes nothing better or worse here. It only reduces the excessive height. It should be so simple that it will not complicate possible future work on arrangement. |
yes.
agreed.
+1 from me. |
(ps: a11y test failure can be ignored, it's unrelated and being looked at elsewhere) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with the caveat that I (nearly) always defer to @gabalafou for questions about CSS heights, margins, and alignments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate to be the spoil sport, but I'm -1 on this. To my eyes, the header after this change looks too cramped at intermediate width. But I'm not a designer, so it's just my personal opinion without much to back it up. But in general, my understanding is that most visual designs tend to use too little whitespace rather than too much.
I understand that the header navbar is frustrating. And I hear the argument about letting perfect be the enemy of good. But I can't help but think that the right path forward is not a bunch of tiny incremental changes that will push and pull the header in directions that might work better for some use cases, worse for others—but rather a focussed effort at really thinking through the header layout at narrow, medium and wide widths, taking 4-5 real sites with quite different headers as use cases.
It's just a considerable effort.
Co-authored-by: gabalafou <[email protected]>
The whole point of this PR was to make the increment really minimal, and only affect the "intermediate width" case, so that other cases are not affected and do not get worse. But if you say that the smaller margins on the currently happening multi-line nav make it worse, then there's fundamentally no small step to improve.
Which means, it's unlikely to happen 😞 A semi-related alternative thought: One could successively hide additional menu entries in a "More" dropdown as the width shrinks. This may be a good enough solution and orthogonal to moving/collapsing other parts of the nav bar. AI generated code shows that this is fundamentally possible (see below), but this is well beyond my HTML/CSS/JavaScript comfort zone, so I'm only dropping the idea. Production-quality implementation would have to come from somebody else. Code<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Responsive Navbar with More Dropdown</title>
<style>
* {
margin: 0;
padding: 0;
box-sizing: border-box;
}
body {
font-family: Arial, sans-serif;
}
.navbar {
display: flex;
background: #333;
padding: 10px;
overflow: hidden;
white-space: nowrap;
}
.nav-links {
display: flex;
list-style: none;
flex-wrap: nowrap;
}
.nav-links li {
padding: 10px 15px;
color: white;
cursor: pointer;
}
.nav-links li:hover {
background: #555;
}
.dropdown {
position: relative;
display: none;
}
.dropdown-content {
position: absolute;
top: 100%;
left: 0;
background: #444;
display: none;
min-width: 150px;
z-index: 1000;
}
.dropdown-content li {
display: block;
padding: 10px;
}
.dropdown:hover .dropdown-content {
display: block;
}
.more {
background: #222;
padding: 10px 15px;
cursor: pointer;
color: white;
display: none;
}
.more:hover {
background: #555;
}
</style>
</head>
<body>
<nav class="navbar">
<ul class="nav-links">
<li>Home</li>
<li>About</li>
<li>Services</li>
<li>Portfolio</li>
<li>Blog</li>
<li>Contact</li>
<li>Other</li>
</ul>
<div class="more">More ▼</div>
<ul class="dropdown-content"></ul>
</nav>
<script>
function adjustNavbar() {
const navbar = document.querySelector('.navbar');
const navLinks = document.querySelector('.nav-links');
const more = document.querySelector('.more');
const dropdown = document.querySelector('.dropdown-content');
const navItems = Array.from(navLinks.children);
// Reset: Move all items back to nav-links
while (dropdown.firstChild) {
navLinks.appendChild(dropdown.firstChild);
}
// Hide More button initially
more.style.display = 'none';
dropdown.style.display = 'none';
// Move overflowing items to More dropdown
while (navbar.scrollWidth > navbar.clientWidth && navLinks.children.length > 1) {
dropdown.insertBefore(navLinks.lastElementChild, dropdown.firstChild);
more.style.display = 'block';
}
// Show dropdown only if it has items
if (dropdown.children.length > 0) {
dropdown.style.display = 'block';
}
}
// Adjust on load and resize
window.addEventListener('load', adjustNavbar);
window.addEventListener('resize', adjustNavbar);
</script>
</body>
</html> |
Hmm. But from a UX perspective, I don't really love that pattern either! 🙈 Just as in the real world, it's a real advantage for landmarks to always appear in the same place. The more things move around, the less easily a person can navigate, whether in the real world or in the digital world. Sorry, I'm really not trying to be obstructionist. This is just very tricky territory. The header nav bar is probably the hardest thing in the theme to get right, and almost impossible to make everyone happy. If you like, I could kick off a GitHub discussion, so that we can collectively align and begin to sketch out a path forward. I totally agree with you that the header layout needs to be improved. But so far I haven't seen any change to it that to me looks like an improvement, and I haven't had the time to think through how to improve it. |
Thanks. A better scaling would be much appreciated, as many projects have a lot of content in the nav (e.g. matplotlib, numpy, scipy). But I'm afraid I've already contributed all ideas I have to alleviate the problem. Possibly UI experts can do a better job, so I will step back. I may just go and apply this PR locally in our template until somebody comes up with a better idea. |
Navbar items have been forced the header height. This means if they get wrapped because the window is not wide enough, the navbar will double its height. This costs a lot of vertical space, which is particularly annoying as screens with limited width typically also have limited height.
This PR changes the behavior to not force the height. Additionally we set a small vertical margin so that it is ensured that items have a bit of space around them. This approach does not change the visible appearance of the navbar if everything fits in one line. But it reduces the vertical size of the navbar for wrapped content.
Before:

After:
