Issue #2151097 by joelpittet, c4rl, IshaDakota, pplantinga, gnuget, longwave, jeanfei, sbudker1: Convert theme_confirm_form() to Twig

Task

Convert theme_confirm_form() to a Twig template.

Remaining tasks

  • Patch
  • Patch review
  • Manual testing
  • Profiling

Steps to test

@todo

Files: 
CommentFileSizeAuthor
#9 screenshot_1.png29.18 KBjjcarrion
#8 confirm_form_after_new_patch.png45.27 KBchakrapani
#8 confirm_form_before_patch.png45.27 KBchakrapani
#8 convert_theme_confirm_form_to_twig-2151097-8.patch1.56 KBchakrapani
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,695 pass(es). View
#2 2151097-convert-theme-confirm-form-2.patch1.6 KBrteijeiro
PASSED: [[SimpleTest]]: [MySQL] 59,497 pass(es). View

Comments

Cottser’s picture

Issue summary: View changes

Adding a commit message to the issue summary so the folks who already worked on #1987410: [meta] system.module - Convert theme_ functions to Twig and #1898454: system.module - Convert PHPTemplate templates to Twig (comment #41 and above aka @c4rl) get credit.

rteijeiro’s picture

Status: Active » Needs review
FileSize
1.6 KB
PASSED: [[SimpleTest]]: [MySQL] 59,497 pass(es). View

Let's see but I'm not sure what I'm doing :(

Cottser’s picture

It would be nice if this was a suggestion like form__confirm_form.

MartiMcFlight’s picture

Assigned: Unassigned » MartiMcFlight
Issue tags: +SprintWeekend2014, +D8MA
MartiMcFlight’s picture

Assigned: MartiMcFlight » Unassigned

Sorry i don‘t know how to implement what Cottser requests in comment #3

Cottser’s picture

Issue tags: +Twig conversion
joelpittet’s picture

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

Some documentation fixes needed for this. If there is a decent way to not even have this template I'm open for suggestions, but otherwise lets just touch the docs up and get a manual testing.

  1. +++ b/core/modules/system/system.module
    @@ -2849,22 +2850,6 @@ function system_run_automated_cron() {
    - * By default this does not alter the appearance of a form at all,
    - * but is provided as a convenience for themers.
    

    This should be added in some respect to the twig template.

  2. +++ b/core/modules/system/templates/confirm-form.html.twig
    @@ -0,0 +1,16 @@
    + * Template for confirm form..
    

    Two periods and should follow convention for the docblock titles in https://drupal.org/node/1823416#docblock

  3. +++ b/core/modules/system/templates/confirm-form.html.twig
    @@ -0,0 +1,16 @@
    + * This template will be used when a confirm form specifies 'config_form'
    + * as its #theme callback.  Otherwise, by default, confirm forms will be
    + * themed by theme_form().
    

    Config forms do this by default, not sure the description gives any any needed information and probably just be replaced by the theme function note about the form appearence.

chakrapani’s picture

Status: Needs work » Needs review
FileSize
1.56 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,695 pass(es). View
45.27 KB
45.27 KB

Here we go. I have added a patch with the changes mentioned in #7.
Also did manual testing and uploading screenshots before and after applying the patch.
I have also verified the HTML source and found no difference.

Deleting a node before aplying patch
Deleting a node after aplying patch

jjcarrion’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
29.18 KB

The patch #8 works as expected. I attach a screenshot

galooph’s picture

The patch from #8 applied cleanly and worked as expected.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit f8ec2d8 on 8.x by webchick:
    Issue #2151097 by rteijeiro , chakrapani , joelpittet, c4rl, IshaDakota...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.