Drupal 10 compatibility fixes

Hello, your patch is based on version 1.7 (tag 8.x-1.7)?
Because branch 8.x-1.x already contains part of the patch with a fix on the use of once instead of jquery.once

https://git.drupalcode.org/project/tb_megamenu/-/compare/8.x-1.7...8.x-1...

I suggest we focus on 2.x for Drupal 10. Which most of the code is already compatible to work with D10, including dependencies handling.
here is patch for 2.x branch, I"m not able to select 2.x branch hence selected alpha release.

Feel free to move back to 1.x if you think so.

I tried the patch on comment #8. But it is giving 2 warnings in upgrade_status scan results (Added the image). So to fix it, I am adding the fixes. Access check is very common so that doesn't require any explanation (I guess). But 2nd warning is that the core/underscore will be removed in Drupal 10. There is a throttle function from underscore.js which is being used in frontend.js file. So I have added the alternative for the same as per this. Kindly review the patch.

I have reviewed the patch in comment #11, and is working as expected.
The module is now compatible to D10 , thus moving it to RTBC.

Hello,

Will a new release be created with this fixes applied?

Thanks

Thanks everyone for the assistance in getting this D10 ready. I will look to get a new release out by end of week.

Thank you all for your work on this. Really quite a nice module.

Waiting for this release and support to Drupal 10. Thank you.

Any word on the release date?

I am aiming for a release tomorrow.

What PHP version will the new release work with? 8.2?

I think there is an issue with this patch. Specifically regarding frontend.js

I almost gave up, thinking I had some kind of weird caching problem giving what happened after applying patch: 3328163-11.d10-compatibility.patch - after clearing drupal cache and browser cache when a page with main menu using tb_megamenu loaded I was getting a browser console error listing line 143 of frontend.js which broke the menu drop-downs that should appear when hovering over a main-menu link.

At first I couldn't figure out how it was possible the problem be line 143 (in browser inspector for sources) line 143 was showing:
var throttled = _.throttle(updateTBMenus, 100); which based on the patch for js/frontend.js that should be: const throttled = throttle(updateTBMenus, 100); and in that file it was line 48 not 143. If you look close you'll notice where line 143 is referenced in the error it reads 'var throttled = ...' but the patch for line 48 reads 'const throttled = ...', clearly not the same (and why I almost thought I has a really strange caching problem happening because I almost missed that difference).

So I did some digging...
One thing is I'm pretty sure at least from what I can see, is the file located at tb_megamenu/js/frontend.js isn't actually used or ever loaded anywhere.
What I found was a different file located at tb_megamenu/dist/js/frontend.js is the actual file being loaded - at least for version 2.0.0-alpha3.

I'm not sure why there would be two different frontend.js files (if it's by design or not) and how this hasn't been noticed prior considering.

I also haven't tried modifying the frontend.js file at tb_megamenu/dist/js/ with the changes made by the patch, I'll be doing that first thing tomorrow (quitting time at the office for today) and I'll report what I find.

Oh, before I forget one other thing to note is in the previous patch tb_megamenu.libraries.yml removed jquery.once for once:

-  core/jquery.once
+  core/once

- which should be done for complete compatibility with Drupal 10 (I'm not sure why that would have been removed from the last patch or if it was just overlooked).

@sidgrafix,

Good catch regarding how the library is requiring once.

The frontend.js in the dist directory is the compiled version. So yes, the js/frontend.js is not loaded, but the dist/js/frontend.js is. These JS files as well as our sass are all available in the module by design (and at the request of other users as well). Since there are changes to frontend.js, it needs to be recompiled.

There are also some elements in the latest patch/merge request that should use dependency injection but does not.

OK, found modifying the /dist/js/frontend.js (similar to /js/frontend.js) works -> but instead of using 'const' I used 'var', and as best I can tell nothing breaks anywhere.

If anyone is looking to upgrade there drupal 9 site to 10 sooner than later with this module I made a patch including everything previously patched plus added back 'jquery.once' to 'once' in the libraries.yml. Everything passes D10 compatibilty per the 'Upgrade Status' module. Side note I also (for my personal sanity) in the patch included the removal of
sourceMappingURL=admin.css.map to stop it from generating a browser console 'DevTools failed to load source map: ....' error/notice when editing tb_megamenu in drupal admin, I don't think that will hurt anything if your just using this module and not building from it.

@themodularlab your explanation helped (definitely clears things up as to why). So if one wanted to recompile the module on there own would that build the /dist/js/frontend.js file from the /js/fontend.js file that was originally modified in the previous patch? (I'm assuming by combining with other js files - correct me if I'm wrong). If that is the case is there some documentation on recompiling step by step on your own? I looked through the info at https://git.drupalcode.org/project/tb_megamenu and see the FRONT END section using NVM but it's not exactly clear (unless it is just as simple as git clone repo in repo run 'nvm use', 'yarn install' and then 'yarn develop') or is there more to it? I'm more so just curious in that regards.

Patch for 8.x version

Confirming that #4 works well and doesn't break an existing custom implementation of TBMM.

themodularlab → closed merge request !63

D10 compatibility has now been merged! Thank you to everyone who helped with this. I hope to have our first 2.x beta out this week (possibly tomorrow).

Great, fantastic!

Can you confirm whether it will be possible to apply the patch for full width to this version?

https://www.drupal.org/project/tb_megamenu/issues/3178668 →

@hassebasse,

That is a good question. It "might" still apply, but I'm guessing it will need to be re-rolled.

Hi, Thanks for updating this to work with Drupal 10. I just installed the alpha, but it's throwing an exception that breaks the menu in Chrome. I'm showing a blank IF statement related to the mobile menu break point.

There's an error here:

if (window.matchMedia("(max-width: 1200px)").matches) {
  document.getElementById("tbm-main").classList.add('tbm--mobile')
}

if () {
  document.getElementById('tbm-main').classList.add('tbm--mobile-hide')
}

I think it has to do with 125 of frontend.js

The else statement is showing up as a blank IF.

Thanks again!

@cmchurch. Are you on the latest alpha?

This is what's in the most recent release (this is the webpack compiled js file dist/js/frontend.js):

      if (window.matchMedia("(max-width: ".concat(breakpoint, "px)")).matches) {
        thisMenu.classList.add('tbm--mobile');
      } else {
        thisMenu.classList.remove('tbm--mobile');
      }

see: https://git.drupalcode.org/project/tb_megamenu/-/blob/2.0.0-alpha4/dist/...

For Drupal 9: Use https://www.drupal.org/project/tb_megamenu/releases/2.0.0-alpha6 →
For Drupal 10: use https://www.drupal.org/project/tb_megamenu/releases/3.0.0-alpha1 →

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