MENU_LOCAL_ACTION / MENU_LOCAL_TASK are a DX WTFage, as they are semantically near-identical, as pointed out here: http://drupal.org/node/542658#comment-1957074

Filing this so we don't forget :)

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Version: 7.x-dev » 8.x-dev

.

joachim’s picture

joachim commented

D8?
You're kidding me, right?

#542658: Move action "tabs" out of local tasks got in on the basis that this be dealt with.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

@joachim: unless you have actual ideas to discuss, there is not much point in trying to target this on D7. So what names would you suggest?

joachim’s picture

joachim commented

Maybe we should have accepted the need to bikeshed on this before committing a patch that did this?

Efforts were also made to fix "a bunch of stupid crap" that had just never been fixed in core due to other priorities. There are many more little (yet important) things that may thrill Drupal users.

[ http://www.cmswire.com/cms/web-cms/the-lowdown-drupal-7-means-big-change... ]

Let's not add stupid crap to D7 as we take it out, eh?

So. (And I can't actually remember which of MENU_LOCAL_ACTION / MENU_LOCAL_TASK is the new one, had to look it up.)

- MENU_LOCAL_ACTION is the new one. This represents an actual action. This remains the same.
- MENU_LOCAL_TASK is not a task or action -- the whole point of the UI report that led to this change was that these are lists or information or whatever. So let's just have this as MENU_LOCAL_ITEM.
- MENU_DEFAULT_LOCAL_TASK similarly should change to MENU_DEFAULT_LOCAL_ITEM.

Bikeshedding invited before I crank out perl pie and make a patch.

joachim’s picture

joachim commented
Version: 8.x-dev » 7.x-dev

And yeah, 7. This is a required follow-on from #542658: Move action "tabs" out of local tasks.

damien tournoud’s picture

damien tournoud commented

#4 makes sense. "Local items" make sense to me.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Category: bug » task

This is not required for #542658: Move action "tabs" out of local tasks.

This is an API change for the sole sake of renaming.

All modules that already started to port to D7 will break. If there was some critical API design flaw, critical bug, or performance optimization, then it would be reasonable.

joachim’s picture

joachim commented

It's required for #542658 because on reviewing the patch, we said this needed to be addressed. And we forgot.
The sooner we commit this, the less breakage.

brianV’s picture

brianV commented

+1 for changing MENU_LOCAL_TASK to MENU_LOCAL_DISPLAY or similar so the name matches the functionality.

I just submitted a patch for #649820: Missing documentation for MENU_LOCAL_ACTION in hook_menu(). That brought it into focus because the functionality for MENU_LOCAL_TASK does not at all match the name:

 *     - MENU_LOCAL_ACTION: Local actions are menu items that describe actions
 *       on the parent item such as adding a new user, taxonomy term, etc.
 *     - MENU_LOCAL_TASK: Local tasks are menu items that describe different
 *       displays of data.

We can't let D7 be released with a WTF like this. It's just embarrassing.

jonathan webb’s picture

jonathan webb commented

Here's a suggestion, rather than change the name of MENU_LOCAL_TASK (since this would break dependent contrib modules), why not add MENU_LOCAL_DISPLAY as a third constant to core which evaluates to the same value as MENU_LOCAL_TASK. Core can be patched with the new constant, and MENU_LOCAL_TASK can be retained in D7 but marked as deprecated (and removed from D8). This would prevent contrib breakage, and the added distinction between this constant and MENU_LOCAL_ACTION will reduce confusion for new developers.

joachim’s picture

joachim commented
Version: 7.x-dev » 8.x-dev
Issue tags: +DrupalWTF

Okay now we're in D8, let's fix this.

The really simple fix is to change:

MENU_LOCAL_TASK -> MENU_LOCAL_TAB
MENU_DEFAULT_LOCAL_TASK -> MENU_DEFAULT_LOCAL_TAB

rationale:

- it almost reads the same
- I know that the original intention in calling these 'tasks' was that some theme might render them as something other than tabs. But they are already referred to as tabs elswhere in code -- eg the 'tab_root' & 'tab_parent' properties in hook_menu(). So this would be adding consistency there too.

amateescu’s picture

amateescu commented
Issue summary: View changes
Status: Active » Closed (won't fix)

This is no longer relevant for D8, and I doubt it can still be changed in D7..