pydata / pydata-sphinx-theme

A clean, three-column Sphinx theme with Bootstrap for the PyData community
https://pydata-sphinx-theme.readthedocs.io
BSD 3-Clause "New" or "Revised" License
557 stars 300 forks source link

[css] De-jumble header items at intermediate widths :) #1784

Open sneakers-the-rat opened 2 months ago

sneakers-the-rat commented 2 months ago

Hello :):)

I've noticed this on a few different sites that use pydata-sphinx-theme - the header items can look a little jumbly at intermediate widths where the navbar items need to wrap, eg. the current dev docs:

Screenshot 2024-04-24 at 7 41 19 PM

It just looks a little random - a lot of whitespace is wasted in the center, and the way that items wrap on the right is odd.

Here's a tiny little change to the CSS that changes the wrapping behavior and makes it look like this:

Screenshot 2024-04-24 at 7 36 50 PM

Specifically, i disabled wrapping in the top-level containers .navbar-header-items__end et al. instead the lower-level navbar-items can wrap, which works with the way that the template currently puts lists of links inside of them.

I also replaced the implicit vertical padding that came from giving items a fixed height to explicit padding in the outermost nav container.

The theme puts >5 header links in the More dropdown, but this still works in the case that the 5 displayed links are extremely long like this:

Screenshot 2024-04-24 at 7 32 24 PM

Ideally, rather than wrap, the search bar and items would just be hidden, but that would require JS since the CSS-only approach of setting a fixed height to the bar and hiding overflow wouldn't work here because the More menu would be cut off too. This is a very slight improvement imo that just makes things look a little tidier without any functional changes :)

drammock commented 2 months ago

Thanks for the contribution! Agree that this is an improvement. I can imagine some alternative approaches that might look better for our specific docs site (separately flex-wrapping the groups of __center and __end items?), but I'm not sure how generalizable they'll be to other sites. @gabalafou what do you think of the approach in this PR?

trallard commented 1 month ago

@gabalafou bringing this to your attention since we have discussed navbar stuff too!

Carreau commented 1 month ago

Conflict fixed.

gabalafou commented 1 month ago

This is on my radar. Variable layout is hard. It's going to take me some time to review this but hopefully I can tackle this in the coming week.

gabalafou commented 1 month ago

I'm kinda hesitant to change the any of the layout properties without a strong design vision for this, which combines both how much customization we want to support and what constraints we want to enforce on the header nav layout

sneakers-the-rat commented 1 month ago

Up to you. I dont have any design vision here except "make sensible use of the space," "avoid wrapping text in tiny boxes," and "keep semantically related things in visually distinct groups." Its an extremely minor change that fixes something that looks imo bad on a bunch of deployments of the skin that ive seen, but idrc if yall want to close this without some design statement :)

gabalafou commented 1 month ago

I appreciate you trying to fix it! It's just that this is probably the hardest CSS on the site to get right. It's also a difficult design challenge, since the theme has to accommodate a number of different headers on different sites and still look good.

To make this more concrete, I'm not really sure that I agree that the second screenshot is an improvement over the first.

Here's why I think that. My understanding is that the brain is wired for spatial mappings. For sighted users, if the search button is on the right side of the page on both a wide desktop and a mobile screen, I don't think it would be a good idea to put the search button on the left side of the page on a medium-wide screen. It might look better visually, but I think it will upset users' expectations.

I totally agree with you that the current implementation needs improvement. It's just not clear to me at the moment how we should improve it.

gabalafou commented 1 month ago

Since this is in my headspace and I had an idea, I'm going to jot it down before I forget it.

I think my preferred design direction for this would be...

We constrain the header nav to one line. No wrapping. As soon as we start running out of space, first we compactify the search button, then start popping groups of items off the edges in a defined order into the hamburger menus.

Defined order would probably have to be:

  1. First pop the icon links together (twitter, github, etc.) into the right hamburger menu (currently these end up in the left hamburger menu, which I think is a mistake)
  2. Light/dark button -> right
  3. Version switcher + header nav links -> left

I'm not quite sure how much work this would be though, nor if constraining the header nav to a single line is feasible or too much work or truly desirable.

gabalafou commented 1 month ago

nor if constraining the header nav to a single line is feasible or too much work or truly desirable.

To elaborate... some of the sites that adopt the theme might need to display four or five links with long titles in the header nav. They might prefer to have the header nav height expand rather than have those header nav links moved into the left hamburger menu. See for example the third screenshot in the PR description, the one with the long links.

drammock commented 1 month ago

See for example the third screenshot in the PR description, the one with the long links.

this site (from one of our regular bug-reporters) would be a good test-bed for any changes; it has 8 header nav items and several of them are fairly long.

dbitouze commented 1 month ago

this site (from one of our regular bug-reporters) would be a good test-bed for any changes; it has 8 header nav items and several of them are fairly long.

It would be a pleasure! :smiley:

sneakers-the-rat commented 1 month ago

For sighted users, if the search button is on the right side of the page on both a wide desktop and a mobile screen, I don't think it would be a good idea to put the search button on the left side of the page on a medium-wide screen. It might look better visually, but I think it will upset users' expectations.

yep. spent awhile trying to do that, and so:

Ideally, rather than wrap, the search bar and items would just be hidden, but that would require JS since the CSS-only approach of setting a fixed height to the bar and hiding overflow wouldn't work here because the More menu would be cut off too.

because the issue is need for real estate, so would either need to swap out to a search icon you could expand the search bar from or hide it entirely, but was trying to make a minimal PR. if you don't think it's an improvement then close this, bc i see it as an unambiguous improvement even if it's not the ideal.

As soon as we start running out of space, first we compactify the search button, then start popping groups of items off the edges in a defined order into the hamburger menus.

sure, again requiring JS intervention and lots of breakpoints or size checking. this is specifically an intermediate screen width issue where wrapping and >1.5em is fine, mobile is already one line. from what i saw the 5 links "more" collapse is hardcoded at generation time so would need a rewrite of that too.

this site (from one of our regular bug-reporters) would be a good test-bed for any changes; it has 8 header nav items and several of them are fairly long.

included long header and nav items in the example in OP too

gabalafou commented 1 month ago

Thanks for your feedback, I'll try to get back to you in the next few days

sneakers-the-rat commented 1 month ago

re-read that and it comes across as more negative than i intended, i just meant i wasn't sure about y'alls process and if it's more of a pain than it's worth go ahead and do it however you'd normally do it, i don't have much time to fix it atm and just didn't want it hanging out indefinitely for y'all :). just trying to help is all not cause a headache.