Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Updated: Comment #0
Problem/Motivation
After #2084197: Uninstalling menu module removes all custom menus current menu module goes more menu_ui module because implements only List and Form controllers for menu and menu link entities.
Proposed resolution
rename menu module to menu_ui module according view_ui module
Remaining tasks
wait for #2084197: Uninstalling menu module removes all custom menus
User interface changes
some strings
API changes
no
Related Issues
#2084197: Uninstalling menu module removes all custom menus
#1882552: Deprecate menu_list_system_menus() and menu_ui_get_menus()
#2030645: Expand Menu with methods
Comment | File | Size | Author |
---|---|---|---|
#66 | interdiff.txt | 0 bytes | dawehner |
#66 | menu_ui-2085243-66.patch | 65.25 KB | dawehner |
#65 | menu_ui-2085243-65.patch | 66.96 KB | dawehner |
#62 | menu_ui-2085243-62.patch | 65.1 KB | dawehner |
#59 | menu_ui-2085243-59.patch | 67.2 KB | dawehner |
Comments
Comment #1
tim.plunkett#2083649: Rename action module to action_ui is similar
Comment #2
jibranSo what about menu link module. Will it be renamed to menu module?
Comment #3
tim.plunkettNo, because it doesn't provide the menu entity, system does. Unless we moved both menu and menu_link to menu...
Comment #4
amateescu CreditAttribution: amateescu commentedThat's what I proposed some months ago (in a IRC conversation, I think), so yes, I'm definitely in favor of doing that :)
Comment #5
dawehnerWhat a horrible idea to try that conversion instead of getting to bed. Don't expect a lot but at least core installs.
Comment #7
tim.plunkettTagging.
Comment #8
tim.plunkettComment #9
webchickI can't see a way that these and other module rename patches get us to beta or release faster. IMO this was an API freeze deadline type of patch that we missed, and therefore should be moved to D9. Not doing so yet though so we could discuss it if I'm wildly off-base.
Comment #10
webchickTalked about this with Alexpott. He says since the name was already like this in D6 and D7, we should not attempt to do this in D8.
Comment #11
sunComing from #2207893: Convert menu tree building to a service.
It's a little weird to have
In addition, #2199633: Menu Link module is marked as required, but should not be required
The only reason for why menu_link is a module appears to be that Drupal\Core components are not able to define/expose config/content entities at this point — or much rather, it's trivial to define an entity in a core component, but the component is not able to participate in the hook system right now, which means that e.g. configuration import/export callbacks/hooks are not triggered.
OTOH, @dawehner raised a good point in that menu + menu link (tree) functionality is not actually required for a Drupal site. For example, a lightweight Drupal site could simply output a static list of links in a theme template — no need for a heavy-weight menu link management. In light of that, keeping this functionality in a module might actually make sense.
However, from an end-user/UX, developer/DX, as well as themer/TX perspective, the current constellation of modules + code fragments definitely makes no sense at all.
The following would be much more sane:
That way, we have
Tentatively moving back to D8, as I hope that people agree that we can't really leave things in the current very weird state.
Comment #12
andypostProper tag.
There's nothing except menu link entity is bound to module names
Comment #13
adevms CreditAttribution: adevms commentedComment #14
adevms CreditAttribution: adevms commentedComment #15
dawehnerLet's try it out.
Comment #17
dawehnerComment #19
dawehnerThis could be green.
Comment #21
sunThanks for kick-starting this, @dawehner!
Most likely, the remaining test failures are caused by the oversights + typos in the routing file...? :)
Comment #22
dawehnerLet's hope
Comment #24
dawehnerMaybe now.
Comment #25
dawehnerChange notice: https://drupal.org/node/2216585
Comment #26
tim.plunkettThanks!
Comment #28
dawehnerReroll
Comment #30
dawehnerThis should be it.
Comment #31
jibranback to RTBC.
Comment #32
tstoecklerLooks great. Re-titling for the new approach.
Comment #33
andypostLeaving rtbc for approval
Comment #34
dawehnerI don't see what need to change an the issue summary
That is the solution. simple.
Change notice: have a look at previous comments; https://drupal.org/node/2216585
Comment #36
dawehnerRErole.
Comment #37
sunComment #38
dawehner.
Comment #39
Landish CreditAttribution: Landish commented30: menu_ui-2085243-30.patch queued for re-testing.
Comment #41
dcrocks CreditAttribution: dcrocks commentedCloned current D8 and did 'git apply' of patch in #36. Apply was clean except for whitespace warnings and install went successfully. Added/modified/deleted menus and menu links with no problems. However when I went to admin/structure/menus/setting I got
Comment #42
dcrocks CreditAttribution: dcrocks commentedError located in core/modules/menu_ui/menu_ui.routing.yml
'menu' should be 'menu_ui'. Tested change and worked successfully.
Comment #43
dawehner@dcrocks
Thank you for pointing that out!
Comment #44
jibranback to RTBC.
Comment #46
dcrocks CreditAttribution: dcrocks commented#2120871: Rename EntityListController to EntityListBuilder renamed MenuListController.php to MenuListBuilder.php
Comment #47
dcrocks CreditAttribution: dcrocks commented#2226903: Step 1: Move static menu links to yml files also adds a file to the menu module. There a lot of active issues in this area that will obviously be seriously impacted by the patch. Should this fix get in asap to avoid playing constant catch-up and allow all the other issues to get this sorted out, or should it wait a bit until the dust settles?
Comment #48
webchickI still don't understand why we are doing this at all at this point. Did alexpott sign off?
Comment #49
sunDoes the reasoning in #11 help?
Comment #50
dawehnerEveryone should be able to work on whatever they want, fine if it does not get in. But seriously I don't want permissions
on what I can/should work on.
Comment #51
dawehnerRerolled.
Comment #53
dawehnerThere we go.
Comment #55
dawehnerFixed some failures.
Comment #57
dawehnerReroll
Comment #59
dawehnerThere we go.
Comment #60
vijaycs85nice change. +1.
Comment #61
sunan .orig + .rej file slipped into this patch ;)
one more :)
Comment #62
dawehnerNo real idea what happened here.
Comment #63
dawehner62: menu_ui-2085243-62.patch queued for re-testing.
Comment #65
dawehnerYet another reroll.
Comment #66
dawehnerThis had the rej/orig files in there
Comment #67
dawehnerOpened a followup: #2239857: Move menu.api.php into menu_link module
Comment #68
tim.plunkett+204, -204
Everything looks good, and tests still pass.
Thanks @dawehner!
Comment #69
alexpottCommitted 07f63fe and pushed to 8.x. Thanks!
Since this is confusing people thinking that the menu module is necessary for menu's I think this is a good idea - it is conforming to the field/field_ui & views/views_ui pattern.
Comment #73
alexpottHad to revert because #1366020: Overhaul SearchQuery; make search redirects use GET query params for keywords sneaked in in the same commit... but did an immediate recommit.
Comment #74
jibranWell @dawehner got double commit credit for such an awesome effort I am really happy with that.
Comment #75
dawehnerThank you alex for letting this in, despite the resistance earlier!
Comment #77
mradcliffeI didn't find the change obvious when trying to search where functions disappeared/were changed to because I wasn't sure if menu_ui was the same or not. I thought perhaps that menu_get_menus was now in the Menu tree building or Menu entity in system module so I searched in the wrong direction.
Search for "menu_ui" in change records has no results: https://drupal.org/list-changes/published/drupal?keywords_description=me...
Seaching api.drupal.org for current menu_ui functions shows the following changes: https://api.drupal.org/api/drupal/8/search/menu_ui. So here's a draft of a change notice that reflects those API functions (non-hook/alter functions only).
Draft Change Record
Title: menu module renamed to menu_ui
Branch: 8.x
Version: 8.0-ALPHA11
The menu module has been renamed to menu_ui to avoid confusion for site builders. All menu module API functions have been renamed. Module developers should also consider using the
<code>entity.manager
service to find menu entities.Comment #78
mradcliffeI missed the comment by dawehner above linking the change record :*(. It was not published and not searchable by default. dawehner published the change record, and I'll modify it from there.