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

Header: Use different templates based on page heirarchy #500

Merged
merged 9 commits into from
Feb 22, 2024

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Feb 13, 2024

See WordPress/wporg-mu-plugins#573 — There are updates to how the local navigation should appear across the site in an effort to simplify, make more responsive, and more consistent. The functionality changes are in wporg-mu-plugins, this companion PR serves to update the templates.

The header templates are included in the page templates, except for the new header-third template. Since this is included based on "hierarchy" rather than template-type, there's a filter in place to swap out the requested header templates. For example, the Block Handbook homepage and interior pages both use single-handbook.html templates, so the template calls for header-alt, but if the page is an interior page, it's replaced with header-third.

This PR also introduces a new block, page-title, which serves as a combination of query-title and post-title in one, using the post title on single posts and a search or archive title for those views. It also adds the () to relevant parser type titles. This avoids needing to create multiple header-alt/header-third templates for each type of content.

Background

There are now 3 header possibilities depending on page hierarchy, with 2 variations — in this PR, they correspond to:

Design name Template
Level 1A patterns/front-page-header.php
Level 2A parts/header.html
Level 2B parts/header-alt.html
Level 3+ parts/header-third.html

We don't need to worry about Level 1B, because Level 1 only applies to home, so each site/section should only have one of the two.

Level 2 are items one level deep, with "2A" being things in the local nav, and "2B" not in the local nav. Splitting them avoids repeating the page title in the local nav bar.

Level 3+ are below that, and have breadcrumbs so you can jump back up to Level 2 & 1 (home).

Screenshots

Note: I used the WP-CLI command page to show L2A, but there's a bug that's causing it to not be highlighted in the local nav, tracked here: #498.

Default Scroll Small screen
level-1 level-1-scroll level-1-mobile
level-2a level-2a-scroll level-2a-mobile
level-2b level-2b-scroll level-2b-mobile
level-3 level-3-scroll level-3-mobile

To test:

For the best look, apply WordPress/wporg-mu-plugins#573 to wporg-mu-plugins.

View the following pages, and make sure they're using the expected template.

@adamwoodnz
Copy link
Contributor

The top of the search results page has become pretty tight now that breadcrumbs are not displayed, should probably be a 20px gap at the top, or maybe 40px.

Prod Local
developer wordpress org__s=block(3 1536x864) localhost_8888__s=block(3 1536x864)

@adamwoodnz
Copy link
Contributor

adamwoodnz commented Feb 14, 2024

On top level handbook pages there's an issue with misaligned sidebar containers now that the breadcrumbs aren't displayed:

Kapture.2024-02-14.at.14.42.01.mp4

I think we'll need to change the --wp--custom--wporg-sidebar-container--spacing--margin--top var for those pages, as that's what controls the top margin of the ToC. 100px works for me.

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.

👍 working well for me apart from a couple of things inline

Copy link
Contributor

@renintw renintw left a comment

Choose a reason for hiding this comment

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

Have you encountered this situation? At /apis.

Screen.Capture.on.2024-02-16.at.02-45-44.mp4

@ryelle
Copy link
Contributor Author

ryelle commented Feb 16, 2024

On top level handbook pages there's an issue with misaligned sidebar containers now that the breadcrumbs aren't displayed

Fixed in 9eaf52e

Screenshot 2024-02-16 at 6 17 45 PM

The top of the search results page has become pretty tight now that breadcrumbs are not displayed

I've updated this to match the other pages, so it's 20px now. f018c3f

Screenshot 2024-02-16 at 6 17 31 PM

Have you encountered this situation? At /apis.

@renintw I have, but I don't think it's related to this PR, I can replicate it on production.

@ryelle ryelle requested a review from adamwoodnz February 16, 2024 23:21
Copy link
Contributor

@renintw renintw left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I have, but I don't think it's related to this PR, I can replicate it on production.

Thanks. I've opened a ticket for it.

@@ -158,7 +158,7 @@
"wporg-sidebar-container": {
"spacing": {
"margin": {
"top": "150px"
"top": "100px"
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, obviously more clarity required; on pages with breadcrumbs it needs to remain 150px, but for the top level handbook pages without breadcrumbs 100px.

The child page sidebars are now misaligned:

Screenshot 2024-02-20 at 12 13 47 PM

@ryelle
Copy link
Contributor Author

ryelle commented Feb 21, 2024

Okay, I think I have it now @adamwoodnz — how's it looking to you?

Screenshot 2024-02-21 at 4 55 22 PM Screenshot 2024-02-21 at 4 55 14 PM

@ryelle ryelle force-pushed the update/simplified-subnav branch from 9e99f50 to eb51035 Compare February 21, 2024 21:59
@ryelle
Copy link
Contributor Author

ryelle commented Feb 21, 2024

Rebased this PR to make sure it still works after the updated layout changes 👍🏻

@adamwoodnz
Copy link
Contributor

Okay, I think I have it now @adamwoodnz — how's it looking to you?

LGTM!

@ryelle ryelle merged commit 69b4f3a into trunk Feb 22, 2024
1 check passed
@ryelle ryelle deleted the update/simplified-subnav branch February 22, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants