Problem/Motivation

The black toolbar is broken. There is an empty toolbar item between "Shortcuts" and "admin".

Proposed resolution

Check if this is the problem and solve it.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#7 toolbar_broken_empty-2537592-7-interdiff.txt927 bytessasanikolic
#7 toolbar_broken_empty-2537592-7.patch1.02 KBsasanikolic
#5 toolbar_broken_empty-2537592-5-interdiff.txt752 bytessasanikolic
#5 toolbar_broken_empty-2537592-5.patch887 bytessasanikolic
#1 toolbar_broken_empty-2537592-1.patch820 bytessasanikolic
tmgmt-menu.png62.03 KBsasanikolic
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sasanikolic’s picture

sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commented
Status: Active » Needs review
FileSize
toolbar_broken_empty-2537592-1.patch820 bytes

Changed the toolbar hook for adding the toolbar theme CSS via page attachments.

sasanikolic’s picture

sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commented
Issue summary: View changes
juanse254’s picture

juanse254 CreditAttribution: juanse254 at MD Systems GmbH commented

Looks good to me.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work 
+++ b/tmgmt.module
@@ -1074,16 +1074,12 @@ function tmgmt_cart_source_overview_validate(array $form, FormStateInterface $fo
+function tmgmt_page_attachments(array &$attachments) {
+  // Load the CSS only if the user has the permission to access the toolbar.
+  if (\Drupal::currentUser()->hasPermission('access toolbar')) {

This explains the if but not what we are actually doing.

// Add CSS for the Translation icon in the toolbar if the user has access to it.

Something like that?

Also, thinking about it, we should check if the user has access to the toolbar *and* access to that menu link...

sasanikolic’s picture

sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
toolbar_broken_empty-2537592-5.patch887 bytes
toolbar_broken_empty-2537592-5-interdiff.txt752 bytes

Not sure about the second permission... Is this correct?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

No :)

Same as ViewJobAccess, that's the one controls access to that page.

sasanikolic’s picture

sasanikolic CreditAttribution: sasanikolic at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
toolbar_broken_empty-2537592-7.patch1.02 KB
toolbar_broken_empty-2537592-7-interdiff.txt927 bytes

Now we're checking for any of the tmgmt permissions. If the user has at least one of them, we load the library/icon.

LKS90’s picture

LKS90 CreditAttribution: LKS90 commented
Status: Needs review » Reviewed & tested by the community

The toolbar is fixed.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

  • Berdir committed 08293a1 on 8.x-1.x authored by sasanikolic 
    Issue #2537592 by sasanikolic: Toolbar broken - empty toolbar item

Status: Fixed » Closed (fixed)

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