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()

Comments

Cottser’s picture

Assigned: Unassigned » Cottser

On it.

Cottser’s picture

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!

Cottser’s picture

From my notes, a nice description via @webchick:

Provides alternative, more specific templates.

Cottser’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
4.48 KB
FAILED: [[SimpleTest]]: [MySQL] 59,056 pass(es), 1 fail(s), and 0 exception(s). View

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.

markcarver’s picture

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.

Cottser’s picture

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.

Cottser’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests, -API change, -theme system cleanup, -Theme Component Library, -Approved API change
FileSize
15.09 KB
PASSED: [[SimpleTest]]: [MySQL] 60,058 pass(es). View
11.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.

Cottser’s picture

Parent issue: » #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()
FileSize
16.32 KB
PASSED: [[SimpleTest]]: [MySQL] 60,014 pass(es). View
3.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().

Cottser’s picture

FileSize
16.52 KB
PASSED: [[SimpleTest]]: [MySQL] 62,931 pass(es). View

Rerolled.

Cottser’s picture

FileSize
16.43 KB
PASSED: [[SimpleTest]]: [MySQL] 63,768 pass(es). View

Rerolled again.

Cottser’s picture

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

Assigned: Cottser » 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: [meta] [experimental] Core review bonus for #2173655: Refactor theme() to _theme(); make it a private API to discourage module developers from circumventing the renderable build system.

Cottser’s picture

Assigned: Unassigned » Cottser

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

Cottser’s picture

Assigned: Cottser » 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.

Cottser’s picture

Status: Needs work » Needs review
FileSize
16.38 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,450 pass(es). View
3.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.

Cottser’s picture

Status: Needs review » Needs work
Cottser’s picture

Status: Needs work » Needs review
Cottser’s picture

FileSize
16.17 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,954 pass(es). View

Helps if you attach the file :)

Cottser’s picture

Status: Needs review » Needs work

Status: Needs work » Needs review

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

Mac_Weber’s picture

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

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

Patch does not apply anymore

hussainweb’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
16.17 KB

Rerolling.

hussainweb’s picture

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

Issue tags: +Needs reroll

No longer applies.

Mile23’s picture

Status: Needs review » Needs work
AbhishekLal’s picture

Status: Needs work » Needs review
FileSize
16.99 KB

Patch re-rolled.

Status: Needs review » Needs work

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

Cottser’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
16.22 KB
2.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