Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hi, are there any plans for Menu position porting to D8?
Comment | File | Size | Author |
---|---|---|---|
#34 | menu_position_d8_port-2215515-33.patch | 138.85 KB | BarisW |
Comments
Comment #1
JohnAlbinI'd love some help, but http://drupal.org/project/menu_block needs to be ported to D8 first.
Comment #2
netsensei CreditAttribution: netsensei commentedHi John,
I've been looking at this over the past days. It's a challenge to wrap my head around this, but I like that. :-) I've been digging a bit in D8 code.
In a nutshell: I think the basic logic of re-setting the active trail somewhere in the request lifetime based on sets of conditional rules, as MP does in D7, also applies to D8.
The hurdle I'm looking at right now: hook_page_delivery_callback_alter() is where the D7 magic happens, but does not exist in D8. So we need to take a look at a different strategy to somehow alter the active trail. With D8 being symfony based, my guess was that this should have become a lot easier to do.
At this point, I've been toying a bit with swapping D8 core services to get around the problem:
menu.active_trail
seems looks like a good spot to start: extend the class and override the getActiveLink() method. The comment in the body even hints at doing this if you want more control over what happens there.The D7 logic 'fakes' a different request path by altering menu preferred link and such to control the active menu item that sets the active trail. In D8, this has become a bit harder to control. The
$this->routeMatch
object in the class, as part of the new Routing API, is a bit unclear to me. It contains an implementation ofRouteMatchInterface (CurrentRouteMatch)
but it could also contain a custom implementation of that interface. That object wraps around the current activerequest
and uses that to fetch the active route used to set the active menu item.So, I'm now left pondering:
1/ Should I create a 'fake' request of sorts and pass that to
$this->routematch
object. Knowing that this might break other stuff further down the line?2/ Should I just do away way the existing
$this->routematch
object in thegetActiveLink()
method and go with my own implementation? This would contain the altering to my implementation ofMenuActiveTrailInterface
instead of opening a can of worms.The other part I don't fully understand after a day and a few evenings of tinkering: how does this affect the Menu Block D8 port?
Comment #3
rootworkCurious if there's been any progress on this. (I'm also not clear how menu_block would play into this, since it seems like its functionality is mostly already integrated into D8.)
@netsensei if you've been working on anything can you post it here or link to a repo somewhere? It'd be great to start testing something.
Comment #4
Lukas von BlarerI think that I'll be working on this in Barcelona. If anyone is interested to join me... Just drop me a line.
Comment #5
morenstratMaybe this blog post can help to get a starting point?
Comment #6
sachbearbeiter CreditAttribution: sachbearbeiter commentedComment #7
star-szrNot sure if anyone else is working on this at the moment but this will likely be what we work on at this sprint: http://www.meetup.com/londug/events/228399619/
Will post updates here.
Comment #8
star-szrI've ported two other D8 modules but both were smaller and one I had commit access to.
Some ideas on handling this, or maybe a combination of some of these ideas (and other ideas are welcome of course):
Comment #9
akalata CreditAttribution: akalata commentedI think separate issues works well, if we have somebody around with commit privileges. :)
If we don't hear back from a maintainer before the Global Sprint Weekend (I'll be happy to join you @cottser!), then Github will probably be the way to go.
Comment #10
BarisW CreditAttribution: BarisW at LimoenGroen for Gemeente Venlo commentedI'm happy to commit patches if they are RTBC'ed. If you can get them to RTBC as well at the sprint weekend, I'll commit them ASAP.
Comment #11
BarisW CreditAttribution: BarisW at LimoenGroen for Gemeente Venlo commentedJust added a 8.x-1.x branch for you to work on.
Comment #12
star-szr@BarisW perfect thanks! That sounds good :)
Comment #13
star-szrWe are just getting started but a repo has been set up here: https://github.com/LonDUG/menu_position
We will make patches and issues as we go.
Comment #14
star-szrSo that was fun, and exhausting. I've sprinted probably a few dozen or so times on core but sprinting on contrib (particularly a module port) is a very different beast.
We did create a few issues but most of the work is half-done on branches on the GitHub repo. We didn't get super far in one day but I can at least sketch out the architecture we came up with. Can't guarantee if/when this group will be able to work on the porting again.
Without further ado (feedback/suggestions/ideas definitely welcome, a lot of these APIs I'm not as familiar with):
config_entity
GitHub branch has a WIP).serviceAlter
GitHub branch.Note: Unfortunately this would also mean only one such active trail manipulation module (see also https://www.drupal.org/project/menu_trail_by_path and others) could be active at one time - I think there's a need in core or contrib for a "chained" menu active trail service that would look for tagged, weighted active trail services so you could handle more complex use cases and/or add your own active trail classes for special cases.
There's also the matter of breadcrumbs which we didn't really get to but theoretically it would work similar but with a breadcrumb builder (of which there can be multiple)
Some other things we somewhat figured out:
forms
branch).Comment #15
lbainbridge CreditAttribution: lbainbridge at Northern Commerce commentedAlright so @porchlight and I had some time while we were at DrupalCon to sprint on this and I think we made some really good progress. Everything we did get done is pushed to the 8.x-1.x branch on the repo @Cottser mentioned above (https://github.com/LonDUG/menu_position). The main thing we focused on was getting the config entity in place and finishing the forms for managing menu position rules.
I am going to create some issues for remaining work on the github repository, but much of what we have done is almost a total re-write, as the new concepts in D8 kind of changed the way we had to think about some of the items, things we finished/made progress on were:
Outstanding work:
We really tried to lay down the tracks for further work on this, take a look, create some issues if we've done something wrong (because we probably have) and let us know how we're doing.
Comment #16
MarcKwee CreditAttribution: MarcKwee commentedHi @lbainbridge & @Cottser,
I am working on your port of Drupal 8 Menu position. We require it for our first big Drupal 8 Project so I intend to finish it in the coming 2 weeks. Currently creating the plugin system. Would it be possible to gain Git access so I can also push it to a feature Branch?.
Cheers, keep up the outstanding work!
Comment #17
star-szr@MarcKwee same username on GitHub?
Comment #18
lbainbridge CreditAttribution: lbainbridge at Northern Commerce commented@MarcKwee awesome! @porchlight and have been working on integrating the condition type plugins that the block module uses instead of writing a completely new plugin type for this module to use, not sure if that will pan out or not yet though. Feel free to fork the repo and send PRs our way as well if you'd like as well.
Comment #19
lbainbridge CreditAttribution: lbainbridge at Northern Commerce commentedAlright, the module is almost useable, we could use some help in the issue queue though, anyone with some time to install it and provide some bug reports in the issue queue on github that would be greatly appreciated, or better yet fork the repo and provide some PRs. The main things outstanding are some caching issues, admin ui improvements and additional plugin definitions.
We still have a ways to go for documentation, testing and code style as well, so feedback is definitely welcome, as this is pretty much a ground-up re-write of the module for D8.
Comment #20
Lukas von BlarerGreat, I will do so in the next days. Thank you!
Comment #21
Lukas von BlarerI created the first issue: #2745295: Drupal\Component\Plugin\Exception\PluginNotFoundException: The "entity_bundle" plugin does not exist.
Comment #22
ckaotikI've also created an issue #2748341: MenuPositionLink is not compatible with MenuLinkContent. So far, other than these two issues, everything seems fine. Keep it up!
Comment #23
lbainbridge CreditAttribution: lbainbridge at Northern Commerce commentedThanks for creating those issues everyone! It's awesome to really see this starting to come together now.
We are starting to think about how we can get this over to drupal.org now. I think there are two ways we could go about this:
1. We could create a massive patch and post it in this issue for a maintainer to apply to the 8.x-1.x branch.
2. I would be happy to help maintain the D8 branch of this module, so if someone would like to make me a maintainer I could push up all the commits and not lose the commit history.
Let me know how I can help get this over, we're really excited about the work here.
Comment #24
BarisW CreditAttribution: BarisW at LimoenGroen for Gemeente Venlo commentedHi Luke,
I'd be happy to add you as co-maintainer, but I don't have permissions to do that.
Only John Albin can do that. So you could ask him, or I'll commit the huge patch.
Comment #25
Lukas von BlarerAnd another two:
#2753415: Deploying using config doesn't work
#2753423: Rule has to have a "Current theme" condition
Comment #26
j-vee CreditAttribution: j-vee as a volunteer commentedHi there!
Thanks everyone for your great work porting this module to D8.
I've been using it while developing my new D8 website and it's working nicely for the most part.
I do get errors though that require me to uninstall the whole module and re-install it and then create all my menu position rules again.
This is what is breaking the module for me:
1. Create a menu item
2. Create a menu item position rule that is attached to the menu item from step 1
3. Manually create a new menu item and attach it under the same menu item, the one created in step 1
When saving the menu item from step 3 I get this error:
Notice: Undefined index: menu_name in Drupal\Core\Menu\MenuTreeStorage->findParent() (line 576 of /PATHTOROOT/core/lib/Drupal/Core/Menu/MenuTreeStorage.php).
And after that straight away I get this warning in my log messages:
Warning: asort() expects parameter 1 to be array, null given in Drupal\Core\Menu\MenuTreeStorage->preSave() (line 369 of /PATHTOROOT/core/lib/Drupal/Core/Menu/MenuTreeStorage.php).
And the menu position rule created in Step 2 disappears from the menu item list.
Now trying to access the Menu Position Rule (
/admin/structure/menu-position
) settings at this point gives me this error:Drupal\Component\Plugin\Exception\PluginNotFoundException: Plugin ID '' was not found. in Drupal\Core\Menu\MenuLinkManager->getDefinition() (line 208 of /PATHTOROOT/core/lib/Drupal/Core/Menu/MenuLinkManager.php).
Cheers!
EDIT 1:
Now after uninstalling the module, I get the following error trying to access my main menu link items. It seems like the module doesn't get uninstalled cleanly:
EDIT 2:
Fixed the error in EDIT 1 by deleting all menu_position related database entries from the table
menu_tree
.Here's an example entry that was left there even after uninstalling the menu_position_link module:
Comment #27
akalata CreditAttribution: akalata commentedHi j-vee: You should check the issue queue for this, and if your issue isn't listed, add it there (or if it is listed, add this detailed info!). This is more of a meta/status issue.
Comment #28
akalata CreditAttribution: akalata commented#27 being said, what's the status of github vs. d.o? I'd like to work on this, but I'm not sure where to start since there are issues in both places.
Comment #29
star-szrHey @akalata I think the most up to date work is still on GitHub since I don't think anyone who has worked on the port has commit access on d.o quite yet.
Comment #30
JohnAlbinSo Luke and Baris are both maintainers now (with "add new maintainers" perms). Go! Go! Go! :-)
Thanks for all the hard work already done on this, everyone!
When it gets merged into the d.o. repository, make sure everyone's git commits match up with their drupal.org usernames, if you can, please.
Comment #31
lbainbridge CreditAttribution: lbainbridge at Northern Commerce commentedJust realized I was made a maintainer earlier this week, I will work on getting the work from github on d.o so that we can proceed with work in a single spot :).
Comment #32
Lukas von BlarerIt would be great to have the code on d.o. Could you push the status of the github repository here?
Comment #34
BarisW CreditAttribution: BarisW at LimoenGroen for Gemeente Venlo commentedSure, just did!
Comment #35
Lukas von BlarerGreat, thank you! Would be great if someone could go through the issues that need review, especially this one: #2745295: Drupal\Component\Plugin\Exception\PluginNotFoundException: The "entity_bundle" plugin does not exist.
Comment #36
ckaotikThat's awesome! How are your plans regarding the issues and pull requests on GitHub? Move them over to drupal.org? Should be doable considering it's only a few issues/PRs and the proposed patches are easily exported by appending
.patch
to the PR's url.Comment #37
rutiolmaIs there any major task preventing an alpha release? What can be done to help?
Comment #38
BarisW CreditAttribution: BarisW at LimoenGroen commentedI just committed #2745295: Drupal\Component\Plugin\Exception\PluginNotFoundException: The "entity_bundle" plugin does not exist.. If someone can work on #2808679: Importing config not working and provide a patch, I think we are ready for a first release.
Comment #39
BarisW CreditAttribution: BarisW at LimoenGroen commentedA D8 release is blocked by issue #2808679: Importing config not working.
We are so close!
Comment #40
Lukas von BlarerWhat is the status for a first release?
Comment #41
Lukas von BlarerIt would be great if we got #2760373: Provide an event to alter the rules in as well.
Comment #42
OlafskiThe blocker Importing config not working mentioned in #39 was fixed a while ago.
edit: URL corrected
Comment #43
Bwolf CreditAttribution: Bwolf commentedIt would be great to know when there will be a stable D8 release I have been using the D7 version for years.
This is a great module and I appreciate the work the maintainers have put into the project!
Cheers!
Comment #44
iancawthorne CreditAttribution: iancawthorne commentedDoes context_active_trail do everything that menu_position did as a Drupal 8 alternative? I've been using it on a few projects due to menu_position not having a release and it seems to cover all scenarios.
https://www.drupal.org/project/context_active_trail
Comment #45
ckaotikThanks for mentioning this module! From a first look, I'd say they have a lot in common. However, menu_position can actually insert the current page into the menu tree, so it shows up in menu blocks. Also, the way context_active_trail uses a different approach by using the context module, which not everyone might need.
So yes, they have a lot in common, but they target different use cases.
Comment #46
Mohammed J. RazemSo the alpha version for D8 released on 3 January 2019.
I'll go ahead and mark this as fixed.
Comment #47
Mohammed J. Razem