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.Follow up for #1871772-108: Convert custom blocks to content entities
Problem/Motivation
+++ b/core/modules/block/custom_block/custom_block.admin.inc
@@ -0,0 +1,98 @@
+function custom_block_type_edit(CustomBlockType $block_type) {
+ return entity_get_form($block_type);
Normally we add drupal_set_title('Edit %label block type', ...) here, but we can improve this later.
Proposed resolution
Update page callback
Remaining tasks
Update page callback
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | drupal-set_page_title_when_editing_custom_block_type-1919914-4.patch | 578 bytes | floydm |
Comments
Comment #1
larowlanUntil #1871772: Convert custom blocks to content entities is in, this is postponed
Comment #2
larowlanChanging tags
Comment #3
sunComment #4
floydm CreditAttribution: floydm commentedPatch attached.
Comment #5
sunComment #6
catchThis should use 'title callback' rather than drupal_set_title().
Comment #7
sunErm, no. Page title != menu link title.
The menu link title uses entity_page_label() as title callback already, so the menu link title is $entity->label().
The title of the page, however, is always supposed to be a user-friendly phrase "Edit %label thingie."
Comment #8
BerdirHm.
1. We actually don't seem to add that "normally". See the content type edit form, which also just as the content type and this is basically a 1-1 counterpiece of that.
2. This doesn't work for the manage/display fields tabs, where it falls back to just the block type label.
So, this patch actually introduces two inconsistencies, it's different for some local tasks and it's different to content types.
Edit: As discussed in IRC, other cases do work like this, e.g. the node and custom block local tasks but they don't have manage fields and similar tasks that wouldn't be doing this.
Comment #9
xjm#4: drupal-set_page_title_when_editing_custom_block_type-1919914-4.patch queued for re-testing.
Comment #10
webchickWe are a little inconsistent, but "Edit $foo" makes more sense to me as a title on an edit form than just "$foo". We already do this on individual nodes and custom blocks, for example, as well as image styles. So it makes sense to do so for their "parents": block types and custom content types. Expanding scope a little, because that's a good point that we should be consistent.
I think catch's concern boils down to #1830588: [META] remove drupal_set_title() and drupal_get_title(), but since that's all talk atm, doesn't seem to make sense to hold this simple patch up on it. Whatever method we work out there will just need to be applied here as well.
While I understand that page/menu title are different things, one thing I'm not clear on though is what's the value in keeping the page title + menu title different from each other in this instance? In IRC Berdir said that that would mean that "Manage fields", etc. would also have the page title of "Edit $foo" but to me that just makes sense; they're all "Edit" operations on the primary thing.
Comment #11
sunThe important difference is that when modules are dynamically creating menu links for these items, then those menu links should only contain the entity label, not "Edit [label]".
Are there examples of modules that are doing that? Administration menu.
Comment #22
quietone CreditAttribution: quietone at PreviousNext commentedMethinks this is outdated. The page title is set when editing content types.
If this is incorrect reopen the issue, by setting the status to 'Active', and add a comment explaining what still needs to be done.