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

About: Add the full About section menu to the About pages #387

Merged
merged 4 commits into from
Feb 12, 2024

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Feb 7, 2024

Update the local navigation in the About section to be consistent across all About subpages, and add the menu to the main About page.

Requires WordPress/wporg-mu-plugins#570. Fixes #385.

Note: This does not add any of the iterations proposed in WordPress/wporg-mu-plugins#535, it only updates the navigation content.

This change makes _nav-about-details.php, _nav-about-people.php, and _nav-about-technology.php have all the same content, but I've left them separate in case we decide to bring back this semi-sectioning idea. Happy to switch to a single _nav-about-subpage.php file instead if we'd rather be aggressively DRY. Update: Changed this to _nav-about-subpage.php, removed the other files, and updated all the page templates to call the new pattern.

Screenshots

Default With dropdown open
about about-dropdown
about-subpage about-subpage-dd

And on small screens:

about-mobile

How to test the changes in this Pull Request:

Use WordPress/wporg-mu-plugins#570 for mu-plugins, otherwise there will be PHP errors.

  1. View the About page
  2. There should be a local navigation bar with 3 dropdowns
  3. Click any page, the same navigation should appear on the subpages
  4. Try the same with a small screen, the navigation should work as expected
  5. View a Download subpage, there should be no change to the navigation here
  6. View the footer of the About page, the 3 menus in the page content should still work

@adamwoodnz
Copy link
Contributor

Happy to switch to a single _nav-about-subpage.php file instead if we'd rather be aggressively DRY.

IMO lean towards YAGNI

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I always thought we needed sub nav for this page 👏

Works for me, I did notice that the focus outline is cut off when operating via keyboard:

Screenshot 2024-02-08 at 2 01 12 PM

Screenshot 2024-02-08 at 2 02 15 PM

Outline colors look a little off too, (not blueberry?) but maybe a more widespread issue, I see it in the page content too.

Maybe this should be addressed in WordPress/wporg-mu-plugins#535, but is it worth trying to align the top of the overlay with the bottom of the nav bar? Question for @WordPress/meta-design I guess. I notice that the global header overlays don't quite align either:

Screenshot 2024-02-08 at 2 17 45 PM

@jasmussen
Copy link
Contributor

Good catch, that looks like the default focus style. I think in some other pages we have a customized 1.5px box shadow. That'd be nice to just apply across, if possible. It can be inset to avoid being cropped.

@fcoveram
Copy link

fcoveram commented Feb 8, 2024

@jasmussen's suggestion is documented in the local nav components as part of WordPress/wporg-mu-plugins#535 designs

Local item component with front-end specs annotations

@ryelle
Copy link
Contributor Author

ryelle commented Feb 8, 2024

Works for me, I did notice that the focus outline is cut off when operating via keyboard:
Outline colors look a little off too, (not blueberry?)

Yeah, as @fcoveram mentions, that will be applied with the other local nav improvements in wporg-mu-plugins.

is it worth trying to align the top of the overlay with the bottom of the nav bar?

The design has it offset like this (note that we can't have a different submenu background + mobile overlay background, which is why I went with the tan). This is also how the nav menu works by default, so it was easier to leave it be :)

Screenshot 2024-02-08 at 11 39 41 AM

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit: LGTM

@fcoveram
Copy link

fcoveram commented Feb 9, 2024

The design has it offset like this … This is also how the nav menu works by default, so it was easier to leave it be :)

Fully agree.

note that we can't have a different submenu background + mobile overlay background, which is why I went with the tan

Perfect. I'll update mockups for documentation purposes.

@ryelle ryelle merged commit 5bc9ab0 into trunk Feb 12, 2024
2 checks passed
@ryelle ryelle deleted the add/about-local-nav branch February 12, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Theme Templates, patterns, CSS
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add local navigation to the About page
4 participants