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.
Updated: Comment #11
Problem/Motivation
Follow up of #2004334: Separate Tabs (MENU_LOCAL_TASK) from hook_menu()
Change record Local tasks are provided by annotated plugins implementing LocalTaskInterface instead of type MENU_LOCAL_TASK in hook_menu()
Proposed resolution
Remaining tasks
Waiting on
- #2076283: Allow local task plugins on paths with a dynamic value (like a node)
- #2045267: LocalTaskBase and LocalTaskInterface has to work with routes containing parameters
User interface changes
API changes
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#24 | interdiff.txt | 375 bytes | dawehner |
#24 | config_translation-2044737-24.patch | 8.37 KB | dawehner |
#21 | config_translation-2044737-14.patch | 8.29 KB | Gábor Hojtsy |
#16 | config-translation-id.patch | 2.92 KB | Gábor Hojtsy |
#14 | config_translation-2044737-14.patch | 11.22 KB | dawehner |
Comments
Comment #1
Gábor HojtsyComment #2
Gábor HojtsyLocal tasks still seem to work for now, but it is highly unlikely that the module would be committed without this conversion.
Comment #3
Gábor HojtsyWatch this issue for how we can do this: #2032309: Use local tasks derivatives to provide local tasks for views - we need to do derivative generation as well based on the mappers. It is not as simple as dropping in a few classes.
Comment #4
Gábor HojtsyOk, I think watching the views issue progress is the best we can do ATM. :)
Comment #5
dawehnerThere are many probles we have to figure out first, before that one can be implemented:
Nevertheless, this is a start to show how it should look like.
Comment #6
vijaycs85Comment #8
Gábor Hojtsy@dawehner: thanks for doing this! Amazing!
Comment #9
dawehnerJust want to say, that once the menu API changes allow us to do that I will come back. This issue and probably field_ui/views will proove that the API does actually cover the usecases.
Comment #10
Gábor Hojtsy@dawehner: yay! Thanks a lot!
Comment #10.0
YesCT CreditAttribution: YesCT commentednot sure why that didn't link to the issue
Comment #11
YesCT CreditAttribution: YesCT commented@dawehner says we are still missing #2076283: Allow local task plugins on paths with a dynamic value (like a node)
Comment #11.0
YesCT CreditAttribution: YesCT commentedah, because it was a change record
Comment #11.1
YesCT CreditAttribution: YesCT commentedwith template and missing issue
Comment #11.2
YesCT CreditAttribution: YesCT commentedoops same issue
Comment #12
YesCT CreditAttribution: YesCT commented@tim.plunkett mentioned in irc that we don't want annotated plugins for local tasks. we want yaml.
and @dawehner pointed to #2050919: Replace local task plugin discovery with YamlDiscovery
Comment #13
Gábor HojtsyYeah this is still in flux... :/
Comment #14
dawehnerYeah #2076283: Allow local task plugins on paths with a dynamic value (like a node) is in!
Comment #16
Gábor HojtsyYay, thanks for your work on this.
I've extracted the id introduction only, so this can be committed ASAP. Let's see how this passes/fails.
Comment #18
dawehnerEvery call to drupalPost should be converted to drupalPostForm() now ... so this is kind of unrelated to the patch.
One thing the patch is using is the following snippet:
This bits could be replaced if the config_translation plugins would also contain the route name instead of just the base path.
Comment #19
Gábor Hojtsy@dawehner: right, the drupalPost change is in #2093007: drupalPost => drupalPostForm() in tests and other core fails. We have been waiting on a core fix so we don't need to uncomment tests to make our stuff pass :) That issue should shortly come back green and get committed :)
As for route names, are all menu callbacks now converted to routes, so we can rely on that? Well, probably those that we use were converted. Mostly config entities and main site settings.
Comment #20
dawehner#16: config-translation-id.patch queued for re-testing.
Comment #21
Gábor HojtsyOk, committed the ID patch with docs for the $id property at http://drupalcode.org/project/config_translation.git/commitdiff/0ec881a1... (although I used an autogenerated silly commit message, sorry :D)
Here is a quick edited version of #14 without those hunks.
Comment #23
Gábor HojtsyFails with
Undefined index: > Notice LocalTaskManager.php 215 Drupal\Core\Menu\LocalTaskManager->getLocalTasksForRoute()
Not sure what would that come from...Comment #24
dawehnerThis is caused by having not converted all the other existing local tasks for the edit tabs. Here is a todo
Comment #25
YesCT CreditAttribution: YesCT commentedComment #27
Gábor Hojtsy@dawehner: do you have examples of those?
Comment #28
dawehner#2095139: Checking for the active tab should use raw variables. is needed to convert any of the config entity local tasks, so I don't know of an example which already has an edit tab.
Comment #29
tim.plunkettSee #2095269: Remove add_edit_tab, core should have default tabs for some similar work, the issue @YesCT asked me to look at. Didn't know about this one.
Comment #30
Gábor Hojtsy@tim.plunkett: yeah that one was about to just remove the edit tab autogeneration. Which issue should we consolidate this then? The less issues, the less rerolls needed :) That one? This one?
Comment #31
tim.plunkettThat matters much less than just getting #2095271: Add default tabs for routes expected by config_translation in first, IMO. I will read through this approach again and see how to best merge them.
Comment #32
Gábor Hojtsy@tim.plunkett: yeah, sure, thanks!
Comment #33
Gábor HojtsyI incorporated the pieces that are still needed from this to #2095269: Remove add_edit_tab, core should have default tabs, where some overlapping code was already there. Thanks @dawehner! Learned a lot from this patch.
Comment #34
Gábor HojtsyCommitted #2095269: Remove add_edit_tab, core should have default tabs and attributed dawehner on it :) Thanks again!
Comment #34.0
Gábor Hojtsycopy and paste mistake before.