Block Library: Add home link block #30926
Conversation
|
Size Change: +554 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
Looking through wpdirectory search results
And in one theme I saw it used as a "back to home" link on a 404 page Do we think that there will be a separate breadcrumb block, or perhaps the navigation block serves as breadcrumbs some time in the future?
Not being able to edit the url sounds reasonable to me. |
|
This is so cool, thanks for working on this! As a quick test, even in this WIP state, looks good:
For when the link is inside navigation, we'll likely need the
To your questions:
Carolina shows some excellent use cases. But I think for this block it's probably best to start with it as Navigation-exclusive only. It should be easy to graduate it to work beyond that one, but it's hard to go the other way.
The question surfaces an added complexity. I recall seeing, somewhere in the github archives, a mockup by @jameskoster which lets you select which page is the home page, and which is the blog page, either in the top centered dropdown or the left admin navigation rail. All that is to say that "home" and "blog", in those situations, are properties of the page. Here, it's a separate block. Which while definitely useful, it immediately makes me think of the page list block, which ideally can also support the "home" classnames. In that vein, and purely out of speculation: could the "home" aspect be a variation of a menu item, or a property that is set elsewhere?
Fine to disable! |
| const { label } = attributes; | ||
|
|
||
| return ( | ||
| <li { ...blockProps }> |
There was a problem hiding this comment.
Naming: is core/home okay, or do we prefer something else?
At the moment the implementation isn't very general (because of li wrapper), so core/home doesn't seem to be a great fit.
There was a problem hiding this comment.
I didn't show it in this branch, but we could update the behavior such that the li wrapper is only added when the parent is a navigation block. This is similar to what the PageList does
There was a problem hiding this comment.
Interesting idea to offer an automatic wrapper element.
We also have Site Title and Site Logo blocks that both link to the homepage. Would the make sense to be added to the Navigation block. I saw a demo from @mtias where he added the Search Form block to the Navigation block so I would assume that Site Title and Site Logo could be used as well. I'm mostly sharing because there is so many possibilities to take into account 😅 |
|
From initial feedback here, I suspect we don't have a good enough use case for a brand new block here. Logic would initially be heavily tied to matching navigation links, which might end up being pretty confusing. I've spun up #30975 as an alternative using a navigation link variation. Let me know what folks think, especially if y'all feel strongly if this should be a new block.
@carolinan thanks for the use case examples. I personally think a breadcrumb block would be pretty handy for folks (and I suspect it'd likely would be implemented separately from the navigation block). I'm not sure if there are current plans for adding a breadcrumb as a new core block.
Good spot @gziolo. 🤔 Would it make sense then that a home link is some type of style variation of site title?
Great feedback @jasmussen |
Just in navigation is fine. The breadcrumb case is a nice one, but I think a specific block for that would be preferred.
No, I'd keep it simple and shallow.
Correct. No UI needed for editing the url. At most we might want to deep link to permalink settings and such, or to the home template if it's a block theme. |
|
|
Leaving a reminder note to myself to update the PageList -> NavigationLinks conversion to convert to a HomeLink block when we detect this case. (It's probably be fine to do as a follow up PR too). |
gwwar
changed the title
|
I was digging into the Navigation Block markup a bit and two things stick out: We have invalid markup for social links, search, and spacerEach of these need to be wrapped by an li tag. Folks can use https://validator.w3.org/#validate_by_input to verify this. I think one way to tidy this up would be to let the Navigation block itself render the top li for each child. Edit: looks like there's #29036 for this There's a lot of duplicated classes applied to navigation link (and likely needed for home link).For things like font/color selections on the nav block we add a number classes like I can simply copy paste this for the home link, but I wonder if we can reduce this.
|
I would be happy to collaborate on this if you like, or to just volunteer to fix any CSS inheritance of the nav block that needs to be done if something breaks in a simplification. |
Your commits are always welcome @jasmussen ✨ |
gwwar
requested review from
ajitbohra,
nerrad,
ntwb,
spacedmonkey and
TimothyBJacobs
as code owners
|
This one is ready for review |
There was a problem hiding this comment.
This is looking like a great start to me. Thanks for all the explorations, even if we ended up on something relatively minimalist:
I'd encourage a code double-check, but since there's already been a few, I'll also let your confidence level press the green button when the time comes.
Did a quick exploration. My first instinct was to have an option directly in the home blocks block toolbar. But then I realized that the toggling on/off of bold/italic options might feel weird, and that we might want 3 options making it less of a toggle and more like a segmented control. Here's a toggle:
Here's a version with a segmented control:
Any preferences or feedback? Then I'll open a ticket. |
@jasmussen The segmented control looks great. If folks choose an icon only, we'd maybe need to allow folks to edit alt-text. I think I addressed noted feedback. It looks like to move this very simplified version forward, we'll want a +1 code review. Does that sound about right? |
Would be a bonus, yes. But I would also apply the law of diminishing returns. I.e. a bit of silence means ship it. |
|
Thanks y'all for the reviews ✨ |
This adds a new dynamic block that renders a link, backed by
home_url(). Fixes #20542We're keeping this block simple to start. There's no url to edit, and subnesting of navigation links is not implemented.
We're also deferring linking to Reader Options (to modify home page settings) in the block settings panel for a follow up PR. We'll need a generalized approach here to allow folks to override these links with a new filter or slot/fill pattern.
There's a decent amount of copy pasted code needed for color/font support from the navigation parent. I do think this can and should be reduced.
For now, I opted to choose a similar implementation as navigation-link for now, while we consider options for what Navigation Block markup might look like. (See #29036 )
homelink.mp4
Testing Instructions
Current work here is a very early WIP, but before I get too deep I have a few questions. We currently favor implementing this as a new block, rather than a navigation link variation:Should we support this only in Navigation? Or are there use cases for adding this outside of the Navigation block, similar to PageList?A: Only in navigation to startShould we support child nesting of NavigationLinks inside of a home block? This will likely duplicate logic and complicate the block.A: Keep it shallowNaming: isA:core/homeokay, or do we prefer something else?core/home-linkThe user shouldn't have to manually update the url, as such do we still need to display a link control UI? The only piece they may want to toggle is if the link opens in a new tab.A: No need for link UIFolks are welcome to jump directly onto this branch if you'd like to collaborate. Just leave a comment or DM me so I'll know to a give heads up before rebasing.