Problem/Motivation

See parent issue.

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#9 interdiff.txt3.59 KBtim.plunkett
#9 2326895-toolbar-9.patch11.12 KBtim.plunkett
#7 interdiff.txt1020 bytestim.plunkett
#7 2326895-toolbar-7.patch10.63 KBtim.plunkett
#5 2326895-toolbar-5.patch10.5 KBtim.plunkett
#3 interdiff.txt596 bytestim.plunkett
#3 toolbar-2326895-3.patch11.29 KBtim.plunkett
#1 toolbar-2326895-1.patch11.38 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Active » Needs review
FileSize
toolbar-2326895-1.patch11.38 KB

Status: Needs review » Needs work

The last submitted patch, 1: toolbar-2326895-1.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
toolbar-2326895-3.patch11.29 KB
interdiff.txt596 bytes

PHP Traits are the worst.

(If a parent and child class both use *the same exact trait* it is considered an error)

Status: Needs review » Needs work

The last submitted patch, 3: toolbar-2326895-3.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
2326895-toolbar-5.patch10.5 KB

Rerolled for now without the hook_element_info() removal in light of #2326409: Annotate render element plugins

Status: Needs review » Needs work

The last submitted patch, 5: 2326895-toolbar-5.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
2326895-toolbar-7.patch10.63 KB
interdiff.txt1020 bytes
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Needs work 
  1. +++ b/core/modules/toolbar/src/Element/Toolbar.php
@@ -0,0 +1,105 @@
+        // The id cannot be simply "toolbar" or it will clash with the simpletest

    More then 80 chars. 

  2. +++ b/core/modules/toolbar/src/Element/Toolbar.php
@@ -0,0 +1,105 @@
+    $breakpoints = \Drupal::service('breakpoint.manager')->getBreakpointsByGroup('toolbar');
...
+    $items = \Drupal::moduleHandler()->invokeAll('toolbar');
...
+    \Drupal::moduleHandler()->alter('toolbar', $items);

    I think we should inject these. 

  3. +++ b/core/modules/toolbar/src/Element/ToolbarItem.php
@@ -0,0 +1,95 @@
+   * A tray is a responsive container that wraps renderable content. Trays present

    more then 80 chars. 

  4. +++ b/core/modules/toolbar/src/Element/ToolbarItem.php
@@ -0,0 +1,95 @@
+    $id = drupal_html_id('toolbar-item');

    This is deprecated.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Category: Bug report » Task
Status: Needs work » Needs review
FileSize
2326895-toolbar-9.patch11.12 KB
interdiff.txt3.59 KB
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community

Thanks for the fixes.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • webchick committed 2461b7b on 8.0.x 
    Issue #2326895 by tim.plunkett: Convert toolbar_element_info() to...

Status: Fixed » Closed (fixed)

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