-
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
Add filter allow modification of navigation block menu items #37717
Comments
Hey Andy, may I ask why you don't want to create a login/logout block instead? If you use a filter, users won't see the login/logout item in the Editor nor in the Previews, and they won't be able to modify it from the Editor (change its settings, change the texts, reorder it, place it outside of a menu, and so on). |
A similar block already exists, but it might not be integrated with the Navigation block: https://github.com/WordPress/gutenberg/tree/trunk/packages/block-library/src/loginout. |
@senadir I wonder if there's some related similarity to this request and an extensibility pattern we added for Cart and Checkout blocks that allows extension authors to dynamically inject blocks without user interaction but still allow subsequent manipulation in the editor after the fact? If so, might be worth pointing to the code here as a potential pattern to be considered for GB? I see the potential need for this for some subset of plugin or extension capabilities where it introduces more friction for end-users to have to go and edit places where they want that plugin's functionality to be injected vs automatically added on plugin activation. Having some sort of dynamic block injection interface for plugin developers, allows for extending the default behaviour of core blocks that are extensible. |
Yeah I think the issue isn't about this being a block or not, but about the process of manually adding the block vs it being there. For the pattern we had in Cart and Checkout, I can't see how it would work here without significant changes to how the navigation block work. Cart and Checkout are both dynamic blocks (uses React), the solution is we implemented is to create a new registry in the frontend with a new function AFAIK, the navigation block is a PHP based block, so whatever system there is, it has to be on PHP Now, there are another questions to arise:
For, us frontend rendering function would take this HTML <div class="wc-checkout">
<div class="wc-checkout-form" />
</div> To this: <CheckoutBlock>
<Form />
</CheckoutBlock> and when injecting in I would love if we can move our forced rendering APIs to Gutenberg, but a prerequisite would be to figure out how it fits with other ways of saving/rendering blocks. |
If the problem is just to avoid the end-user friction of adding blocks manually but exposing the block in the Editor still leads to a better user experience (change its settings, change the texts, reorder it...), maybe instead of trying to figure out APIs to modify the HTML output of a block, we should try to figure out APIs to add new blocks to the templates. |
The function I created in a plugin simply adds a login/logout to the last item in the current menu. Installing and activating a simple plugin is much easier than having to touch every menu. It also allows me to use the same plugin for traditional themes or block themes. I'm not looking to create a new block. As referenced above I think such a block exists. I really just want to add a new list item to the menu programmatically. This is easily done for non-block themes but impossible for block themes. Seems like a regression. I've looked through all the hooks but I don't see a way to accomplish this. I appreciate all the comments. I don't really see the need for the user to be able to edit the injected menu item. They can simply turn it off by deactivating the plugin. |
I wanted a single purpose plugin to add a login/logout menu to the end of the existing menu automatically. No user input required. |
Ok, so it seems like @nerrad was right. Your concern is the user friction of adding that item manually. Thanks for your answer, Andy. This is very helpful 🙂 |
I think I found a better place to create a simple filter that can accomplish the purpose. PR coming. |
This will be my first PR to Gutenberg. The change will be in the navigation.php file, but where do I make the change for the PR? |
New PR, WordPress/wordpress-develop#2168 |
I have some concerns with the filter proposal. As mentioned by others, it creates a dissonance in that it allows items to be added to a menu, but those items are not present or editable in the block editor. I think that would lead to a confusing experience for users. It also doesn't support the features that the block editor offers. What if the user wants to add the Login item in a different position? Does the item support all the styling options that other blocks do and can the user see the results of those styling options in the editor? From a technical point of view, another concern is that this is an API that's useful for only a single block. If we continue down this path, every block ends up with unique and inconsistent APIs. The goal should be to provide consistency for all blocks in this respect. Apologies if this comment sounds overly negative, the input and effort is really appreciated. It's probably a bit early to jump straight to a solution for this, as I feel like there isn't a strong shared understanding of what the problem is yet. I think it'd be good have a talk about the options available first. I also don't see many of the regular navigation block contributors active in this discussion yet, so I think it'd be great to get some involved. I'll ping a few (@getdave @tellthemachines @jasmussen @spacedmonkey @adamziel). |
@talldan to answer some of your concerns.
These items are meant to be added programmatically by a plugin. The user installs the plugin, it shouldn't be confusing where the additional menu item came from or how to remove it.
From the
All blocks are not the same. This is specifically for a navigation/menu block. WordPress has always been extensible via action and filter hooks. Are blocks denied this extensibility?
Don't worry about sounding negative. I'm used to contributing to core where problems and solutions are often presented at the same time. I have looked through the code and while this may not be the best place for a filter to be able to accomplish the task of modifying a navigation menu items it seems to be. Also, there is no current method to accomplish this. I have a POC plugin if anyone cares to test. https://github.com/afragen/login-logout-primary-menu-item Fundamentally, hooks and using them correctly and responsibly, is up to the developer. To deny that a hook should exist, especially when a valid user concern has been presented, just seems counter to the extensibility that we try to build into core.
I am always open to answering questions and concerns. Thanks, I do appreciate any an all attention to this issue. |
That's only half the story. If the item doesn't show in the editor, the user has a harder time getting their menu to look as they want it to because there's always something missing. That also has knock-on effects with the entire layout of the page.
I'd counter by saying that most blocks work the same way. There are very few distinguishing factors in how blocks work, static vs dynamic being probably the biggest distinction between blocks. Looking at the proposal, there's nothing that technically precludes other blocks from having a similar API. The reason this is being developed only for the navigation block is that this is the only proposed use case, and that's based on a very tiny amount of data. The challenge though is to think about the entire block ecosystem and try to provide a consistent developer experience.
To some degree, yes. The WordPress backwards compatibility clause does mean new APIs have to be very carefully considered because they have to be supported indefinitely.
There's no denial here, just sharing concerns and also strongly recommending that alternative options are explored to see if those concerns can be addressed. |
@afragen Firstly thank you for raising this Issue and generating a good discussion about extensibility of the Navigation block. I can completely understand the use case you are presenting. It is very valid, especially when considering it from the perspective of classic Themes. I've devised a couple of user stories that might capture the requirements below - let me know if this works:
Some questions and concernsWhilst I follow the use case you present, I also share others' concerns regarding not exposing a UI within the editor to allow the user to directly manipulate these blocks. As you are no doubt aware, the block editor strives for a direct manipulation paradigm whereby whatever you see in the editor matches what is rendered on the front of the site. This is often quite different from the "classic" approach and can therefore cause dissonance when trying to re-create existing APIs within a block based context.
One of my concerns surrounds what you said above. Whilst the new items will technically support all the features of a "normal" [directly manipulated] block, how would we expose these to the user? Presumably as we will not render these items within the editor, we will need to create some kind of Consider for example, that the user may not have been the one who activated the Plugin. Now some items have suddenly appeared in all menus across the entire site. When they go to the editor to review all the menus however, these items are not present. This could be a disorientating experience and one which I think we should strive to avoid. Key requirements / (possible) solutionThe key requirements here appear to be:
I wonder whether we could devise an API which would allow items to be dynamically appended to blocks on the front end (of the site) but also concurrently insert matching items into the Editor. That way we avoid making it a requirement to re-save all the menus in order to add the links, but still expose these items within the editor for direct manipulation on a case by case basis if required. I don't want to dive into the technical aspects of achieving that right now as we don't have a consensus on whether it's a good approach. But hypothetically this proposal would seem to be the best of both worlds. I appreciate however this is very high level and idealistic - let's see what folks think. Thanks again for raising this and let's continue to iterate to see if - between us all - we can devise a good solution. |
The question I hear everyone asking is: Is Gutenberg a WYSIWYG editor? I really want it to be one. To me, it can only happen when the website shows the same thing as the editor. I am concerned about adding an API that can alter one, but not the other. It's like changing the output (website) without changing the input (editor markup). More tactically, I imagine this problem as two parts:
The existing block menus are a markup stored in the database. Perhaps the plugin could update it on activation? The future menus will start in the block editor. Perhaps the initial block could already contain a logout button, giving the user a chance to remove it if they wish so? Would one of the |
Another thing to consider is that in this specific case, a user is unlikely to want the login button added to every menu. @afragen Did you have a way to control that previously with your plugin? |
Many plugins filled this gap for a login/logout link using the wp_nav_menu_objects filter. The new block nav doesn't apply this filter so people are left with unfunctional login/logout links in their menu when switching to a block based theme. Maybe developers don't need to filter block based navs like they used to the old PHP menu items. I think that's a worrisome trend, but even if you feel that way, the current 5.9 version of WP is in this in-between place where you can edit the menu in FSE or the old menu interface, or even the customizer. If those 3 ways of editing a menu aren't compatible, plugin developers are left to write more code, docs, etc to help users through the confusion. Assuming (a) we don't want to map old menu features/filters 1-to-1 to the new FSE nav blocks, I'd suggest at least making sure the loginout block works inside the nav block. This is the work around that we would like to suggest to users. Currently in 5.9, I can't add the loginout block inside of a menu. I can only add it outside, and it's clunky to do that in Twenty Twenty Two. |
If the user doesn't like how a plugin that uses this proposed filter looks or works, all they have to do is deactivate it.
I would argue that more targeted filters would make it easier for developers to target and therefore easier to debug providing a better developer experience. At the moment, I feel that not having a method to interact with a menu's items programmatically to be a regression in the block pattern from the non-block pattern. |
@getdave you are very welcome.
Exactly.
The examples and code that I use is supposed to be as simple as possible but at the same time integrate within the new navigation block. If a user happens upon my plugin, which is only on GitHub, the choice they have is to have it active or not. Because of the way I add the menu item any styling to the menu, as a whole, is picked up by the menu item.
"With great power comes great responsibility." This has always been the motto of the plugin developer. If a user of the site has control over the layout but doesn't have control over other aspects of the site they should talk to the person who has that control. We can't solve all problems in code. We can only strive to make some things easier to accomplish.
I respectfully disagree about the need to always retain the direct manipulation paradigm within the editor. In this instance the only direct manipulation needed is whether or not to have a plugin that uses the proposed filter active or not. From my limited understanding of how block elements are stored, and I believe that this is within the database, I think it's a really bad idea to encourage more direct manipulation of the database. Something like that isn't reversible by simply deactivating a plugin. |
Again, in my example, removing the additional menu item is a simple matter of deactivating the plugin that adds it. The filter you describe above is only useful if I'm developing a block plugin, I'm not. As I've said I think encouraging plugin developers to directly manipulate the database is a bad idea. The plugin would then "persist" even if it were deactivated. |
Deactivate the plugin, it's probably not for them. Alternately it could be coded to only add it if the menu contains certain items. Like if the first menu item is a Home link. All this data is available within the proposed filter. It's simply a matter to design the plugin to only add the additional menu item when a specific set of criteria are met. |
Actually they are left with no login/logout links. |
I don’t have a lot to add to @getdave’s comment which I think sums up the problem pretty brilliantly (and which also pretty much reflects my opinion on the matter) I’d just like to express that if I were in your shoes @afragen I’d probably be somewhat frustrated that a fairly small request for an additional filter is met with such strong opposition. What I hope to do with this comment is to put this into some perspective: As @getdave and others have pointed out, the editor-frontend parity (the direct editing paradigm) is rather paramount to FSE. Now it’s of course arguable that given the benefit of having the login/out link added upon plugin activation, we might consider this ease-of-use to outweigh the benefits of direct editing and thus just add the filter to extend the nav menu. It has been brought up that for a user who sees the login/out link on the frontend it will then be harder to figure out where that link is coming from, since it’s not visible in the editor. One might counter that this is no different from the pre-FSE situation, where there was no indication in the post editor (or anywhere other than the plugins menu really) where that link was coming from. I’d say however that FSE makes all the difference: We now have direct manipulation, plus the block paradigm. While activating a plugin used to be the most straight-forward way to achieve a certain effect (and to many of us still is), I’d argue that this is changing. More on that in a moment. IMO, one of the strongest arguments against adding a new filter to a block in order to retain a familiar pre-FSE behavior is that at scale, it takes the project into the wrong direction: There are probably a lot of plugins that make similarly small straight-forward changes to parts of the markup and would thus in an FSE world need filters added in various places. While this is certainly possible, it goes against the principle of parsimony that AFAIU WordPress has historically employed when adding new extension points. Furthermore, the issue of the editor not reflecting the frontend is also much exacerbated as scale: If a WP install happens to have multiple such plugins installed, the frontend could look considerably different from what is shown (and can be changed) in the editor. All in all, we would trade an opportunity for a User Experience that fits in with block and FSE paradigms and a deliberate code architecture that supports it for a plethora of new filters added across the codebase. I thus agree that a login/out block is the preferable solution here; and that the same will likely apply to a host of other plugins. I’m confident that Gutenberg (and now FSE) is already spurring the community to provide block implementations of pre-FSE plugins. Crucially, we must make sure that User Experience doesn’t suffer, compared to activating a plugin. On the bright side, we already have a few good building blocks in place:
Based on the above, the User Experience that we can get right now is already pretty close to what I think we want to achieve: The only thing that’s apparently missing is being able to add the login/out block to the nav menu rather than just next to it (as @gziolo noted); but that should be something we can fix. Conclusion. Adding functionality to a WordPress site might work differently in an FSE world than enabling a plugin. Using blocks as the main user interface will however allow more direct manipulation and easier customization. Thanks to the familiarity of the now-pervasive block paradigm, it will soon feel even more intuitive. Translating other plugins’ functionality into a counterpart block will likely look similar to the example of the login/out plugin above: We’ll probably be able to use some pieces that we already have, and will need to get a bit more creative about some other ones. FWIW, a few of us will focus on exactly this kind of problem in the next couple of weeks. |
Ok I've gone ahead and created an Issue to track need to add login/out block to Nav block. |
Here's a follow-up spec of sorts that details what kind of extension mechanism could be implemented in order to retain a similar User Experience as offered by the legacy hooks (i.e. auto-append Login/out block upon plugin activation): #39439 |
I do appreciate the efforts though it seems more complicated than my suggestion on one simple filter. |
Having read @ockham's proposal I'm broadly in favour as it addresses both the original concern (ability to append an item(s) upon Plugin activation) and those raised by others (retaining direct manipulation paradigm). My understanding is that your Plugin would hook into a single filter which would
Now by default if you disable the Plugin the Nav items disappear from front end and the editor. However you can optionally choose to persist the ghost item indefinitely if you so wish. I agree the implementation seems complex but provided there's a good API this shouldn't make Plugin developers' lives any harder. Perhaps we need to see a PR exploring this approach in order to be able to make an informed assessment. |
Sharing my perspective here — the proper solution to the problem needs to be isomorphic, as discussed more broadly in #39439. However, supporting one-off server filters that only modify front-end output seems ok to have, as there will be some valid cases where it's ok to not reflect on the editor what happens on the front-end. It should just not be promoted as the solution to the problem, but it's one tool available. |
@mtias are there plans for this filter to be added anytime soon. The PR is complete and it is fairly straight forward. It would be at least nice to have until the proper solution can be determined. |
Yes, it sounds like a good idea. There are already block-specific filters included in the Navigation block and it is the most complex one 👍🏻 |
What problem does this address?
In non-block themes the
wp_nav_menu_items
filter allows the dev to manipulate the displayed menu. I have a simple plugin that uses this filter in non-block themes to add a login/logout menu item dynamically to any menu.No such filter exists for block themes. This is not about manually adding another item into the navigation block. It's about doing it dynamically.
What is your proposed solution?
See: https://core.trac.wordpress.org/ticket/54684
I thought a filter in the
WP_Block_List
class constructor would work, and it does in a POC. I have looked into the other thoughts that @hellofromtonya has graciously provided, but they do not allow me to actually manipulate the menu items programatically.Another thought might be to add a filter to
block-library/blocks/navigation.php
to each place where$inner_blocks
is created by callingnew WP_Block_List()
The text was updated successfully, but these errors were encountered: