Remove theme_task_list() and call theme('item_list__tasks') instead.

Is the full theme.inc loaded at this point? I suspect it may not be.

Possibly not, but that is definitely a crappy reason to create yet another theme function. Hopefully there's a way around that?

Also, in an effort to get a better picture of issues remaining in the HTML5 Initiative, we are removing the "html5" tag from issues that are not directly HTML5-related. Note: Any issues assigned to the "Markup, CSS and JavaScript" components will still be broadcast on the HTML5 Twitter feed so that interested parties are aware and can participate.

Is the full theme.inc loaded at this point?

Yes, _drupal_maintenance_theme() loads theme.inc

Yes, the theme system should be fully operational in the installer as well as in update.php.

The only special part in theme_task_list() is this "active"/"done" text suffix to every item in the list:

   $output .= ($status ? '<span class="element-invisible">' . $status . '</span>' : '');

The suffix indicates whether a task in the list has been completed already, and whether a task is the currently active step.

I wonder whether this cannot be achieved and resolved differently -- through HTML attributes maybe?

Tagging for accessibility to hopefully get some insight from our experts.

Another trouble with $title:
theme_item_list() uses h3 but theme_task_list() uses h2

The difference is not only that theme_task_list() has the extra invisible text, but that it has done / active classes (which is what the text was added to replicate).

I don't believe that there is currently a reliable method of communicating this meaning using html elements + attributes. I'd be happy to test theories that others want to try out.

The meaning we are trying to communicate, as I understand it, is progression through a set of tasks: which are complete, which is active, which remain undone.

Wait, currently installer uses seven theme which has no sidebar_first region so no tasks are printed!
Suppose we need some magic to return back task-list

EDIT There's issue #1661814: Make "in maintenance" Seven use a responsive design

The heading level can be easily adjusted in the CSS of Seven theme (which is used for the installer and update.php by default).

Class attributes for items are supported by theme_item_list() already, so that difference presents no problem.

The meaning we are trying to communicate, as I understand it, is progression through a set of tasks: which are complete, which is active, which remain undone.

Yeah, exactly.

If all fails, we'll need a custom theme function preprocessor for theme('item_list__tasks') that injects the suffixes for accessibility - based on a non-standard additional 'active_task' theme variable.

Attached patch demonstrates how that could work.

I additionally hacked the node default page to output a task list for easier testing/debugging.

There are some other theme system issues in the queue that are actually dealing with the problem space of enabling template_proprocess_* functions for derived/subpattern theme hooks.

Closely related to that, a special preprocessor as needed here requires access to all variables that have been passed into theme() (or alternatively, properties on the render element). I'm not sure whether there is an issue for that already, but I've quickly hacked the theme() function to pass all variables forward to the process functions and actual theme function.

What do you think?

+++ b/core/includes/theme.inc
@@ -924,15 +925,16 @@ function theme($hook, $variables = array()) {
-  // If a renderable array is passed as $variables, then set $variables to
-  // the arguments expected by the theme function.
   if (isset($variables['#theme']) || isset($variables['#theme_wrappers'])) {
     $element = $variables;
     $variables = array();
+    // If a renderable array is passed as $variables, then convert all element
+    // #properties into variables that may be expected as arguments by the theme
+    // function or preprocess functions.
     if (isset($info['variables'])) {
-      foreach (array_keys($info['variables']) as $name) {
-        if (isset($element["#$name"])) {
-          $variables[$name] = $element["#$name"];
+      foreach (array_keys($element) as $key) {
+        if ($key[0] === '#') {
+          $variables[substr($key, 1)] = $element[$key];
         }
       }
     }

This make hook_theme() variable definition useless.
Suppose better to define additional variables in custom preprocess

+++ b/core/includes/theme.inc
@@ -971,6 +973,9 @@ function theme($hook, $variables = array()) {
+    // Provide the originally passed in theme hook name to process functions.
+    $variables['theme_hook_original'] = $original_hook;
+

This should solve this problem

+++ b/core/includes/theme.inc
@@ -2083,6 +2088,29 @@ function theme_item_list($variables) {
+    $active = isset($variables['active_task']) ? $variables['active_task'] : key($variables['items']);

The only question how to pass this additional variables here

+++ b/core/includes/theme.inc
@@ -2083,6 +2088,29 @@ function theme_item_list($variables) {
+    $done = isset($variables['items'][$active]);

Also just add and override defaults

    $variables['type'] = 'ol';
    $variables['attributes']['class'][] = 'task-list';

+++ b/core/update.php
@@ -325,7 +325,10 @@ function update_task_list($active = NULL) {
-  drupal_add_region_content('sidebar_first', theme('task_list', array('items' => $tasks, 'active' => $active)));
+  drupal_add_region_content('sidebar_first', theme('item_list__tasks', array(
+    'items' => $tasks,
+    'active_task' => $active,
+  )));

active task could be set here by preparing $tasks but leads to a code duplication

I think introduced $variables['theme_hook_original'] makes sense but better have predefined $variables['theme_context_data'] or a kind of...
this variable could be passed after all variables defined in hook_theme() or with it's own key

Please consider this as a prototype only.

The main challenge is that we want to allow for process functions that "sub-class" a generic theme function to achieve a more specific purpose.

That means all lists go through theme_item_list(). But a task list requires some additional massaging before the $variables are passed into the final theme function.

The idea of this patch is to allow to pass additional variables to the pre/process functions. This doesn't make the 'variables' declaration in hook_theme() obsolete. It is still used to define and supply default values for each variable.

The final theme function naturally and logically only knows about the $variables that are defined for itself. Any other variables are just simply ignored.

The added 'theme_hook_original' variable definitely pollutes the $variables some more with magic internal variables, but it is not the first variable that is being injected by default by the theme system.

I'm not necessarily defending the prototype I whipped up there; all I'm saying is that it is "a" solution for the overall problem space of reducing/merging theme functions into single implementations. There are some theme system challenges involved, which definitely need to be addressed in dedicated issues.

Very closely related to this are:
#956520: Available theme hook suggestions are only exposed when there are preprocess functions defined
#939462: Specific preprocess functions for theme hook suggestions are not invoked
#1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()

With all of that being said, there are a couple of different possible solutions to the problem space.

The most trivial one would be to redefine the task_list theme hook definition into:

'task_list' => array(
  'variables' => array(...),
  'theme function' => 'item_list',
),

The problem with that approach, however, is that it requires to duplicate the 'variables' definition of 'item_list'. And whenever I hear or see "duplicate", a nuclear bomb starts to tick, in my book.

Though what I'm primarily after is that we could as well investigate to introduce "true sub-classing" of theme hooks; i.e., allow one definition to inherit another definition. So 'task_list' would inherit from 'item_list', and would be able to specify additional variables. — And now that I said it, this particular idea is moot, because it would manipulate the definition of variables for the final theme function, but the final theme function (item_list) only knows about the variables being defined for item_list. So please scratch the sub-classing idea. ;)

Thanx a lot for deep explanation, actually there could be a lot of different implementations so we need to choose a most suitable for most cases.

From twig perspective all 'magic' should happen in preprocess functions so my prototype in patch

#12: drupal8.theme-task-list.12.patch queued for re-testing.

Sadly this has changed too much and is going to need a rewrite.

Because @jessebeach I think is still going to make the sprint I'm hoping we can advance this.

re-roll.

Reroll, combining template_preprocess_item_list() with existing one.

#18: drupal8.theme-task-list.18.patch queued for re-testing.

Where is node_page_default() now? There's still a reference:

$ grep -ir 'node_page_default' core/*
core/modules/node/node.module: * @see node_page_default()

It's just not clear to me where to insert:

        '#suffix' => '</div>',
      );
    }
+   $build['tasks'] = array(
+     '#theme' => 'item_list__tasks',
+     '#title' => t('Installation tasks'),
+     '#items' => array(
+       'foo' => 'Fool me',
+       'bar' => 'Bar me',
+       'baz' => 'Buzz me',
+     ),
+     '#theme_context' => array(
+       'active_task' => 'bar',
+     ),
+     '#attached' => array(
+       'css' => array(drupal_get_path('module', 'system') . '/system.maintenance.css'),
+     ),
+   );
    return $build;
  }

I think this one is replaced with view.
Also related #713462: Content added via drupal_add_region_content() is not added to pages

@andypost - do you know which view?

Also, is it too late for this replacement in the D8 cycle?

@mgifford suppose a 'frontpage' view so the front page is view. Please re-roll the patch

Re-rolled without node.module. I haven't done anything with the frontpage View.

Also noticed that function although node_page_default() is removed from D8, node_form_system_site_information_settings_form_alter() still references it in the inline docs * @see node_page_default()

re-roll for element-invisible change.

tagging.

updated.

tag cleanup, and title cleanup

re-roll of 28.

#32: drupal8.theme-task-list.1222254-32.patch queued for re-testing.

Not sure this addresses the updates in function update_task_list() but least the patch should apply.

#36: drupal8.theme-task-list.1222254-36.patch queued for re-testing.

get_t() was removed.. No longer needed.

oops.. bot..

dibbs

I didn't like the approach of adding the classes and extra markup in preprocess - we can do that instead before we call the theme function.

Rerolled with this new approach, and now things work with theme_item_list without that specific preprocess function that was blocked on
#1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()

Also, this fixes some untranslatable strings that were forced in from within theme_task_list in D7.

Not sure why these are tagged with accessibility review, if we leave the markup the same as it was before, do we need a new review?

#44: twig-task_list-1222254-44.patch queued for re-testing.

re-roll.

This wasn't a trivial re-roll, but....

Unassigning jenlampton - she probably wont keep working on this after 2 years :)

still needs proper re-roll and fix

Not sure did the right thing, just giving it a try.

Thanks @saki007ster for giving it a try. I just went through comparing the work you did with the patch I re-rolled in #50 with what you posted. You've simplified it a lot. Now part of that is that we've eliminated a lot already. For instance theme_task_list() doesn't seem to be part of Core any more.

My question though is how do your changes help meet the goal in the issue summary? Maybe this should just be closed? I've just given this a quick review.

@mgifford I just re-rolled the patch though it was not a trivial one then, But now I also feel this can be now closed as theme_task_list() is already no more in core which was the main aim of this issue.

@saki007ster - makes sense to me... Thanks!

Sorry that you went through the re-roll before seeing that theme_task_list() had been removed.

Really appreciate your time!