Skip to content

Breadcrumb: Some code quality improvements #72873

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.

Sign up for GitHub

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

Jump to bottom
Merged
ntsekouras merged 5 commits into from
Nov 4, 2025
Merged

Breadcrumb: Some code quality improvements #72873

ntsekouras merged 5 commits into from
Nov 4, 2025

Conversation

@ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Oct 31, 2025 •
edited
Loading

What?

Follow up of: #72714

This PR addresses some feedback from there:

  1. Changes the title for 404 to Page not found
  2. Trims the search query to 10 words
  3. Refactors the paged breadcrumbs code not to do any string manipulation and handle each case explicitly. Some more helper functions could be considered there (maybe for getting the markup for the current item) but we can look into it after we've added the whole of the main functionality that is wanted. It'd be a simple change..

Testing Instructions

  1. Insert Breadcrumbs blocks and observe the change in 404 breadcrumb item
  2. Search something quite long and see the truncated breadcrumb item
  3. Test that paged cases work exactly as before and as expected.
  4. It's easier to review by hiding whitespace

@ntsekouras ntsekouras self-assigned this Oct 31, 2025
@ntsekouras ntsekouras requested a review from ajitbohra as a code owner October 31, 2025 11:59
@ntsekouras ntsekouras added the [Type] Code Quality Issues or PRs that relate to code quality label Oct 31, 2025
@ntsekouras ntsekouras added the [Block] Breadcrumbs (experimental) Affects the Breadcrumbs Block label Oct 31, 2025
@ntsekouras ntsekouras mentioned this pull request Oct 31, 2025
Merged
@github-actions
Copy link

github-actions bot commented Oct 31, 2025 •
edited
Loading

Flaky tests detected in 79a08d2.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19040777725
📝 Reported issues:

tyxla
tyxla reviewed Oct 31, 2025
View reviewed changes
@github-actions
Copy link

github-actions bot commented Oct 31, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ntsekouras <[email protected]>
Co-authored-by: tyxla <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

tyxla
tyxla reviewed Oct 31, 2025
View reviewed changes
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

BTW, disregarding the above feedback, this tests well for paginated archives (category, post tag, custom taxonomy, author, year, month, date, custom post type archives) in my tests 👍

However, I noticed that we don't support paginated posts (content separated by page breaks):

Screen.Recording.2025-10-31.at.15.04.03.mov

This is something we should address separately - I've opened an issue for it in #72876.

tyxla
tyxla reviewed Nov 3, 2025
View reviewed changes
This was referenced Nov 3, 2025
Merged
Merged
tyxla
tyxla approved these changes Nov 3, 2025
View reviewed changes
Copy link
Member

@tyxla tyxla left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tests well and code LGTM 👍 Thanks!

(will need a rebase it seems)

@ntsekouras ntsekouras merged commit 9fc4175 into trunk Nov 4, 2025
33 checks passed
@ntsekouras ntsekouras deleted the follow-up-72714 branch November 4, 2025 06:56
@github-actions github-actions bot added this to the Gutenberg 22.1 milestone Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tyxla tyxla tyxla approved these changes

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@fabiankaegy fabiankaegy Awaiting requested review from fabiankaegy fabiankaegy is a code owner

Assignees

@ntsekouras ntsekouras

Labels

[Block] Breadcrumbs (experimental) Affects the Breadcrumbs Block [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Milestone

Gutenberg 22.1

Development

Successfully merging this pull request may close these issues.

3 participants

@ntsekouras @tyxla