custom block titles [#76666]

attached patch introduces hook_block_alter, which allows calling modules to alter the subject and content of any displayed block. This hook is pretty efficient--it's only called once per page request, and the modifications are made directly to the block array prior to theming via a constructed reference array.

i kept the implementation light for this first posting--we might want to make the hook a little more devel friendly, or we may also want to rework some functions around how we do the altering (for the time being, i just threw the code into the block_list function, as that's where the blocks array is being built).

here's a link to a test site which illustrates the hook in action: http://hookblockalter.xcarnated.com/

and a the implementation of the hook that is making the changes at that test site:

function block_alter_block_alter($blocks) {
  foreach ($blocks as $id => $data) {
  	switch ($id) {
  	  case 'user_1':
  	    unset($blocks[$id]->subject);
  	    break;
  	  case 'user_2':
  	    $blocks[$id]->subject = t('The new guys');
  	    break;
  	  case 'search_0':
  	    $blocks[$id]->subject = check_plain(variable_get('block_alter_example', t('Search')));
  	    break;
  	  case 'blog_0':
  	    $blocks[$id]->content = t('%count posts in this little list', array('%count' => substr_count($blocks[$id]->content, '<li>'))) . $blocks[$id]->content;
  	    break;
  	  case 'forum_0':
  	    $blocks[$id]->content  = str_replace(array('<ul>', '</ul>'), array('<ol>', '</ol>'), $blocks[$id]->content);
  	    break;
  	  case 'poll_0':
  	    $blocks[$id]->content .= '<p>'. t('This is the bestest poll ever...');
  	    break;
  	}
  }
}

now i know that some of that content editing is a bit of hack... :) but at this point we're really limited by the way the content is built (similar to node->body). when we've implemented a better system for compiling the content (ala forms API), then the content altering will of course become much more elegant.

Comment	File	Size	Author
#31	customblocktitles_6.patch	9.01 KB	hunmonk
#23	customblocktitles_5.patch	9.78 KB	hunmonk
#19	customblocktitles_4.patch	9.69 KB	hunmonk
#18	customblocktitles_3.patch	7.2 KB	hunmonk
#17	customblocktitles_2.patch	7.2 KB	hunmonk
#13	customblocktitles_1.patch	6.33 KB	hunmonk
#10	customblocktitles_0.patch	6.25 KB	hunmonk
#6	customblocktitles.patch	6.26 KB	hunmonk
#2	hookblockalter_1.patch	6.71 KB	hunmonk
#1	hookblockalter_0.patch	730 bytes	hunmonk
	hookblockalter.patch	1.29 KB	hunmonk

Comments

Comment #1

hunmonk commented 2 August 2006 at 03:35

Status	File	Size
new	hookblockalter_0.patch	730 bytes

sheesh. don't know what i was thinking with that static variable approach--totally unneccesary! here's an updated patch that's a little more elegant. exact same functionality

test site: http://hookblockalter.xcarnated.com/

Comment #2

hunmonk commented 2 August 2006 at 05:10

Status	File	Size
new	hookblockalter_1.patch	6.71 KB

ok, i took this one step further: now there is a core feature that allows an admin to override the block title for any block. :) i did this by simply adding a title column to the blocks table, and a form field to edit it on the block config screen. this override comes before the hook_block_alter, so that hook takes precedence. if the title field is left blank, then the block title uses the default.

test site: http://hookblockalter.xcarnated.com/

Comment #3

dopry commented 2 August 2006 at 05:54

Code style looks good. I like the ability to manipulate block titles.

The block alter is an awesome function. I think its a good start, and a first step towards bringing blocks into the rendering api...

Comment #4

hunmonk commented 2 August 2006 at 14:17

one thing i'm not sure about: right now the module hook trumps the custom user settings--is that how we want it? reversing the order will involve a little more coding, as the current setup doesn't play well with that workflow...

Comment #5

killes@www.drop.org commented 3 August 2006 at 15:50

I think that if you need a hook to alter blocks, you should rather duplicate the block code in the module you are writing.

To allow site admins to change block titles is a different thing alltogether and I think it is consistent with allowing to change menu titiles, so I am +1 on that idea.

Comment #6

hunmonk commented 3 August 2006 at 16:23

Title:

hook_block_alter -- custom block modification for modules

» custom block titles

Status	File	Size
new	customblocktitles.patch	6.26 KB

after further thought, i agree w/ killes--let's bag the hook, but keep the spiffy custom title feature... :) attached patch includes only this custom title feature, and i also added the ability to have no display title by entering <none> in the custom title field.

demo site for this adjusted patch is still: http://hookblockalter.xcarnated.com/

Comment #7

dries commented 4 August 2006 at 07:07

The custom block title is nice, IMO, and the code looks good. (The _alter hook is not nice.)
Do we want this functionality?

Comment #8

gerhard killesreiter commented 4 August 2006 at 11:01

I recall that people have asked for this functionality over the years.

Comment #9

hunmonk commented 4 August 2006 at 16:13

@Dries: i agree that the _alter hook is not nice, which is why it was removed from the latest patch. i really think that the custom block titles is a great feature, though, w/ a very light implementation. i'd like to see it go in, so let me know if there are any more adjustments i need to make... :)

Comment #10

hunmonk commented 7 August 2006 at 03:02

Status	File	Size
new	customblocktitles_0.patch	6.25 KB

broken by the content types patch. updated patch attached.

demo site for this is still: http://hookblockalter.xcarnated.com/

Comment #11

moshe weitzman commented 7 August 2006 at 04:02

Status:

Needs review

» Reviewed & tested by the community

tested functionality and update path. also did a code review. people want this ... RTBC

Comment #12

dries commented 7 August 2006 at 14:33

Status:

Reviewed & tested by the community

» Needs work

It looks a bit weird that the title is not wrapped in a fieldset, whereas other options are ...

Also, is "for no display title" proper English?

Third, "when it is displayed on the page" is weird language, and mostly redundant information.

Lastly, why "Display title:" and not "Block title:" or just "Title:"? I'm not sure what the Display is for ... it sounds more technical than it ought to ...

(BTW, a configurable/overwriteable more link would be nice too ... but can be left for another patch.)

Comment #13

hunmonk commented 7 August 2006 at 16:36

Status:

Needs work

» Needs review

Status	File	Size
new	customblocktitles_1.patch	6.33 KB

updated patch attached.

@Dries: i believe i've addressed all of your concerns. my main concern with the help text there is to communicate that what this alters is the displayed title on the page, not the name of the block for admin purposes, etc.

so what i've done is placed that element into a fieldset called "Custom display settings", where other things of this type can live as well if we put them in sometime down the road. This effectively communicates the right message, and allowed me to greatly simplify the element's description.

test site: http://hookblockalter.xcarnated.com/

Comment #14

hunmonk commented 7 August 2006 at 19:09

Status:

Needs review

» Reviewed & tested by the community

i'm going to slide this back to RTBC, as i *think* i've handled all of dries' concerns above. let me know if not... :)

Comment #15

dries commented 14 August 2006 at 11:39

Status:

Reviewed & tested by the community

» Needs work

Patch no longer works (update.php changes).

Did you test what happens with a user-defined block? There won't be two title textfields, would there?

Comment #16

hunmonk commented 14 August 2006 at 13:25

Did you test what happens with a user-defined block? There won't be two title textfields, would there?

yep, looks like there is. i hadn't considered this. :)

i'll get that fixed up, and the adjustment for the the new update system...

Comment #17

hunmonk commented 15 August 2006 at 00:20

Status:

Needs work

» Needs review

Status	File	Size
new	customblocktitles_2.patch	7.2 KB

ok, i think i've effectively solved the issue: i repurposed the 'Block specific settings' fieldset to include both the custom title element and any block specific stuff brought in by the configure op. if it's already a custom block, then the custom title element is skipped, and if the fieldset has no elements, it's skipped as well.

also fixed up the update.php thing.

Comment #18

hunmonk commented 15 August 2006 at 21:12

Status	File	Size
new	customblocktitles_3.patch	7.2 KB

more under-the-hood improvements. chx pointed out that we really should normalize the table situation and move the title column for custom blocks into the blocks table. this was no small feat, but i believe i've accomplished it. :)

the main issue was reworking the block rehashing function a little, as it was killing the custom block titles before they could be properly constructed in the rehash.

i'm pretty happy with the current implementation--if there are any more adjustments needed, let me know.

Comment #19

hunmonk commented 15 August 2006 at 21:22

Status	File	Size
new	customblocktitles_4.patch	9.69 KB

whoop. wrong patch. :) here's the right one...

and the test site: http://hookblockalter.xcarnated.com/

Comment #20

chx commented 15 August 2006 at 23:23

I had a code review and this time I like it. An actual test would be welcome, of course.

Comment #21

dopry commented 15 August 2006 at 23:36

Code looks good still. But I'm not the style guide...

I still like the functionality, and works for me (tm).

It may be nice to have the default title near the in the description just for reference, but that would be pendantic since its in the title anyway.

Comment #22

dries commented 16 August 2006 at 05:00

Custom block titles are never escaped, it seems. Neither are custom block bodies. Not sure that is considered to be an issue. I figured I'd bring this up nonetheless.

Comment #23

hunmonk commented 16 August 2006 at 15:50

Status	File	Size
new	customblocktitles_5.patch	9.78 KB

Custom block titles are never escaped, it seems.

yes, good catch. after a lengthy discussion in IRC about the best place to put this check, it seems that it should be right when the custom title is substituted for the default title in function block_list. although this is a bit early in the output cycle for a check_plain, it allows us to have the check in only one place (thus preventing holes from poor plain php theme designs), and also lets default module titles keep any markup they may have.

Neither are custom block bodies.

i'm pretty sure they are, right here:

$data['content'] = check_markup($block->body, $block->format, FALSE);

updated patch attached.

test site: http://hookblockalter.xcarnated.com

Comment #24

m3avrck commented 17 August 2006 at 17:24

Hmm, I really like this patch but from a usability perspective, I think we can make it easier.

We have modules that define blocks, by doing so you get the "Most Recent Poll" description for a block. With this patch, you can now add a custom title to the block instead of using the default description.

When you add a custom block, you give it both a description and a title. Hmm... why would you need both? Drupal up until now has always used the displayed title for a block as a description. They are one in the same.

With this patch, they could be different. Do they need to be?

I would say, we get rid of block "descriptions" and replace it with block titles. When a module adds a block, it adds the title for the block. This can then be edited if need be. When you add a custom block, you give it a title which you can edit too. This makes sense--from a usability perspective, this is so much easier. Even I had to double check what the difference was and I use blocks on a daily basis. Not only that, but with this change, we can output the title to the block on the overview page, instead of just the description, which gets weird with custom blocks.

Seems to confusing otherwise.

Comment #25

dries commented 17 August 2006 at 20:20

We show the description on the block configuration page. I'm ok with removing the description, if feasible. But what to do with blocks that have dynamic titles (eg. user block). Might not be a problem, I haven't checked. Might work for custom blocks. Certaintly worth exploring!

Comment #26

hunmonk commented 17 August 2006 at 21:47

@Dries: dynamically titled module blocks should not be an issue--the code is designed in such a way that it only overrides the module title if another title is provided. since there's no such thing as a description for module-generated blocks, i don't see any issues here.

as far as removing the description from custom blocks, i think it's not a problem--but i also think it's feature creep on this patch. the patch's goal was to enable overridable titles for module blocks. it only addressed custom blocks in a way that resulted in normalization of the db tables and an integrated UI--the custom block functionality is the same as it's ever been.

i'd hate to see this functionality get left out of core for the next release because of infinite tweaking. it's solid, it's requested, and i think it's ready. how about we leave the rest for another patch?

Comment #27

moshe weitzman commented 18 August 2006 at 00:15

i have to agree that removing description is a separate issue. fight scope creep.

Comment #28

dries commented 18 August 2006 at 07:47

Sure, we can leave that for another patch. I'm happy with this patch as is. I'm leaving this for Neil to commit -- he might have additional usability requirements.

Comment #29

m3avrck commented 18 August 2006 at 15:51

Sure, apologies for the feature creep :-)

This patch is still solid and works as advertised. I'd say it's ready to go as well. We can create an issue later to clean up the interface and maybe consolidate the title/description/etc. Heck, might even be able to clean up the whole interface for blocks too but that's for another day :-)

Comment #30

dries commented 18 August 2006 at 19:03

Status:

Needs review

» Reviewed & tested by the community

Can be committed unless someone objects.

Comment #31

hunmonk commented 21 August 2006 at 05:26

Status	File	Size
new	customblocktitles_6.patch	9.01 KB

keeping up w/ HEAD.

please note that currently there's a bug in HEAD at admin/build/block, whereby blocks are being saved and ordered incorrectly. this bug has nothing to do with this patch--i believe it was introduced by the fapi pull patch.

test site: http://hookblockalter.xcarnated.com/

Reviewed & tested by the community

» Fixed

Comment #35

(not verified) commented 5 September 2006 at 08:31

Status:

Fixed

» Closed (fixed)