Part of #3153234: [Meta] Olivero JavaScript should be selecting [data-drupal-selector] attributes where possible

We need to convert second-level-navigation.es6.js to use regular JavaScript selectors to using attribute selectors where possible. For attributes, we should use

 data-drupal-selector="selector-name"
CommentFileSizeAuthor
#9 data-drupal-selector.png69.2 KBindrajithkb
#9 3213074-B-P.png60.52 KBindrajithkb
#7 js-lint.png15.15 KBindrajithkb
#6 Screenshot 2021-05-20 at 14.30.51.png208.57 KBgauravvvv
#5 3213074-5.patch12.25 KBmherchel
#3 3213074-3.patch6.58 KBmherchel

Comments

mherchel created an issue. See original summary.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Active » Postponed

Currently blocked by #3210443: Olivero: Focus after submenu close via ESC key and #3212998: Olivero: Normalize JavaScript selectors in scripts.es6.js

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
StatusFileSize
new 3213074-3.patch6.58 KB

Still blocked, but here is some initial work. The rest should be relatively easier.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Postponed » Active

Unblocked

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Active » Needs review
StatusFileSize
new 3213074-5.patch12.25 KB

This should be good.

gauravvvv’s picture

gauravvvv commented
StatusFileSize
new Screenshot 2021-05-20 at 14.30.51.png208.57 KB

data-drupal-selector is added by patch #5. Added screenshot for reference.
Can be move to RTBC.

indrajithkb’s picture

indrajithkb commented
Status: Needs review » Needs work
StatusFileSize
new js-lint.png15.15 KB

Hi @mherchel i have checked the patch #5 with yarn run lint:core-js I have found one warning with too many call-back, it would be great if we can resolve the warning for meet the drupal js standards. Other than that everything is okay.

image

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Needs review

Hi @mherchel i have checked the patch #5 with yarn run lint:core-js I have found one warning with too many call-back, it would be great if we can resolve the warning for meet the drupal js standards. Other than that everything is okay.

Let's create a followup issue to discuss this. It passes CI, and the primary goal of this issue is to refactor to use the [data-drupal-selector] selectors.

Thanks for the review though.

indrajithkb’s picture

indrajithkb commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 3213074-B-P.png60.52 KB
new data-drupal-selector.png69.2 KB

Hi @mherchel, Now i checked for "drupal-data-selector", now we can see the drupal-data-selector after the patch #5.

Before patch

image

After patch:

image

Moving to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3213074-5.patch, failed testing. View results

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Reviewed & tested by the community

Unrelated failure

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3213074-5.patch, failed testing. View results

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Reviewed & tested by the community

Another unrelated failure.

  • lauriii committed 55361f8 on 9.3.x 
    Issue #3213074 by mherchel, Indrajith KB, Gauravmahlawat: Olivero:...

  • lauriii committed 0b6aa64 on 9.2.x 
    Issue #3213074 by mherchel, Indrajith KB, Gauravmahlawat: Olivero:...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 55361f8 and pushed to 9.3.x. Also cherry-picked to 9.2.x because Olivero is experimental. Thanks!

Status: Fixed » Closed (fixed)

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