Problem/Motivation

Regions module allows other modules (not only the active theme) to define regions, so blocks can be added (by block or context modules). The idea is this will help with interoperable features between themes with potentially different regions. However regions.modules doesn't check whether or not the active theme has already defined a region before adding through hook_footer(). It also adds a prefix to avoid potential conflicts, which makes comparing regions difficult.

Proposed resolution

Check if the active theme defines regions before adding. Remove the custom prefix to make this easier (no risk of conflicts, since we'll be checking).

User interface changes

The region labels no longer include 'Regions:' prefix, in the core block settings and context ui.

API changes

  • The project name is no longer used in hook_define_regions().
  • For ui consistency, a region title is now required in the hook_define_regions().
  • The full path is now required, when defining optional css and js.
  • $theme_key is now an optional param in _regions_list().

Comments

btopro’s picture

sound like good improvements. I'd also go for dropping the project property as it makes an assumption about where users can place files that they want included.

scottrigby’s picture

Title: Check if active theme defines regions before adding » Check active theme definitions before adding regions
Status: Active » Needs review
StatusFileSize
new6.94 KB

Patch refactors _regions_list() to check if the active theme defines regions before adding. Adds new requirement to hook_define_regions() and update api docs. Cleans up comments.

scottrigby’s picture

Ok… dropped project property, now expects modules to provide the path to css & js, and updated the api docs.

I didn't squash these, but kept as two separate commits - both in this patch (should work with git-apply).

scottrigby’s picture

Issue summary: View changes

update UI & API changes

btopro’s picture

These look like good changes. I'll have to update my implementation of it but that's not a big deal :) Nice work, I'll try updating my implementations of the API once the dev is published (monday at least :p) and I'll let you know how it goes but this makes sense to me.

scottrigby’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

update API