Suggested commit message:

Issue #1885564 by Cottser, SebCorbin, joelpittet, drupalninja99, jenlampton, longwave, aboros, trevorkjorlien, socketwench, shanethehat, mbrett5062, rteijeiro: Convert theme_task_list to Twig template.

Split from #1885564: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig

Task

Convert theme_task_list() to a Twig template

Remaining

  • Patch needs review
  • Manual testing needed - particularly for authorize.php

Manual testing steps

  • task-list.html.twig - visit update.php, look for list in left sidebar under Druplicon - "Verify requirements", "Overview", etc.

#1885714: Remove theme_install_page()
#1885850: Remove theme_update_page()
#1189822: Convert maintenance-page.html.twig to HTML5

Files: 
CommentFileSizeAuthor
#14 interdiff.txt1.56 KBCottser
#14 2329505-14.patch3.34 KBCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,872 pass(es). View
#12 2329505-12.patch2.64 KBCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,685 pass(es). View

Comments

joelpittet’s picture

Status: Active » Needs review
FileSize
3.18 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,291 pass(es). View

Here's the split.

joelpittet’s picture

FileSize
3.18 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,289 pass(es). View
2 KB

Cleanup of the preprocess and markup update to match changes in HEAD, remove parens and move space inside the span.

joelpittet’s picture

FileSize
3.71 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,609 pass(es). View
2 KB

Whoops, forgot the hook_theme() in both the above and added the wrong class, and those parenthesis are needed.

joelpittet’s picture

FileSize
1.72 KB

And wrong interdiff..argh... sorry... again.

rteijeiro’s picture

+++ b/core/modules/system/templates/task-list.html.twig
@@ -19,7 +19,7 @@
+    {% if task.status %}<span class="visually-hidden"> ({{ task.status }})</span>{% endif %}

Is this extra space inside the span needed?

Cottser’s picture

It was there before, so it might not be needed but we should preserve it for now to limit the scope of this to converting to Twig.

rteijeiro’s picture

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

Ok, then it's a RTBC. Update page works as expected and code looks good.

Cottser’s picture

Issue tags: -Needs manual testing

Great, thanks @rteijeiro :D

Cottser’s picture

Issue summary: View changes
FileSize
3.71 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,605 pass(es). View
549 bytes

One line tweak, and adding a suggested commit message to the issue summary (from the parent issue).

Cottser’s picture

Issue summary: View changes

joelpittet++

Cottser’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

I'm almost certain this will need a reroll after #2250119: Run updates in a full environment.

Cottser’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.64 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,685 pass(es). View

No major changes, it turns out #2250119: Run updates in a full environment just moved the theme function to theme.inc. That means the 'include' line in drupal_common_theme() should no longer be needed, so I removed it.

joelpittet’s picture

Status: Needs review » Needs work

@Cottser #12 seems to be missing the theme function removal.

Thanks for the re-roll.

Cottser’s picture

Status: Needs work » Needs review
FileSize
3.34 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,872 pass(es). View
1.56 KB

D'oh. Will cancel the last one. Thanks @joelpittet!

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

Thank you:) Back to RTBC as per #7

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

webchick’s picture

Also, this would be out of scope of this issue, which is just a conversion, but there's no way I would've associated a template called "task-list.html.twig" with "a list of maintenance tasks to perform." Maybe we could discuss this in a follow-up?

  • webchick committed f92c2cb on 8.0.x
    Issue #2329505 by Cottser, SebCorbin, joelpittet, drupalninja99,...
joelpittet’s picture

Status: Fixed » Closed (fixed)

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