Follow-up of #1664940: [Policy, patch] Decide on JSHint configuration and part of #1415788: Javascript winter clean-up

Run jshint on the files with the configuration from the parent issue or use jshint.com with the following options: 

/*jshint forin:true, noarg:true, eqeqeq:true, undef:true, curly:true, browser:true, expr:true, latedef:true, newcap:true, trailing:true */
/*global Drupal, jQuery */

Fix any warnings or errors the tool finds.
Check manually that the fixes did not break any functionalities
Create patch and upload for the testbot.

Files: toolbar/toolbar.js

CommentFileSizeAuthor
#15 core-jshint-toolbar-1684876-15.patch1.88 KBnod_
#11 toolbar.jshint.patch1.8 KBdroplet
#10 core-jshint-toolbar-1684876-10.patch2.26 KBnod_
#3 toolbar.patch936 bytesdroplet
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Active » Closed (works as designed)
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Closed (works as designed) » Needs work 
core/modules/toolbar/js/toolbar.js: line 58, col 9, The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype.
droplet’s picture

droplet CreditAttribution: droplet commented
Status: Needs work » Needs review
Issue tags: +Quick fix
FileSize
toolbar.patch936 bytes
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Quick fix

thanks

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Issue tags: +Quick fix

oups

alexpott’s picture

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

Committed 560b809 and pushed to 8.x. Thanks!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

This broke toolbar in vertical mode. Fix is over in #1968328: Expandable children are gone from vertical toolbar.

Status: Fixed » Closed (fixed)

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

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Closed (fixed) » Needs work

New JSHint config #1995996: Update JSHint configuration.

core/modules/toolbar/js/toolbar.menu.js: line 38, col 15, 'href' is defined but never used.

core/modules/toolbar/js/toolbar.js: line 181, col 13, 'height' is defined but never used.
core/modules/toolbar/js/toolbar.js: line 231, col 12, '$tray' is defined but never used.
core/modules/toolbar/js/toolbar.js: line 248, col 21, 'oldOrientation' is defined but never used.
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs work » Needs review
FileSize
core-jshint-toolbar-1684876-10.patch2.26 KB
droplet’s picture

droplet CreditAttribution: droplet commented
FileSize
toolbar.jshint.patch1.8 KB
droplet’s picture

droplet CreditAttribution: droplet commented

#10 RTBC. :)

nod_’s picture

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

yeah, you're right. Better keep it JSHint-only. Thanks.

( edit ) haha that's a fun cross-post. I'll leave the jshint overlay issue alone :p

alexpott’s picture

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

This still appears to need work...

jshint core/modules/toolbar/js/
core/modules/toolbar/js/toolbar.js: line 228, col 14, '$button' is defined but never used.

1 error
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs work » Needs review
FileSize
core-jshint-toolbar-1684876-15.patch1.88 KB

d'oh!

removing $tray meant the $button variable is not used anymore. that'll teach me.

nod_’s picture

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

ok with patch applied no more jshint errors for the file

alexpott’s picture

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

Committed 60e162c and pushed to 8.x. Thanks!

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