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.
Comment | File | Size | Author |
---|---|---|---|
#6 | interdiff.txt | 12.4 KB | star-szr |
#6 | 2160735-6.patch | 17.62 KB | star-szr |
Comments
Comment #1
star-szrThanks @aspilicious, just fixing up the issue link in the summary for now.
Comment #2
aspilicious CreditAttribution: aspilicious commentedThis probably needs tests and documentation. I verified this and it's exactly what I need to be able to fix my module.
Comment #3
star-szrThanks 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).
Comment #4
star-szrUpdating the issue summary and I can take on updating the patch and writing tests.
Comment #5
star-szrWe already have some docs in place for this hook even :)
From the theme() docblock:
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.
Comment #6
star-szrHad some time to come back to this. Should be complete now as far as test coverage and docs, ready for review.
Comment #7
aspilicious CreditAttribution: aspilicious commentedLooking good, the regression we have without this hook is pretty serious but I'll leave it major for now....
Comment #8
webchickUh, 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.
Comment #9
webchickForgot one of the 7,000 things.
Comment #10
aspilicious CreditAttribution: aspilicious commentedWell 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
Comment #11
star-szrWorking on updating the change record: https://drupal.org/node/2100775
Comment #12
star-szrChange record updated: https://drupal.org/node/2100775/revisions/view/2858969/6865283
Comment #14
forestmars CreditAttribution: forestmars commented