Updated: Comment #4

Problem/Motivation

Part of the original plan with the changes to the theme system architecture was to model things after the structure of form API hooks. After all the discussion in #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter() we ended up added the equivalent of hook_form_FORM_ID_alter() without adding (the equivalent of) hook_form_alter(). This breaks valid use cases.

Proposed resolution

Add the missing alter hook, hook_theme_suggestions_alter().

Remaining tasks

Add test coverage.

User interface changes

n/a

API changes

Add hook_theme_suggestions_alter().

Original report by @aspilicious

While porting some parts of display suite I noticed that it is impossible to dynamically add theme_suggestions for each entity. The hooks added in #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter() require the HOOK name to be available in the function name.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

star-szr’s picture

Issue summary: View changes

Thanks @aspilicious, just fixing up the issue link in the summary for now.

aspilicious’s picture

Status: Active » Needs review
FileSize
731 bytes

This probably needs tests and documentation. I verified this and it's exactly what I need to be able to fix my module.

star-szr’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks for getting this going!

Functionally that looks about right, but the new alter should go before the hook-specific alter (same as hook_form_alter() for example).

star-szr’s picture

Title: Add a general hook_theme_suggestions_alter hook [regression] » Add hook_theme_suggestions_alter() [regression]
Assigned: Unassigned » star-szr
Issue summary: View changes
Parent issue: » #2004872: [meta] Theme system architecture changes

Updating the issue summary and I can take on updating the patch and writing tests.

star-szr’s picture

FileSize
8.58 KB

We already have some docs in place for this hook even :)

From the theme() docblock:

Alternate hooks can be suggested by implementing the hook-specific
 * hook_theme_suggestions_HOOK_alter() or the generic
 * hook_theme_suggestions_alter().

Here's a work in progress patch, still needs some more test coverage and a bit more work on the theme.api.php docs/examples.

star-szr’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
17.62 KB
12.4 KB

Had some time to come back to this. Should be complete now as far as test coverage and docs, ready for review.

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Looking good, the regression we have without this hook is pretty serious but I'll leave it major for now....

webchick’s picture

Category: Bug report » Task
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record, +Approved API change

Uh, I'm not sure how a new hook added to another new hook that was added for the first time in D8 could possibly be a regression of anything. ;) However, it does complete the API, so the change makes sense.

There's no profiling data here which makes me a bit nervous, but we can probably expect it to be similar to #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter() which didn't hold up that patch.

I'd like to get this in prior to alpha so that Display Suite and any other modules attempting to port to D8 can make use of it. The patch comes with test coverage and docs so I couldn't find anything to complain about!

Committed and pushed to 8.x. Thanks!

We'll need the existing change notice expanded for this change as well.

webchick’s picture

Title: Add hook_theme_suggestions_alter() [regression] » Change notice: Add hook_theme_suggestions_alter() [regression]

Forgot one of the 7,000 things.

aspilicious’s picture

Well it's a regression as it was possible to accomplish this with a preprocessor function in D7. And with the introduction of the new hooks in D8 some off the functionality got lost because we couldn't add the suggestions in the preprocessor anymore.

So yeah it WAS a regression but a hidden one.

Thnx for committing this. =D

star-szr’s picture

Working on updating the change record: https://drupal.org/node/2100775

star-szr’s picture

Title: Change notice: Add hook_theme_suggestions_alter() [regression] » Add hook_theme_suggestions_alter() [regression]
Assigned: star-szr » Unassigned
Category: Task » Bug report
Status: Active » Fixed
Issue tags: -Needs change record

Status: Fixed » Closed (fixed)

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

forestmars’s picture

Issue summary: View changes