-
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
Home Link: remove ability to edit home link label #32677
Conversation
Size Change: -53 B (0%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
I like this, it simplifies what the block should do and avoid getting into confusing states. |
This is nice:
Edit: Actually the differential is because this block has its own classes — "wp-block-home-link" instead of "wp-block-navigation-link" for the list item, and "wp-block-home-link__content" instead of "wp-block-navigation-link__content" for the anchor. It then duplicates some of the CSS styles that govern the appearance of menu items. But because it does not duplicate them all, it doesn't work in all contexts. I understand why the styles are duplicated, it is a best practice. But in the case of the navigation block, menu items need to be able to inherit padding, colors, submenu colors, margins, have defaults for all of those, defaults that change when they are submenu items. If the Home Link block is not ever meant to be used outside of the navigation block, instead of duplicating the CSS, could we add additional CSS classes? So instead of:
we have this?
This would solve the problem entirely. |
Home link is currently not nestable, since it isn't a navigation link and we chose to keep it out of scope for an initial pass. I'm a little hesitant to add a different block class to this item. Would it be hard to duplicate styles in SCSS? |
Not hard, but the risk of drift with this block is very very high due to the complexity and amount of edgecases we have to account for. I understand the desire to keep concerns separated, but the added complexity and risk of drift in this case I'm not sure is worth it. I think of it this way: it is the responsibility of the navigation block how blocks inside are presented. It's for the same reason that Page List-as-a-navigation item styles are absorbed in the navigation container block, those styles aren't useful for the block when used outside of it. What other options do we have for keeping things in sync like this, which alleviate the duplicate class concern? Could we have a different more generic CSS class used across all menu items? |
This would certainly work. It probably depends on what other content we'll be adding to the navigation block. I think we might want to pull in a few other folks for thoughts, maybe cc @talldan @getdave @draganescu @tellthemachines |
As I was reading through this thread I was thinking "why don't we use a generic class that applies to all items that are children of nav"...and then I ready @jasmussen's comment:
I think this is probably the best solution. It would need to be low specificity. I wonder whether rather than require each block to have these classes hard coded, we could look to apply it dynamically via the |
Related: #31879 |
I realized that my feedback about the homelink class is unrelated to this particular PR, since the home link is already merged. In that vein, that feedback shouldn't be a blocker to this PR. We can perhaps extract it to a ticket and land this one? Still pretty important we find a solution that doesn't mean copying all the CSS. |
Sounds reasonable to me, I added a quick issue #33048 to capture the work. This PR still needs a ✅ before merge. |
Users should always be able to directly control any navigation item label. Maybe they want to change it to "Homepage," add a simple "🏠" emoji, or do a 1,000 other things we haven't thought of. Just output the default "Home" text and let users decide if they want to change it. |
Hah, I missed your last comment @justintadlock. The main concern with the UX is it can lure people to rename it by mistake (or because the theme added a label) and then find themselves with an item that has a hardcoded URL yet is indistinguishable from other editable links. An icon button is a good use case — in the original issue for the block there's a mention to add support for showing it as an icon. I think it's also possible that we don't need this block at all and instead "Home" should be something available to choose for any link item through the add / edit link flow. In any case, it'd be good to see how people interact with it: making the label editable later is easier than removing the ability. |
The problem with that route is that it is a regression, taking away a feature that users have had for years. Users already have the ability to edit the Home label today via the Navigation block, and they have the ability to edit it via the old Navigation Menus system that exists in core. I already know of several of my theme users who create menus like: 🏠 Home 👩 About Me ✍️ Writing 📧 Contact With this change, the only way they will be able to do that is via CSS, which is the opposite direction that we want to be pushing users. Or, a custom link, which defeats the purpose of a dedicated Home link. Plus, they lose the nicer image-based emoji with CSS. Other users use Font Awesome icons (there's a plugin for inserting the icons into RichText) instead of emoji. |
This is definitely possible. The only problem would be when we show this Home link option in the Link UI search results. As a user would I click I think this problem is actually why the Nav Link block currently asks users to choose from a plethora of link types before they can actually add their link. We're trying to make it super obvious how you get to choose a specific type of link (Post, Page, Category, Home...etc). Whilst this provides nice context for developers, I don't believe it's particularly intuitive and (IMHO) it's getting worse. I feel this whole problem space needs some thorough design review and direction. If folks feel it would be valuable I could look into opening an overview Issue. |
What happens with translations? |
Incorrect or missing translations is a valid use case and the block has been available for a while 👍 Let's close out this PR We can revisit functionality/the need for this block if Navigation flows change. |
"Home" is a convenience block, you can always just insert a regular link item pointing to the home. |
To make the Home Link block more straightfoward to use, changes here remove the ability to provide custom label text to the Home Link. Note that since this is a dynamic block and we're removing an attribute, it doesn't look like we need to add a deprecated version. (Let me know if we still should do this).
Behavior Changes:
Testing Instructions:
Before:
before.mp4
After:
after.mp4