Convert jQuery to vanilla Javascript

Removed jquery from one file. Another seems larger. But still possible.

i have a question regarding this file https://git.drupalcode.org/project/navigation/-/blob/1.x/js/navigation-b... in terms of this task

This js file is almost identical to https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/block...
Is there a real reason regarding duplication of file? Can we just provide similar HTML markup than block.js expecting? By providing all the necessary classnames and table ID (maybe some other ID, dunno)

If we are not going to fully depend on block.js, maybe we can partially depend on block.js and instead of overwriting a whole file -> we can override only necessary methods / full behaviors.

And finally - this file contains some jquery prototyping, like $.fn.drupalSetSummary , and this kind of things is defined in https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/form.js?...
It's written on jQuery, so makes sense to nest it from core and don't override on the own javascript

Same question to `navigation-block.admin.js` file. It's almost identical to core's `block.admin.js`.

@kostyashupenko you're correct that the Javascript is near identical. It's a copy and paste with "Block" replaced with "Navigation block". There are a couple of differences in that, Blocks are set per theme, Navigation Blocks are not. Also in 📌 Restrict configuration of the Navigation bar footer items Fixed , the ask is to restrict administration of Navigation blocks to only the Content area, which would further adjust the Javascript.

The general idea here is to have an independant concept of a "Naviagation Block". They are "block-like", but there are differences. Thus we want to manage our own Javascript independent of the core Block system so as not to be coupled to it as much as possible / practical. In addition, with this going into core some day, I believe (don't quote me) that we shouldn't be introducing new Javascript with jQuery.

Hope that makes sense.

There are some efforts in core in this direction.

bronzehedwick → changed the visibility of the branch 3412125-convert-jquery-to to hidden.

I pushed a patch that should address all the jQuery to vanilla conversions. Looking for help testing, as I have less knowledge of all the edge cases and test procedures. Thanks!

finnsky → changed the visibility of the branch 3412125-convert-jquery-to to active.

@bronzehedwick thanks you for work. One thing here. Why --force push was used? It became hard to control changes

I've added common library from https://www.drupal.org/project/drupal/issues/3028968 ✨ Create Javascript library for searching rendered lists on the client. Needs work and added required optimisations.

Thanks for the review @m4olivei! Taking a look at those comments now.

Why --force push was used? It became hard to control changes

@finnsky Sorry about that, I didn't know the etiquette here. I rebased the branch against 1.x, as there was some eslint related fixes there I wanted to integrate to properly lint the code here. I rebased to keep a linear history.

Is there a different protocol I should follow?

@finnsky Sorry about that, I didn't know the etiquette here. I rebased the branch against 1.x, as there was some eslint related fixes there I wanted to integrate to properly lint the code here. I rebased to keep a linear history.

Hey @bronzehedwick. I'm not an expert on the etiquette, but from what I've gleaned, its generally best to avoid force push on a branch that you didn't open, and/or after others have joined in collaboratively on your MR, either via commiting themselves, or review comments. Once we have approval on a MR and the issue is marked as Reviewed & tested by the community (RTBC), the MR will be squashed & merged, so the commit history on the MR can be as verbose as it needs to be, usually the more the better, so I would say do make changes in separate commits. Some reviewers, myself included, sometimes prefer to walk through changes commit by commit.

Got it, thanks @m4olivei! I'll follow that procedure in the future.

@bronzehedwick hello thank you for work!

Idk what to do now.
I wanted to move that filter back to separated file.
But seems you also did some changes there.
Probably you can split it to 2 commits? one for move and one for changes?

Also sadly navigationBlockSettingsSummary still not works for me.

Gonna review.

ckrina → closed merge request !160

ckrina → reopened merge request !160

I confirm navigationBlockSettingsSummary doesn't work, or the vertical tabs summary. To test it follow the videos @m4oliveiposted on his feedback in https://git.drupalcode.org/project/navigation/-/merge_requests/160#note_... in navigation-block.js.

We can continue with current code with small piece of jquery for triggering and listening jquery events. Because jquery can listen vanilla events but vanila cannot catch jquery.
Next events need to be listened/triggered.
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/form.js?...

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/form.js?...

This is looking good to me! Nice work all!

RTBC for me, but I'll leave as Needs review for @finnsky to have another look.

Looks good to me. Great work!

One thing here i would like to check deeper maybe:

I've checked table drag weights before and after patch. It works but behaves different. Not sure how critical it may be. As far as i understand that weights play some role in backend block positioning. Probably we need to test it with lot of blocks.

Before patch:

After:

We discussed in slack that it shouldn't affect backend. So let's move it to review. Thanks you team!

https://drupal.slack.com/archives/C7AB68LJV/p1710357432657529

Yass!! Merged! Great work and collaboration, team!

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