Problem/Motivation

There are way too many theme functions / template files in core. Some are similar (if not identical) and should be consolidated.

Proposed resolution

We have two options:

  1. Remove theme_checkboxes() and call theme('container') instead (build a render array with '#theme' => 'container')
  2. If we really do need a separate preprocess function for this, let's use theme('container__checkboxes') instead of theme('container') and add all our preprocess magic into template_preprocess_container__checkboxes()

Remaining tasks

Decide which of the 2 options above to pursue, and do it.

User interface changes

None

API changes

Removal of theme_checkboxes

#2041825: Remove theme_radios() and call theme('container') instead
#1819284: [meta] Consolidate all form element container templates, and add theme_hook_suggestions

CommentFileSizeAuthor
#33 2041845-33.patch5.19 KBandypost
#33 interdiff.txt551 bytesandypost
#32 2041845-32.patch5.18 KBandypost
#12 remove-2041845-12.patch3.13 KBmanuel garcia
#6 drupal-2041845-6.patch3.08 KBmanuel garcia
#4 drupal-2041845-4.patch2.63 KBmanuel garcia
#2 2041845-Remove-theme_checkboxes-2.patch1.8 KBericduran

Comments

ericduran’s picture

ericduran commented
Assigned: Unassigned » ericduran

Working on this.

ericduran’s picture

ericduran commented
Status: Active » Needs review
StatusFileSize
new 2041845-Remove-theme_checkboxes-2.patch1.8 KB

This patch piggy backs on the work from #2041825: Remove theme_radios() and call theme('container') instead.

Once that gets in this is the only change requires.

The last submitted patch, 2041845-Remove-theme_checkboxes-2.patch, failed testing.

manuel garcia’s picture

manuel garcia commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new drupal-2041845-4.patch2.63 KB

#2152201: Convert theme_checkboxes() to Twig just got in, so theme_checkboxes is history. We are now using template_preprocess_checkboxes & core/modules/system/templates/checkboxes.html.twig

Attached a patch that removes the preprocess function and the twig file, and changes its #theme_wrappers to container.

Status: Needs review » Needs work

The last submitted patch, 4: drupal-2041845-4.patch, failed testing.

manuel garcia’s picture

manuel garcia commented
Status: Needs work » Needs review
StatusFileSize
new drupal-2041845-6.patch3.08 KB

I believe we are loosing one css class form-checkboxes on this move towards using container.

To restore it we should add: $element['#attributes']['class'][] = 'form-'. $element['#type']; to template_preprocess_container (within the // Special handling for form elements. chunk).

Otherwise we could also add it within form_pre_render_checkbox($element) function I suppose. However I believe adding it for every form element using container is a better approach.

Status: Needs review » Needs work

The last submitted patch, 6: drupal-2041845-6.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented 
+++ b/core/includes/theme.inc
@@ -1789,6 +1789,7 @@ function template_preprocess_container(&$variables) {
+    $element['#attributes']['class'][] = 'form-'. $element['#type'];

+++ b/core/modules/system/system.module
@@ -474,7 +474,7 @@ function system_element_info() {
+    '#theme_wrappers' => array('container'),

I don't know for sure if this will work but, maybe you can set the theme_wrapper to 'container__checkboxes' and if the suggestions work with the preprocess functions you should be able to do template_preprocess_container__checkboxes to add in those extra classes.

Though that may need this:
#939462: Specific preprocess functions for theme hook suggestions are not invoked

mitokens’s picture

mitokens commented
Parent issue: » #1804614: [meta] Consolidate similar twig templates and properly use theme suggestions in core
andriyun’s picture

andriyun commented
Assigned: ericduran » Unassigned
andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 8.0.x-dev » 8.1.x-dev
manuel garcia’s picture

manuel garcia commented
Status: Needs work » Needs review
StatusFileSize
new remove-2041845-12.patch3.13 KB

Rerolled and added the css class back in template_preprocess_container__checkboxes as per suggestion on #8.

Status: Needs review » Needs work

The last submitted patch, 12: remove-2041845-12.patch, failed testing.

manuel garcia’s picture

manuel garcia commented

Well I don't know why those tests are failing, markup looks identical before and after...

manuel garcia’s picture

manuel garcia commented

Actualy, before:

<!-- BEGIN OUTPUT from 'core/themes/classy/templates/form/radios.html.twig' -->
<div id="edit-status" class="form-radios">

After:

<!-- BEGIN OUTPUT from 'core/themes/classy/templates/form/container.html.twig' -->
<div data-drupal-selector="edit-status" id="edit-status--wrapper" class="fieldgroup form-composite js-form-wrapper form-wrapper">
manuel garcia’s picture

manuel garcia commented

Oh g... my last two comments were meant to be for #2041825: Remove theme_radios() and call theme('container') instead

star-szr’s picture

star-szr
he/him
Primary language English
commented

My comment from #2041825-22: Remove theme_radios() and call theme('container') instead applies here as well.

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

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now 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.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now 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.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now 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.

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

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now 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.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.

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.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new 2041845-32.patch5.18 KB

re-roll for 10.1

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt551 bytes
new 2041845-33.patch5.19 KB

fix for specific preprocess

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

Didn't review yet

But seems there are failures in #33

andypost’s picture

andypost
he/him
Primary language Russian
commented

Yes, 3 failed tests but results are not returned

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.