Move static config entity local tasks to local_tasks.yml [#2135787]

Comment	File	Size	Author
#23	interdiff.txt	842 bytes	amateescu
#23	local-task-config-entity-23.patch	7.16 KB	amateescu

#21	interdiff.txt	676 bytes	amateescu
#21	local-task-config-entity-20.patch	6.33 KB	amateescu

#19	interdiff.txt	3.1 KB	amateescu
#19	local-task-config-entity-19.patch	5.67 KB	amateescu

#14	local-task-config-entity-14.patch	8.77 KB	YesCT

#14	interdiff-2135787-3-14.txt	1.03 KB	YesCT
#3	interdiff.txt	1.27 KB	Gábor Hojtsy
#3	local-task-config-entity-3.patch	8.13 KB	Gábor Hojtsy

#2	local-task-config-entity.patch	8.21 KB	Gábor Hojtsy

Comment #1

YesCT CreditAttribution: YesCT commented 14 November 2013 at 13:30

Parent issue:

» #2135777: Follow-up [meta] local task conversion, move out of hook_menu() and to local_tasks.yml

Log in or register to post comments

Comment #2

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 November 2013 at 13:53

Status:

Active

» Needs review

File	Size
local-task-config-entity.patch	8.21 KB

Here is a patch which is complete to all config entities (at least those handled by config translation) *except* field instances, which need a local task derivative provider. Needs a little bit more time to do :)

Log in or register to post comments

Comment #3

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 November 2013 at 14:11

File	Size
local-task-config-entity-3.patch	8.13 KB

interdiff.txt	1.27 KB

Ok I looked at the fields stuff, and that is a whole can of worms. I think YesCT also already has a bug about that, so let's do it there. Cannot find the link right now... So here I think this is now the full scope :) Fixed the newlines, and I think this should be ready to review / commit :D

Log in or register to post comments

Comment #4

YesCT CreditAttribution: YesCT commented 14 November 2013 at 14:12

I was looking at #2 ...

+++ b/core/modules/menu/menu.local_tasks.yml
@@ -0,0 +1,4 @@
+menu.menu_edit:
+  title: 'Edit menu'
+  route_name: menu.menu_edit
+  tab_root_id: menu.menu_edit

+++ b/core/modules/menu/menu.module
@@ -86,11 +86,6 @@ function menu_menu() {
-  $items['admin/structure/menu/manage/%menu/edit'] = array(
-    'title' => 'Edit menu',
-    'type' => MENU_DEFAULT_LOCAL_TASK,
-    'context' => MENU_CONTEXT_PAGE,
-  );

do we lose the idea of MENU_CONTEXT_PAGE
?

Log in or register to post comments

Comment #5

YesCT CreditAttribution: YesCT commented 14 November 2013 at 14:14

#2109937: Manage fields local tasks not displayed at admin/config/people/accounts

Log in or register to post comments

Comment #6

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 November 2013 at 14:22

The menu context page constant does not really do anything on that item. See https://api.drupal.org/api/drupal/includes%21menu.inc/constant/MENU_CONT... this means the tab shows on the page :) This flag is meaningful in combination with other contextual states (eg MENU_CONTEXT_INLINE), but those are now removed from the menu system. So you get the same thing before/after.

Log in or register to post comments

Comment #7

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 November 2013 at 14:23

Title:

Config Entity Follow-up local task conversion, move out of hook_menu() and to local_tasks.yml

» Move static config entity local tasks to local_tasks.yml

Retitled for full scope.

Log in or register to post comments

Comment #8

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 November 2013 at 14:46

Issue tags:		+D8MI, +language-content, +Spark
Related issues:		+#2111823: Convert field_ui / Entity local tasks to YAML definitions

So #2111823: Convert field_ui / Entity local tasks to YAML definitions already covers the dynamic local tasks in field_ui, so we should be good here. This patch is as complete as it can get :) Reviews please!

Log in or register to post comments

Comment #9

YesCT CreditAttribution: YesCT commented 14 November 2013 at 15:43

Assigned:	Unassigned	» YesCT
Status:	Needs review	» Needs work

+++ b/core/modules/shortcut/shortcut.local_tasks.yml
@@ -2,3 +2,13 @@ shortcut.overview:
+  weigt: 10

noticed this. I'll fix it and finish reading through.

Log in or register to post comments

Comment #10

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler commented 14 November 2013 at 15:59

+++ b/core/modules/entity/entity.local_tasks.yml
@@ -0,0 +1,9 @@
+entity.view_mode_edit:
...
+entity.form_mode_edit:

+++ b/core/modules/image/image.local_tasks.yml
@@ -0,0 +1,4 @@
+image.style_edit:

+++ b/core/modules/system/system.local_tasks.yml
@@ -61,3 +61,7 @@ system.date_format_list:
+system.date_format_edit:

Is there a reason these are not removed from the respective hook_menu() implementations?

Log in or register to post comments

Comment #11

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 November 2013 at 16:04

@tstoeckler: there are some tasks that were not there before, eg. the views one and the entity module ones AFAIS. (Which were I think missing from earlier rounds of patches to ensure edit tabs on config entities are present by default). The image module one I did forget to remove from the module, that needs to be done. Not going to post an update since @YesCT is on changing stuff.

Log in or register to post comments

Comment #12

YesCT CreditAttribution: YesCT commented 14 November 2013 at 16:21

+++ b/core/modules/shortcut/shortcut.local_tasks.yml
@@ -2,3 +2,13 @@ shortcut.overview:
+shortcut.set_edit:
+  title: 'Edit set name'
+  route_name: shortcut.set_edit
+  tab_root_id: shortcut.set_customize
+  weigt: 10

+++ b/core/modules/shortcut/shortcut.module
@@ -96,16 +96,6 @@ function shortcut_menu() {
-  $items['admin/config/user-interface/shortcut/manage/%shortcut_set/edit'] = array(
-    'title' => 'Edit set name',
-    'route_name' => 'shortcut.set_edit',
-    'type' => MENU_LOCAL_TASK,
-    'weight' => 10,
-  );

this is taking out /edit... but

shortcut.set_edit:
  path: '/admin/config/user-interface/shortcut/manage/{shortcut_set}/edit'
  defaults:
    _entity_form: 'shortcut_set.edit'
  requirements:
    _entity_access: 'shortcut_set.update'

shortcut.link_add_inline:
  path: '/admin/config/user-interface/shortcut/manage/{shortcut_set}/add-link-inline'
  defaults:
    _controller: 'Drupal\shortcut\Controller\ShortcutSetController::addShortcutLinkInline'
  requirements:
    _entity_access: 'shortcut_set.update'
    _csrf_token: 'shortcut-add-link'

shortcut.set_customize:
  path: '/admin/config/user-interface/shortcut/manage/{shortcut_set}'
  defaults:
    _entity_form: 'shortcut_set.customize'
  requirements:
    _entity_access: 'shortcut_set.update'

has a customize without /edit and a different route for /edit

+++ b/core/modules/system/system.local_tasks.yml
@@ -61,3 +61,7 @@ system.date_format_list:
+system.date_format_edit:
+  title: 'Edit'
+  route_name: system.date_format_edit
+  tab_root_id: system.date_format_edit

I think this was just missing. It has no cooresponding hunk removing stuff from system_menu(). ok.

Log in or register to post comments

Comment #13

YesCT CreditAttribution: YesCT commented 14 November 2013 at 16:37

#12 1. is ok. I get it now.

Log in or register to post comments

Comment #14

YesCT CreditAttribution: YesCT commented 14 November 2013 at 17:18

File	Size
interdiff-2135787-3-14.txt	1.03 KB
local-task-config-entity-14.patch	8.77 KB

+++ b/core/modules/taxonomy/taxonomy.module
@@ -268,15 +268,6 @@ function taxonomy_menu() {
-  $items['admin/structure/taxonomy/manage/%taxonomy_vocabulary/list'] = array(

/list didn't work before this patch anyway ok.

+++ b/core/modules/user/user.module
@@ -786,10 +786,6 @@ function user_menu() {
-  $items['admin/people/roles/manage/%user_role/edit'] = array(

similar here, /edit didn't work before anyway and goes to the default manage/%user_role. ok.

SO.

only the weight typo and the image style removal of the local task from it's hook_menu.

I tried a bunch of these manually too and they work fine.

This looks rtbc to me.

Log in or register to post comments

Comment #15

YesCT CreditAttribution: YesCT commented 14 November 2013 at 17:19

Assigned:	YesCT	» Unassigned
Status:	Needs work	» Needs review

needs review for testbot and unassigning.

Log in or register to post comments

Comment #16

jsbalsera

he/his

Spanish

Córdoba, Spain

CreditAttribution: jsbalsera commented 14 November 2013 at 20:31

After applying the patch the "Edit" tab disappears from, for example, "admin/structure/types/manage/article/fields", but I suppose that this is covered by #8

Same happens in contact, like in "admin/structure/contact/manage/personal/fields", but again I think this is covered by #8

All the other changes (shortcut, roles, etc) behave exactly the same, so for me would be RTBC

Log in or register to post comments

Comment #17

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler commented 15 November 2013 at 12:29

Re #11: As far as I'm aware, the entity ones are in fact present in entity_menu(), so I guess those would need to be removed as well.

Log in or register to post comments

Comment #18

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 16 November 2013 at 08:18

I think its dangerous to commit this without #2111823: Convert field_ui / Entity local tasks to YAML definitions since it will hide all field UI entry points from the UI. So you will not be able to add fields to node types, block types, etc. That is pretty dangerous. This should be committed "at the same time" as #2111823: Convert field_ui / Entity local tasks to YAML definitions, basically right after each other.

Log in or register to post comments

Comment #19

amateescu CreditAttribution: amateescu commented 18 November 2013 at 13:35

Status:

Needs review

» Reviewed & tested by the community

File	Size
local-task-config-entity-19.patch	5.67 KB

interdiff.txt	3.1 KB

5 files were hidden/shown/deleted

File	Size
local-task-config-entity.patch	8.21 KB

local-task-config-entity-3.patch	8.13 KB

interdiff.txt	1.27 KB
interdiff-2135787-3-14.txt	1.03 KB
local-task-config-entity-14.patch	8.77 KB

It's not dangerous if you leave out the tasks that are already handled by that patch :)

Removed them from the patch so I think we're good to go here.

Log in or register to post comments

Comment #20

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 18 November 2013 at 16:46

I agree, looks good :) The other ones need to be with the dynamic field ones so the tabs don't break. We already broke account fieldability before (which will still not be fixed in alpha5 unless we don't get #2111823: Convert field_ui / Entity local tasks to YAML definitions in as well).