Problem/Motivation

In contrast to titles plugin IDs don't change over time. This is a problem when you translate menu links for example. Icons might disappear,
if css classes rely on those titles.

toolbar_menu_navigation_links() uses

$element->options['attributes']['class'][] = 'toolbar-icon-' . strtolower(str_replace(' ', '-', $definition['title']));

Proposed resolution

Change it to use the id of the plugin. This might change the required css classes so those needs to be adapted in toolbar.icons.css
as well.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#5 toolbar-use_plugin_ids_instead-2309501-5.patch5.47 KBgarphy
#5 interdiff.txt1.23 KBgarphy
#3 toolbar-use_plugin_ids_instead-2309501-3.patch4.25 KBgarphy
#7 interdiff.txt789 bytesgarphy
#7 toolbar-use_plugin_ids_instead-2309501-7.patch5.54 KBgarphy

Comments

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes
Issue tags: +Novice

Explained what needs to be done. I hope this is enough information.

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented
Assigned: Unassigned » garphy
garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented
Status: Active » Needs review
StatusFileSize
new toolbar-use_plugin_ids_instead-2309501-3.patch4.25 KB

Status: Needs review » Needs work

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

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented
Status: Needs work » Needs review
StatusFileSize
new toolbar-use_plugin_ids_instead-2309501-5.patch5.47 KB
new interdiff.txt1.23 KB

Fixed test that was relying on an updated class name.

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/toolbar/toolbar.module
@@ -463,7 +463,7 @@ function toolbar_menu_navigation_links(array $tree) {
     // @todo Change to use the plugin ID as class since titles might change.
     //   https://www.drupal.org/node/2310365

You should be able to drop the todo now. (maybe close the mentioned issue here as a duplicate of this one).

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented
Related issues: +#2309501: Use plugin IDs instead of titles for toolbar classes
StatusFileSize
new toolbar-use_plugin_ids_instead-2309501-7.patch5.54 KB
new interdiff.txt789 bytes

Removed the @todo and closed the duplicate issue.

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented
Related issues: -#2309501: Use plugin IDs instead of titles for toolbar classes +#2310365: Use the plugin ID as class in toolbar since titles might change
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

really nice work!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Indeed, lovely! Thank you :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 9830a27 and pushed to 8.0.x. Thanks!

  • alexpott committed 9830a27 on 8.0.x 
    Issue #2309501 by garphy | dawehner: Use plugin IDs instead of titles...

Status: Fixed » Closed (fixed)

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