Minor issues in the Toolbar module.

  • In toolbar.html.twig, nav tag errors.
  • In menu--toolbar.html.twig, space errors.
  • In ToolbarVisualView.js, '(antiOrientation === 'vertical') ? true : false' can be simplified to '(antiOrientation === 'vertical'), since its returning true/false.
CommentFileSizeAuthor
#6 interdiff-2603988-2-6.txt1.67 KBheykarthikwithu
#6 2603988-6.patch726 bytesheykarthikwithu
#2 2603988.patch1.99 KBheykarthikwithu
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heykarthikwithu created an issue. See original summary.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Status: Active » Needs review
FileSize
2603988.patch1.99 KB
heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Priority: Minor » Normal
Issue tags: +Quick fix
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Title: Minor issues in the Toolbar module. » Unnecessary ternary condition in toolbar JS
Category: Bug report » Task
Status: Needs review » Needs work
Issue tags: +JavaScript

Agreed on the ternary condition. You could even remove the parenthesis.

the nav tag is fine, I even asked morten if he was ok with it. He was: #2548027-4: Follow-up add back aria-label to toolbar tray.
I don't see the problem with the menu--toolbar file.

Setting back to NW, can you roll the patch with only the ternary condition change please?

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Assigned: Unassigned » heykarthikwithu

working in this.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Assigned: heykarthikwithu » Unassigned
Status: Needs work » Needs review
FileSize
2603988-6.patch726 bytes
interdiff-2603988-2-6.txt1.67 KB

remove the parenthesis.
rolled the patch with only the ternary condition change.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Status: Needs review » Reviewed & tested by the community

All good, thanks!

alexpott’s picture

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

Committed 970cfb3 and pushed to 8.0.x. Thanks!

  • alexpott committed 970cfb3 on 8.1.x 
    Issue #2603988 by heykarthikwithu: Unnecessary ternary condition in...

Status: Fixed » Closed (fixed)

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