Not able to select region at block creation

Also posted at #503908: Assign a new block to a region on add block page

If you create a block at admin/build/block/add, which regions will be displayed? The one from the default theme, the current theme or the admin theme?

Isn't is possible to move the add block link to a particular theme so you'll which regions to display?

NVM: i just had a look at the patch and it displays a select for each theme

bot was broken - retest

Oh, testing slave #26... you're "special."

1. Just wondering, haven't looked at the code yet, but in case we are creating a new block, should we do an insert instead of an update?

2. It is not clear why we save the region both in the _validate and the _submit handler. Care to explain why that is necessary?

I applied this patch and played around with it a bit. I found it slightly odd that the main block administration page is for the enabled themes, but that the new per-block configuration also considers themes that are not enabled. In 80% of the cases (including a default core install), only 1 theme is enabled so it is somewhat odd that you're presented with themes you might not have seen yet. Any thoughts on that?

double check…

Rerolled #15, works ok for me

Committed this to CVS HEAD after I fixed the code comments. Thanks.

Nice one!

This didn't work on adding a new block. Only on update. This small patch fixes this.

Awesome patch - I've been thinking that something like this is needed, especially in light of the work on #473268: D7UX: Put edit links on everything which would link a lot of users directly to the individual block configuration pages.

In addition to the followup already posted here, there are other issues with the original patch that should be fixed:

1. Using system_get_theme_data() causes a filesystem scan, which is unnecessary; I'm pretty sure list_themes() is a better choice.
2. The patch does not properly deal with hidden theme regions (in particular, it lets you assign blocks to them). It needs to make use of system_region_list(), as is done on the other block administration page; see block_admin_display_form().
3. The help text on the add blocks page needs to be updated, since it currently says: "New blocks are disabled by default, and must be moved to a region on the blocks administration page to be visible."
4. We should think about changing the user interface here a bit; in my opinion, this setting is too important to hide away in a collapsed fieldset like the others.

I'd reroll the patch myself, but all these issues are bugs/UI improvements, so they can be dealt with after code freeze, and I really don't want to spend the time tracking down the test failures now :)

Did not mean to reset status just yet - let's see if the test failures are real after all.

This patch also uses list_themes. This is conform with the creation of the local task on admin/structure/block in hook_menu.

+1

Applied this against core and it worked just fine.

Committed to CVS HEAD. We still need tests though, so marking this 'code needs work'.

Also still needs work for the issues I described in #26.

@David_Rothstein I'm using list_themes in my latest patch. This is the same function used in hook_menu. So if we changed it in one place, we should match the other one. Maybe open a new issue for this?

Right, my first point in #26 was fixed in the latest patch, but points 2-4 were not. However, we can deal with that post code freeze.... somewhere... although I think this issue is a good place, so as to consolidate all the followups in one location :)

the following patch should at least fix the issue where hidden regions should not be displayed in the select block. It also fixes the help message.

@David_Rothstein Regarding interface. I think this might help too #556390: Vertical Tabs on block visibility settings

HEAD is broken

There is also a bug with this noted by Jacob Singh: that -1 is saved as the region for disabled blocks and then comes back with "The block {title} was assigned to the invalid region -1 and has been disabled." Not nice :| Reported at http://drupal.org/node/544360#comment-2127370

Looks like those tests mentioned by Dries are indeed lacking.

@webchick noted the same -1 issue on http://drupal.org/node/517688#comment-2170538 while testing overlays.

Submitted separate issue for the new block problem at #648710: Error when creating new block: The block %info was assigned to the invalid region %region and has been disabled., so this can concentrate on the existing issue with visible regions.

Marked #716388: Custom user added blocks can be added to non-existent regions as a duplicate of this one.

Marking critical. As I point out in #716388: Custom user added blocks can be added to non-existent regions (marked as duplicate), users can add a block to a region that is not visible on the block manage page. This means that a user can add a block, it will appear on the site, but the user will not be able to access the block to modify / remove it. To me it is critical if a user can add content to their site, but cannot remove it again.

This should fix the issue with hidden regions showing up on the block add/configure page. Switched the options list to use system_regions_list().

This patch also fixes the help text on the page.

I've attempted to write tests for this but am running in to some problems. I believe that that test in the attached patch should pass, however system_regions_list() seems to act funny in the context of a test. The hidden regions are still being displayed in the select list during a test run. Though I've confirmed that they are not there when visiting the page normally.

Marking as needs to review so that the tests get run to confirm the failures.

Anyone have any insight?

Hmm. Not exactly what I expected. Lets try this one.

bot.

Committed to CVS HEAD. Thanks.