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

Fix breadcrumbs. #166

Merged
merged 10 commits into from
Jan 19, 2023
Merged

Fix breadcrumbs. #166

merged 10 commits into from
Jan 19, 2023

Conversation

StevenDufresne
Copy link
Contributor

@StevenDufresne StevenDufresne commented Jan 16, 2023

Closes: #163

This PR creates a component and calls breadcrumb_trail to create the site breadcrumbs. I chose to not use site-breadcrumbs because of how complex the breadcrumb logic is for this theme.

Since handbook paths can be up to 5 parts long of varying lengths, I had to come up with a system to truncate. I added some CSS to handle that truncation by hiding items left to right starting after the "home" link as the viewport shrinks. It seems to work well although I'll admit I'm not certain I've seen the longest path to test against yet. I have also only handled up to 5 parts. I think we should reconsider our approach if articles are nested 6 or more times.

Alternatives

Use site-breadcrumbs filter.

When the breadcrumb code is created in this theme, HTML is already added to the $items list. If we wanted to make use of the wporg_block_site_breadcrumbs filter we would have to strip that all out. Not the hardest thing to do, but slight risk.

I could be convinced down this path, in 50/50.

Screenshots

Desktop Tablet Mobile
localhost_8888_block-editor_how-to-guides_internationalization_ localhost_8888_block-editor_how-to-guides_internationalization_ (1) localhost_8888_block-editor_how-to-guides_internationalization_ (2)

@StevenDufresne
Copy link
Contributor Author

I just found one that is mindblowingly long:

Developer / Coding Standards Handbook / Inline Documentation Standards / JavaScript Documentation Standards

@jasmussen, Any idea on what we can do here?

@jasmussen
Copy link
Collaborator

Developer / Coding Standards Handbook / Inline Documentation Standards / JavaScript Documentation Standards

Keeping in mind I'm not aware of what's technically feasible here, my instinct would be this:

Developer / ... / Inline Documentation Standards / JavaScript Documentation Standards

That is, show the first and last clickable breadcrumbs.

Alternately we just allow wrapping. It would be good to see this in context before we settle on it, though.

@ryelle
Copy link
Contributor

ryelle commented Jan 16, 2023

Use site-breadcrumbs filter.

This method was what I intended we'd do for all these subsites when I made it, you can see in Documentation how I used it there.

Could you refactor whatever's giving you HTML to return the url/title format used by wporg_block_site_breadcrumbs?

Since handbook paths can be up to 5 parts long of varying lengths, I had to come up with a system to truncate.

We've had a similar issue on Documentation (up to 4 parts, but article titles can be very long. I also wrote some CSS to hide those middle paths in the Docs theme. Whatever the solution for this, it would be best to port back up to the site-breadcrumbs block, and IMO, this is the main reason for everyone to use the same block, since we're running into all the same problems.

@StevenDufresne
Copy link
Contributor Author

Alright, I updated to use the site-breadcrumbs and reverse out CSS dealing with making it look good on mobile. Let's address that in the my plugin itself: WordPress/wporg-mu-plugins#325.

@StevenDufresne StevenDufresne marked this pull request as ready for review January 19, 2023 02:54
@StevenDufresne StevenDufresne requested review from dd32 and ryelle January 19, 2023 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Component: Breadcrumbs
4 participants