Problem/Motivation

Follow-up from #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter(), which we wanted to mark as fixed since the code is in to preserve the intent of that issue. We need @code sample in the documentation, and tests for when we send an array of hooks to theme() (via drupal_render()).

Proposed resolution

Finish it up! :)

Remaining tasks

  • Docs in theme.api.php fleshed out with @code samples.
  • Test coverage for use cases when an array of hooks is passed to theme().

User interface changes

n/a

API changes

n/a

#1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()

CommentFileSizeAuthor
#42 add_code_sample_and-2111079-42.patch17.13 KBmohit_aghera
#38 interdiff-35-38.txt2.04 KBmac_weber
#38 add_code_sample_and-2111079-38.patch16.16 KBmac_weber
#37 interdiff.txt2.37 KBstar-szr
#37 add_code_sample_and-2111079-35.patch16.22 KBstar-szr
#33 2111079-32.patch16.99 KBLal_
#26 2111079-26.patch16.17 KBhussainweb
#21 2111079-20.patch16.17 KBstar-szr
#18 interdiff.txt3.26 KBstar-szr
#18 2111079-18.patch16.38 KBstar-szr
#12 2111079-12.patch16.52 KBstar-szr
#11 interdiff.txt3.15 KBstar-szr
#11 2111079-11.patch16.32 KBstar-szr
#10 interdiff.txt11.74 KBstar-szr
#10 2111079-10.patch15.09 KBstar-szr
#4 2111079-4.patch4.48 KBstar-szr
#13 2111079-13.patch16.43 KBstar-szr

Comments

star-szr’s picture

star-szr
he/him
Primary language English
commented
Assigned: Unassigned » star-szr

On it.

star-szr’s picture

star-szr
he/him
Primary language English
commented

For the record I am still chipping away at this, I've started work on the tests. If anyone else wants to work on the docs that would be more than welcome!

star-szr’s picture

star-szr
he/him
Primary language English
commented

From my notes, a nice description via @webchick:

Provides alternative, more specific templates.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2111079-4.patch4.48 KB

After many delays here's a first pass at improving the docs, so far I haven't came up with a great example for hook_theme_suggestions_HOOK_alter(), ideas on that and improving the hook_theme_suggestions_HOOK() example welcome.

Status: Needs review » Needs work

The last submitted patch, 4: 2111079-4.patch, failed testing.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs work » Needs review

4: 2111079-4.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 4: 2111079-4.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs work » Needs review

4: 2111079-4.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 4: 2111079-4.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs work » Needs review
Issue tags: -Needs tests, -API change, -theme system cleanup, -Theme Component Library, -Approved API change
StatusFileSize
new 2111079-10.patch15.09 KB
new interdiff.txt11.74 KB

Cleaning up tags and here is a new patch that should do everything it needs to. Should be ready for review as long as it's green.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Parent issue: » #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()
StatusFileSize
new 2111079-11.patch16.32 KB
new interdiff.txt3.15 KB

A few minor fix-ups, and thought it might be worth fixing up the routing file here because I didn't do it correctly in the first place in #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter().

star-szr’s picture

star-szr
he/him
Primary language English
commented
StatusFileSize
new 2111079-12.patch16.52 KB

Rerolled.

star-szr’s picture

star-szr
he/him
Primary language English
commented
StatusFileSize
new 2111079-13.patch16.43 KB

Rerolled again.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue tags: +Core Review Bonus

Trying out @thedavidmeister's core review bonus idea because I reviewed his patch at #2191101-7: Replace calls to theme() with drupal_render() in Views plugins.

thedavidmeister’s picture

thedavidmeister commented
Assigned: star-szr » Unassigned
Status: Needs review » Needs work
Issue tags: -Core Review Bonus 
+ * The following example provides an optional block--not-front.html.twig
+ * template that could be used to change the markup or display logic for blocks
+ * shown on every page but the front page:
+ * @code
+ * function block_theme_suggestions_block(array $variables) {
+ *   $suggestions = array();
+ *   if (!drupal_is_front_page()) {
+ *     $suggestions[] = 'block__not_front';
+ *   }
+ *   return $suggestions;
+ * }
+ * @endcode

I have a suggestion here, if this is a code sample for hook_theme_suggestions_HOOK and a newbie might not have a clear understanding of what hook and HOOK are, the sample would be clearer if they weren't both "block" in the sample. MYMODULE_theme_suggestions_MYHOOK maybe?

Other than that, I think the docs are good.

For the tests, I see we're only testing the case that we add new items to the suggestions array in the alter hook. In reality, after reading the comments/docs I'd expect just about any array manipulation I want to do to arrays in the alter hook to work - not just addition of new suggestions.

That means that we should add tests around altering for both deleting existing suggestions from the array (probably reasonably common) or directly editing existing suggestions (likely less common but probably still justifies test coverage).

#2094585: [policy, no patch] Core review bonus for #2173655: Refactor theme() to _theme(); make it a private API to discourage module developers from circumventing the renderable build system.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Assigned: Unassigned » star-szr

Thanks @thedavidmeister! Reassigning so I can have another go at this.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Assigned: star-szr » Unassigned

I haven't been able to get to this, so unassigning for the time being to free this up so I can focus on #2201781: Pass all theme hook suggestions to theme preprocess functions to allow for suggestion-specific overrides.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 2111079-18.patch16.38 KB
new interdiff.txt3.26 KB

Rerolled, only minor changes to make this make sense and work today, most of which can be seen in the interdiff other than adding public to the controller methods which I did as part of conflict resolution.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs review » Needs work
star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs work » Needs review

Rerolled after #2430909: hook_theme_suggestions() hook_theme_suggestions_HOOK() documentation is incorrect about how the hook is invoked, minor changes to line up with that issue.

star-szr’s picture

star-szr
he/him
Primary language English
commented
StatusFileSize
new 2111079-20.patch16.17 KB

Helps if you attach the file :)

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs review » Needs work

Status: Needs work » Needs review

Mac_Weber queued 21: 2111079-20.patch for re-testing.

mac_weber’s picture

mac_weber commented

I think this is the correct status for this issue.
Marking it to retest to see if we need a reroll

mac_weber’s picture

mac_weber commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch does not apply anymore

hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 2111079-26.patch16.17 KB

Rerolling.

hussainweb’s picture

hussainweb
Primary language English
commented
Issue summary: View changes

Since #2632996: Remove @todo tags at functions hook_theme_suggestions_HOOK() and hook_theme_suggestions_HOOK_alter() in file Theme.api.php was closed as duplicate in favour of this one, I am updating the suggested commit message to include contributors from there.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue tags: +Needs reroll

No longer applies.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work
Lal_’s picture

Lal_
He\Him
commented
Status: Needs work » Needs review
StatusFileSize
new 2111079-32.patch16.99 KB

Patch re-rolled.

Status: Needs review » Needs work

The last submitted patch, 33: 2111079-32.patch, failed testing.

Cottser credited Ankit Agrawal.

Cottser credited mohit_aghera.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new add_code_sample_and-2111079-35.patch16.22 KB
new interdiff.txt2.37 KB

Adding the credits from #27 in our new fancy way :)

Updating the reroll to undo the short array syntax reverts.

mac_weber’s picture

mac_weber commented
StatusFileSize
new add_code_sample_and-2111079-38.patch16.16 KB
new interdiff-35-38.txt2.04 KB

Fixed array styles.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Version: 8.5.x-dev » 8.6.x-dev
Status: Needs review » Needs work
Issue tags: +Needs reroll
mohit_aghera’s picture

mohit_aghera commented
Status: Needs work » Needs review
StatusFileSize
new add_code_sample_and-2111079-42.patch17.13 KB

Patch was not being applied to 8.6.x branch. Re-rolling for 8.6.x

markhalliwell’s picture

markhalliwell
Primary language English
commented
Priority: Major » Normal
Status: Needs review » Needs work 
  1. +++ b/core/lib/Drupal/Core/Render/theme.api.php
@@ -602,70 +602,84 @@ function hook_preprocess_HOOK(&$variables) {
+ * The following example provides an optional block--not-front.html.twig
+ * template that could be used to change the markup or display logic for blocks
+ * shown on every page but the front page:
+ * @code
+ * function block_theme_suggestions_block(array $variables) {
+ *   $suggestions = [];
+ *   if (!drupal_is_front_page()) {
+ *     $suggestions[] = 'block__not_front';
+ *   }
+ *   return $suggestions;
+ * }
+ * @endcode

    8.x doesn't have drupal_is_front_page().

    Can we, instead, just base this on a real use case like system_theme_suggestions_maintenance_page()? 

  2. +++ b/core/lib/Drupal/Core/Render/theme.api.php
@@ -602,70 +602,84 @@ function hook_preprocess_HOOK(&$variables) {
- * function MYMODULE_theme_suggestions_alter(array &$suggestions, array $variables, $hook) {
...
+ * function MYMODULE_theme_suggestions_node_alter(array &$suggestions, array $variables) {

    This is modifying the existing code example for hook_theme_suggestions_alter(), which shouldn't include the base theme hook per the example.

    Furthermore, the @todo Add @code sample in hook_theme_suggestions_HOOK_alter() hasn't actually been replaced with this patch.

---

Also, this is just documentation changes and a couple more tests. These aren't "major" changes.

jofitz’s picture

jofitz
he/him
commented
Issue tags: -Needs reroll

Removed "Needs Reroll" tag.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.