On the blocks administration page is an ‘Enabled’ column with a checkbox to enable the block. Additionally, we have a ‘Region’ column which defines the region. But if a block is not enabled, it doesn’t and shouldn’t have a region assigned. This patch removes the ‘Enabled’ column and adds a select option to the ‘Region’ column.

Comments

moshe weitzman’s picture

great idea ... block admin with multiple themes is quite tricky so we should test this patch in that scenario. of course, i have a feeling that scenario behaves unexpectedly with or without this patch.

RobRoy’s picture

Tested this out. Works well. A very nice improvement. +1

There's still some work to be done with block administration and this is a step forward.

RobRoy’s picture

Title: Simplify blocks administration » Simplify blocks administration - Remove enabled checkbox, add <none> option to region dropdown list instead

Changed title.

Cross referencing http://drupal.org/node/92630

moshe weitzman’s picture

RobRoy - if possible, mark this as RTBC or say what more is needed.

RobRoy’s picture

Status: Needs review » Reviewed & tested by the community

Okay, marking this RTBC. I gave it a +1 in my first comment above and the patch is still great. Shortly I will submit a patch in the issue I referenced in #3 to add the region drop down to the block add/edit page as well.

RobRoy’s picture

Status: Reviewed & tested by the community » Needs work

I'm modifying this patch so that we use a new constant BLOCK_REGION_NONE instead of just -1, but I had one question: On line 324 I was going to write a comment for why we are using !== instead of != (like core is doing for all ==='s). Why are we using a !== instead of != there?

RobRoy’s picture

StatusFileSize
new3.35 KB

Also, is the (int) on 289 necessary since we are passing that through %d which would ensure that a boolean is converted to an integer value? Just want to make this clean.

Here is a change with ONLY the addition of the constant. Still think maybe we should change !== to != and remove the (int). Thoughts?

kkaefer’s picture

Status: Needs work » Needs review
StatusFileSize
new3.55 KB

RobRoy is right. We don't need (int) and !==.

profix898’s picture

Status: Needs review » Reviewed & tested by the community

Code looks good and works as expected.
Really nice improvement :)

dries’s picture

Status: Reviewed & tested by the community » Fixed

I like. Good job. Committed.

Anonymous’s picture

Status: Fixed » Closed (fixed)