Problem/Motivation

Replace theme_*_form() with theme suggestion for #theme => form. This eliminates the registration of templates that do nothing except render {{ form }} children. @see #2151821: Convert theme_system_config_form() to Twig

Proposed resolution

#theme_wrapper => 'form' is used as a theme_wrapper which doesn't have suggestions so that will need to move to a #theme property for this to work.

Delete

core/modules/system/templates/confirm-form.html.twig

Remaining tasks

  1. Change '#theme_wrapper' => 'form' to '#theme' => 'form'.
  2. Then all the theme_*_form() calls will be suggestions for form--system-config.html.twig for example and don't need to be implemented.
  3. For extra brownie points use a twig block like we have for block.html.twig to avoid the suggestions from providing the form tags as well when implemented.

Theme functions to convert:

  • theme_system_config_form()
  • theme_confirm_form() already in core as config-form.html.twig
  • theme_forum_form()
  • Maybe views_exposed_form and a few others but implemented suggestions.

User interface changes

n/a

API changes

n/a

Files: 
CommentFileSizeAuthor
#7 replace_theme_form-2265991-7.patch20.55 KBjoelpittet
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,559 pass(es), 1,006 fail(s), and 42 exception(s). View
#3 2265991-theme-form--sugestions-3.patch21.83 KBjoelpittet
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2265991-theme-form--sugestions-3.patch. Unable to apply patch. See the log in the details link for more information. View
#3 interdiff.txt3.43 KBjoelpittet
#1 2265991-theme-form--sugestions-1.patch18.94 KBjoelpittet
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,822 pass(es), 1,081 fail(s), and 46 exception(s). View

Comments

joelpittet’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
18.94 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,822 pass(es), 1,081 fail(s), and 46 exception(s). View

Ok this will likely fail, but I thought I'd give a direction to which I'm thinking this issue should go.

Locally the forms are loading, so at least I can run the tests in the UI:)

There may be a better way to deal with the form theme suggestion defaults because I know there was an array('form', 'form') in #theme, which isn't the end of the world but that would be nice to get as clean as possible.

I left the word _form() at the end of the forms even though I prefixed them with theme_form__, which is a bit redundant but that may be for another issue.

This isn't necessary the way I wrote the FormBuilder changes but it's explicit, opinions?

-      '#theme' => 'language_negotiation_configure_form',
+      '#theme' => 'form__language_negotiation_configure_form',

This was added but if they are still theme functions to wrap the results in a form template. It isn't needed for a twig template, just done to get the patch rolling. Twig template just extends @system/

  // Temporary pass through to form.html.twig until template conversion.
  $output = _theme('form', $variables = array('form' => $output));
  return $output;

Did a rough conversion of theme_views_ui_expose_filter_form to twig template to show it's use.

The big win is no more theme functions or templates for empty theme_*_form(), form content wrappers. And you can totally override the form template if you want to or just extend it's contents with a block(the way we would be doing @see theme_views_ui_expose_filter_form() conversion in the patch())

Status: Needs review » Needs work

The last submitted patch, 1: 2265991-theme-form--sugestions-1.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
Issue tags: +Template consolidation
FileSize
3.43 KB
21.83 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2265991-theme-form--sugestions-3.patch. Unable to apply patch. See the log in the details link for more information. View

I don't think the template suggestions are picking up, see the form__block_list.

Status: Needs review » Needs work

The last submitted patch, 3: 2265991-theme-form--sugestions-3.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: 2265991-theme-form--sugestions-3.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
20.55 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,559 pass(es), 1,006 fail(s), and 42 exception(s). View

Just a re-roll.

Status: Needs review » Needs work

The last submitted patch, 7: replace_theme_form-2265991-7.patch, failed testing.

sun’s picture

The basic proposal makes sense to me, but I don't understand why this issue attempts to change #theme_wrappers into #theme?

The #theme of a form renders the inner HTML of the form only, because there's not really a use-case for customizing/overriding the wrapping <form> markup itself. Therefore, the wrapping form markup is output through a separate #theme_wrapper.

I don't see a value in embedding the wrapping form markup into form templates - doing so will only make templates more complex and cause bugs IMO.

m1r1k’s picture

Should we convert theme_menu_overview_form() as well?

joelpittet’s picture

@sun I think it was because theme_wrappers don't have suggestions, I could be wrong on this but that was the impression I was under. (also wrote that in the Issue Summary but likely in the wrong place)

@m1r1k Yes that should be on this list as well.

Cottser’s picture

Title: Replace theme_*_form() with theme suggstion for #theme => form. » Replace theme_*_form() with theme suggestion for #theme => form.
joelpittet’s picture

Status: Needs work » Closed (won't fix)

@andypost seemed to fix this in a much more direct way here #1987400: forum.module - Convert theme_ functions to Twig

Let's do that, closing this experiment down for now.