-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
New block: Add a Breadcrumbs block #32500
base: trunk
Are you sure you want to change the base?
New block: Add a Breadcrumbs block #32500
Conversation
Update: I'm still keen to continue developing this block (I switched over to working on some other projects these past few weeks, but hope to keep chipping away at this in my spare time). If anyone has a chance to take a look, I'm keen to get feedback on the approach, and input into some design considerations:
Many of these ideas might not be required for an MVP for this block, so I'm happy to hear any ideas! |
0bbe814
to
ea49cfc
Compare
I've given this a rebase and fixed up some linting issues, so this should be good for folks to take for a spin if you'd like to test it out. There's still a bunch of To-do list items (and likely many To-do items that I haven't thought of), so please do let me know if you have ideas on how you think this block should be implemented, or needed features. |
Hey @andrewserong Testing the Gutenberg PR build. This looks really really nice! Great work! I made 3 nested pages: top page - 2 page - 3 page Panels: Nesting level. I am wondering if it should start without the value 0 present (if possible) or a dash - (no value). As I initially read it as Nesting level 0 (meaning no nesting levels). I then read the text: Set to 0 to show all nesting levels. Nesting level does not show the home page. I would assume when selecting Nesting level 0 that the home page would also show. Separator character. When removing / I notice the placeholder says e.g. / I closed the various panels such as Typography and Spacing. Deselected the Breadcrumbs block and reselected it again. I noticed that Typography and Spacing was open again. I assume there is save mechanism so that when I close/open panels it would remain that way even upon deselecting and reselecting the Breadcrumbs block. Andrew this is really really nice work! Thank you for working on it! Btw the block toolbar alignment controls work really well! I found this interesting resource: |
Thanks for the feedback @paaljoachim, great suggestions!
Yes, I couldn't quite think of a good name for it, either, but even "Breadcrumbs" would be better to drop the superfluous word "settings".
Good catch, I like the idea of adding that in, and I think you're right, it would be good to handle that implicitly as part of the nesting levels. It might be worth also having a toggle for whether or not to show it? Either way, I like your idea of being able to override the site title / home breadcrumb 👍 Icons would be a cool addition, too, I'm curious how far we can get without introducing icons as a first pass, and perhaps treat icons as an enhancement in a follow-up. Thanks again for the feedback! I'm mostly focusing on helping out with some of the design tooling at the moment, but will come back to this and chip away at the suggestions here, let me know if you think of any others 🙂 |
Hey Andrew. Using the name Breadcrumbs instead of Breadcrumbs settings seems fine (for now anyway). A toggle to show the home page title seems like a good idea. Then a way to override the site title / home breadcrumb.
I agree. It sounds like a nice follow-up for a later point. I will let you know of additional suggestions as they show up. |
8c8a048
to
34b3091
Compare
I had a bit of time at the end of the week to make a couple of additions to the PR (thanks again for the feedback, Paal!):
Here's how it looks now in the editor: I'm sure there'll be some tweaks needed, but it feels nice having the site title in there now. I'll be working on other things next week, but hope to pick this PR up again after that! |
Looks good Andrew! Retesting. I changed the home page to Front page. Even though underlines should be in place because of accessibility it would be nice to have an option to turn these off. Here is an example from my dev site where I am recreating the main site: Here we can see none active links green while the active link contains the default black color. NB. It also uses all capital letters which I assume will be in the typography controls. |
Great suggestions, thanks for testing again, Paal! Yes, I think it'd be great to be able to control the display of underlines and custom secondary colors to support those sorts of designs, I imagine they'll be very common patterns. I'll dive in when I can circle back to this PR 🙂 |
4cefa11
to
826c7b1
Compare
Just pushed a small update to opt the block in to the font weight, style, and text transform typography controls. This gives a bit more flexibility to the variety of designs the block can support, from the creative to the slightly obnoxious 😄. Here's what it currently looks like: For the underline, it'll be great to be able to control it. I see there's an open WIP PR in #31768 to add in that typography control, too. |
Thanks for the additional testing @paaljoachim, much appreciated!
I've pushed an update to the
I believe border controls are only displayed on themes that use a
Very strange! I haven't been able to replicate this yet in my test environment, but I'll do some more digging. Can you please share the following:
Thanks again, it's very helpful having someone test this out while I'm tinkering away at this PR! 🙇 |
I've just pushed a change to explore adding in support for post categories, too. The logic now attempts to use the post/page hierarchy if available and then, if not available, it'll fall back to attempting to use the first category attached to the post. Here's how it looks in TT1-blocks currently on a post with a hierarchical category added: In the above screenshot, the hierarchy goes:
With the additional logic, some of the functions are now getting a little long, so before this PR comes out of Draft and when it's a little further along, I might spend some time cleaning / tightening up the code. |
Thanks for the extra info! Glad we got to the bottom of the site title issue, perhaps we can add some error handling in the Breadcrumbs UI for the site title to handle the case that the site title is empty 👍 |
Just to clarify, this PR now behaves like the Heading block. The block-based alignment allows for |
@paaljoachim just a follow-up on the border controls. In Gutenberg, the base Once the UI for the border controls stabilises, I believe the plan is to switch many more blocks over to displaying the UI controls by default (which would ultimately also include this block). This PR uses the same block supports mechanism to opt-in to the controls as the Button block does, so if and when this PR lands, we'll be able to go through a similar process and add a small follow-up PR to switch on the controls by default across all themes. In the mean-time, themes can opt-in to experimental controls, so I typically use TT1-Blocks as my testing theme (and manually edit the |
Thanks for the clarifications Andrew! |
Fantastic, thanks for the screenshots, Paal! Great to see an example of how others manage a UI for selecting icons, that looks like a neat way to handle it. I like the detail that even if "Icon" is selected, the label is still editable, too (I assume for accessibility). |
Hey Andrew @andrewserong |
…via editable RichText fields
…page options are enabled
1c1435c
to
d757a12
Compare
Thanks for the ping @masteradhoc! Unfortunately I haven't had much time to dust off this PR over the past few WP releases as there have been higher priority features and fixes to work on. I'm not sure how much time I'll have to continue working on this PR, but I've at least given this a rebase and fixed up a few linting issues, and I'll switch this over to ready for review. It could probably use a fresh pair of eyes to test out the current state of the feature, and whether it's a good enough shape for a v1 for core. I'm aware there are lots of plugins that implement Breadcrumbs features, so I'd be curious to hear what folks think might make for a good core version that balances most-needed features without being too opinionated. For example, the current PR adds schema.org markup to the output so that it (hopefully) plays nicely with search engine results, but I'm not too sure if it's a good inclusion for a v1. |
I just did a quick scan of the code so it can't count as a "review", but I do have a couple of notes:
|
I don't know that I'd make this block available in the post editor, it feels more like a theme/template feature to me. In that respect I'd expect it to not only support pages, but posts and taxonomies too. In terms of display options, the following are common requests from WooCommerce users:
|
Wonderful, thank you for the feedback @aristath, that's exactly the kind of insight I was after! 🙇 Deferring to plugins for SEO support sounds much better, and good to know that adding in a filter should make this straightforward. Also, thanks for the info about microdata vs I've removed the schema.org markup and added a filter for
Thanks @jameskoster, yes, I was imagining it only available in the site editor — I've pushed an update for that. The current feature set will support both posts and pages. It checks if we're in a hierarchical post type (e.g. pages) and uses that hierarchy, and if not, falls back to using hierarchical categories as in posts. I could imagine potentially adding in custom taxonomies in a follow-up for other use cases?
Good to know! In the current state of this PR folks can edit both of these by clicking into them within the editor canvas (for the separator character that means typing in a different character for it), so good to know we've (at least partially) covered those bases. |
Nice work. One important aspect of any new block we introduce is to boost the confidence that we actually want it and need it, because they are easy to add, hard to remove. |
@andrewserong |
Very good point! Happy to hear feedback from folks as to whether or not, on balance, this is appropriate as a new core block.
Thanks for the feedback! I think @aristath outlined well some of the challenges with including the breadcrumb schema in the core block. We currently don't appear to do this for any other core blocks, and to Joen's point about things being easy to add but hard to remove, it could be difficult to remove schema markup after the fact if we wanted to remove it, but would be much easier to add it in a follow up if it turns out that we do need it. So, I think I probably lean toward not including the schema markup for a v1 of the block. It'd then be possible to open follow-ups that explore potentially adding in schema markup (or JSON) to the output, if need be. I think part of the challenge of this PR in general has been to come up with a good set of defaults that makes for a simple, usable and useful block for most cases, while being flexible enough to be extended and improved over time. Again, please do share any and all feedback! I'll chip away at any feedback and ideas as I have time 🙂 |
It makes no sense to have breadcrumbs without the proper markup. Reference: https://developers.google.com/search/docs/appearance/structured-data/breadcrumb SEO plugins have their own Breadcrumbs block, so it's important this core block has the minimal markup required. They will not filter this block to add it! Just for reference, here is what SEO plugins are doing: |
Hey @andrewserong |
Thanks for the ping, Paal! Just an update: unfortunately I don't currently have time to continue working on the Breadcrumbs block PR for now, as I've switched to working on other priorities that have design consensus. Before picking this PR back up again, we probably need a decision to be made on the desired scope from a design angle / what folks can agree would be a good MVP for the block in the plugin. If / when that decision is made, it might be worth dusting this PR off again after WP 6.4 and seeing if it could be made viable again. Thanks for everyone's continued enthusiasm for the feature! 🙂 |
@jameskoster @jasmussen @richtabor |
Interesting discussion! There's a clear interest in a Breadcrumbs block. I'm not sure it should be part of the core suite, but there's a chance it could be added when someone browses patterns or blocks in the Inserter (related: #61504). After some internal discussion, I put together a design that I hope balances customizability and simplicity. I imagine it'd be extensible, meaning third parties could add custom settings and options (like icons or a new toggle to display the home link as an icon). The block would consist of links to pages, navigation levels, taxonomies, etc. Using the toolbar, users could set the justification, alignment, and width—same as with the Group block and its variants. The same settings would also be available in the Inspector. There'd be a new option for Users would go to Inspector to customize the block's appearance and behavior. When they enable "Show home link", the block will display a link to the home page and users will see an input field in the Inspector. It will allow them to customize the name of the home link. It'd default to the site name but users could enter anything, e.g., Users could click the ellipsis menu to access more settings, like the current page toggle and a navigation level slider. When the current page toggle is on, the block will display the name of the currently viewed page as the last breadcrumb level. It will not be active, so it wouldn't have the underline like other breadcrumbs. Users could change its appearance under The Users could change the separator in the Styles tab. Of all the dozens of websites I checked, all used one of the three options I included in the design. It could be a good starting point and if users ever need something else, they'd install a plugin that extends the block. I also included a toggle for displaying the leading separator and reworded the copy a bit, though I'd argue the pattern isn't common enough to be part of the block's default setup. Happy to keep it in the design if you feel it's valuable. Once again, users could change the separator color in the Color section. Let me know if you have any thoughts! |
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @QROkes, @jarekmorawski, @BrunoAHVincent, @rodolfomartinez. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Description
Related to #21943
This PR proposes adding in a new Breadcrumbs block, to render navigation links of the hierarchy of a page. This is a server rendered block, which would typically be used in a page template, most likely before or after the Post Title.
Features include
Some of these features are implemented in a subtle way so as to not be intrusive. To override the site title, once it's displayed as the first breadcrumb, a user can click into the site title and manually update it. Likewise, the separator character defaults to
/
but can be edited by clicking on it and replacing it with another character.Testing instructions
Repeat the above with a normal post, and use hierarchical post types.
If you wish to test the custom block spacing control, be sure to use an FSE theme, and set the
settings.spacing.blockGap
value totrue
in yourtheme.json
file (for more information on this see the testing instructions in a blockGap PR like #34630)Screenshots
2023-06-05.14.15.55.mp4