Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The menu/routing topic on api.drupal.org is woefully lame. It is filled with @todo instead of the information that should be there.
https://api.drupal.org/api/drupal/core!includes!menu.inc/group/menu/8
Comment | File | Size | Author |
---|---|---|---|
#7 | interdiff.txt | 8.01 KB | jhodgdon |
#7 | 2290129-menu-routing-docs-7.patch | 21.39 KB | jhodgdon |
Comments
Comment #1
jhodgdonTime to write some more docs...
Comment #2
jhodgdonHere's an initial patch. Hopefully it's at least better than the horrible thing that is on that page now (which is a bunch of left-over D7 docs mostly), and hopefully reviewers will find and gently point out the typographical and factual errors.
I've left a few To Dos in the file. I was asking pwolanin and timplunkett in IRC about the hooks and derivatives, and they said it's still in flux. So I'm adding the "revisit before release" tag to this issue (or else we should plan to open a separate issue after this one is done for now; I'm not sure what the usual policy is on that?).
Comment #3
joachim CreditAttribution: joachim commentedLooks very good! (I wish there'd been overviews like this a year ago!!!)
Just a few things:
Is that line meant to look like that?
Should we mention that this works because 'block' is the name of an entity type? Also, how does the entity system register that it can deal with wildcards?
I gather that the menu links system is probably about to change substantially, but best to go ahead and commit this so what's currently there is documented, and update it later.
Comment #4
jhodgdonThanks for the review!
(1) was a copy/paste error, whoops.
(2) good point, but I don't think I want to get into it in this topic... so took out even more of the text and pointed to the Entity API docs. I also added a new section to those docs about routing.
Comment #5
Crell CreditAttribution: Crell commentedRequirements are primarily for adding regexes to restrict the route parameters. We've usurped it for other stuff like permissions. That should be mentioned here too.
Nowhere else do we call these wildcards, because that's not really the right term. "Placeholder" is what I've been using most.
Returning an HtmlFragment object is also supported. Actually so is an HtmlPage object, or a Response.
Note that if no weight is specified or 2 routes have the same weight they're then sorted alphabetically by...? (I'm not actually sure what it is right now.)
Comment #6
Gábor HojtsyGenerally a good one, some points:
May also return a 403 if you want to mix in that the routes check access too :)
I think it may be a mis-statement that menu is based on the routing system, the menu system uses routes but not really "based" on it I guess. Not in a sense of how the routes are based on Symfony's routing at least.
Paste error?
If you want to list these, then listing _controller would be also key.
Not sure explaining it like a group is a good idea. Contextual links require a group defined from PHP. It is true local tasks are not shown if there is not at least 2 on the same level to be shown. Not sure documenting it as a 'group' is a good idea. It sounds confusing vs. contextual links.
Worth mentioning that if the base_route == route_name then its the default tab to be shown on the route on the given level.
Ha, I did not know. Updated https://www.drupal.org/node/2122253/revisions/view/7366961/7383453 :)
Good point, documented at https://www.drupal.org/node/2133247/revisions/view/7366953/7383463
This does not actually exist at least in the docs.
Comment #7
jhodgdonThanks for the reviews! I think I've fixed most/all of the points from #5 and #6. A few notes:
#5 - point 4 - I don't know what the sorting is either and it is most likely in flux, so I'd prefer not to even mention it for now.
#6 - point 3 - was fixed in previous patch. I think you reviewed #2?
#6 - point 9 - typo on my part, this should have said hook_contextual_links_alter().
Comment #8
jhodgdonOh weird. The patch I uploaded in #4 didn't show up at the top of this issue in Files. ?!? Strange. It didn't get run through the testbot either. No wonder you might have reviewed the patch in #2 instead! Anyway. The latest interdiff is from the patch in #4, and comment #4 has an interdiff to the patch in #2 in case that is what you reviewed.
Comment #9
webchickThis looks great to me, and seems like all of Gábor's feedback was addressed. Going to err on the side of getting this in sooner than later, given it's one of the primary APIs that module developers use and the existing info is "woefully lame." ;)
Committed and pushed to 8.x. Thanks!
Comment #12
ianthomas_ukrevisit before release candidate is a more appropriate tag. The tag is needed because the system wasn't finalised when the documentation was written.
Comment #13
webchickThis ended up getting completely revisited and re-written at https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Menu%21me... so I believe we can remove this tag now.