This issue is created for D10 compatibility of the module to provide manual patches in separation to https://www.drupal.org/project/we_megamenu/issues/3299477 that has got automated patches from the project bot that isn't fully resolving the compatibility issues.

CommentFileSizeAuthor
#14 we_megamenu-fix-php8-deprecations.patch2.08 KBp.kasianov
#14 we_megamenu-fix-drupal-10.patch20.82 KBp.kasianov
#11 Screenshot 2022-08-26 at 19.35.21.png66.57 KBfathima.asmat
#11 interdiff_7-11.txt3.77 KBfathima.asmat
#11 we-megamenu-d10compatibility-3300703-11.patch34.94 KBfathima.asmat
#8 Screenshot 2022-08-13 at 7.37.16 PM.png104.52 KBameymudras
#7 interdiff_2-7.txt20.81 KBfathima.asmat
#7 we-megamenu-d10compatibility-3300703-7.patch32.05 KBfathima.asmat
#6 interdiff_2-6.txt1.16 KBakashkumar07
#6 3300703-6.patch11.68 KBakashkumar07
#2 we-megamenu-d10compatibility-3300703-2.patch12.45 KBfathima.asmat

Issue fork we_megamenu-3300703

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

fathima.asmat created an issue. See original summary.

fathima.asmat’s picture

fathima.asmat commented
StatusFileSize
new we-megamenu-d10compatibility-3300703-2.patch12.45 KB

A patch for fully fixing all the compatibility issues raised by upgrade status is attached here.

Before patch:

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Fix later      158  Call to deprecated function menu_ui_get_menus(). Deprecated 
                    in drupal:9.3.0 and is removed from drupal:10.0.0. Use      
                    Drupal\system\Entity\Menu::loadMultiple() instead.          
--------------------------------------------------------------------------------

FILE: web/modules/contrib/we_megamenu/src/Plugin/Derivative/WeMegaMenuBlock.php

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Fix later      15   Call to deprecated function menu_ui_get_menus(). Deprecated 
                    in drupal:9.3.0 and is removed from drupal:10.0.0. Use      
                    Drupal\system\Entity\Menu::loadMultiple() instead.          
--------------------------------------------------------------------------------

FILE: web/modules/contrib/we_megamenu/src/WeMegaMenuBuilder.php

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Fix later      258  Call to deprecated function render(). Deprecated in         
                    drupal:9.3.0 and is removed from drupal:10.0.0. Use         
                    Drupal\Core\Render\RendererInterface::render() instead.     
--------------------------------------------------------------------------------

FILE: web/modules/contrib/we_megamenu/we_megamenu.module

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Fix later      617  Call to deprecated function menu_list_system_menus().       
                    Deprecated in drupal:9.3.0 and is removed from              
                    drupal:10.0.0. Use Drupal\system\Entity\Menu::loadMultiple()
                    instead.                                                    
--------------------------------------------------------------------------------

FILE: web/modules/contrib/we_megamenu/we_megamenu.libraries.yml

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Check manually 0    The 'form.we-mega-menu-backend' library is depending on a   
                    deprecated library. The core/jquery.once asset library is   
                    deprecated in Drupal 9.3.0 and will be removed in Drupal    
                    10.0.0. Use the core/once library instead. See              
                    https://www.drupal.org/node/3158256                         
--------------------------------------------------------------------------------
Check manually 0    The 'form.we-mega-menu-frontend' library is depending on a  
                    deprecated library. The core/jquery.once asset library is   
                    deprecated in Drupal 9.3.0 and will be removed in Drupal    
                    10.0.0. Use the core/once library instead. See              
                    https://www.drupal.org/node/3158256                         
--------------------------------------------------------------------------------

FILE: web/modules/contrib/we_megamenu/we_megamenu.info.yml

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Check manually 0    Value of core_version_requirement: ^8 || ^9 is not          
                    compatible with the next major version of Drupal core. See  
                    https://drupal.org/node/3070687.                            
--------------------------------------------------------------------------------

AND after patch:

drush upgrade_status:analyze we_megamenu
 [notice] Processing /var/www/html/web/modules/contrib/we_megamenu.

================================================================================
Mega Menu for Drupal 8/9, --
Scanned on Fri, 07/29/2022 - 12:52

No known issues found.
fathima.asmat’s picture

fathima.asmat commented
Assigned: fathima.asmat » Unassigned
Status: Active » Needs review
luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented
Status: Needs review » Needs work

Setting back to NW with the following review notes:

  1. +++ b/we_megamenu.info.yml
@@ -2,4 +2,4 @@ name: 'Mega Menu for Drupal 8/9'
+core_version_requirement: ^8 || ^9 || ^10

    The support range has to be ^9.2 || ^10 since drupal/once was added in 9.2.

    This means that this change drops support for Drupal 8 entirely. 

  2. +++ b/we_megamenu.libraries.yml
@@ -1,3 +1,8 @@
+external-jquery-once:
+  version:
+  js:
+    https://cdnjs.cloudflare.com/ajax/libs/jquery-once/2.2.3/jquery.once.min.js: { type: external, minified: true }
+

    We'll want to use the core once library instead.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

Thanks everyone for participating in porting day! Getting close with this issue :)

akashkumar07’s picture

akashkumar07 commented
Status: Needs work » Needs review
StatusFileSize
new 3300703-6.patch11.68 KB
new interdiff_2-6.txt1.16 KB

This patch addresses #4. Please review.

fathima.asmat’s picture

fathima.asmat commented
StatusFileSize
new we-megamenu-d10compatibility-3300703-7.patch32.05 KB
new interdiff_2-7.txt20.81 KB

Thanks for the feedback Luke. Patch and Interdiff are attached to address #4.

I would recommend that the module maintains a different branch to keep jQuery once to support versions < 9.2 and update the module's Drupal page/documentation as necessary.

ameymudras’s picture

ameymudras commented
StatusFileSize
new Screenshot 2022-08-13 at 7.37.16 PM.png104.52 KB

Tested this module on Drupal 10.0.x & php 8.1
1. The patch applies cleanly
2. Upgrade status errors have been resolved
3. Was able to enable the module but on admin/structure/we-mega-menu I see following error

Exception: Object of type Drupal\system\Entity\Menu cannot be printed. in Drupal\Core\Template\TwigExtension->escapeFilter() (line 450 of /app/web/core/lib/Drupal/Core/Template/TwigExtension.php).

ameymudras’s picture

ameymudras commented
Status: Needs review » Needs work
fathima.asmat’s picture

fathima.asmat commented
Assigned: Unassigned » fathima.asmat
fathima.asmat’s picture

fathima.asmat commented
StatusFileSize
new we-megamenu-d10compatibility-3300703-11.patch34.94 KB
new interdiff_7-11.txt3.77 KB
new Screenshot 2022-08-26 at 19.35.21.png66.57 KB

Thanks for reviewing it @ameymudras.

I have attached a patch with the fix for #9. this patch also included further fixes for explicit access checks for the entity query and PHP 8 warning regarding optional argument declared before compulsory argument on class methods.

I ave tested and confirmed /admin/structure/we-mega-menu is accessed without any issues, see attached.

fathima.asmat’s picture

fathima.asmat commented
Assigned: fathima.asmat » Unassigned
Status: Needs work » Needs review
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Issue tags: +Needs manual testing

Working during porting day.

This needs testing on Drupal 10. Here are some instructions:

https://www.drupal.org/community-initiatives/contribution-events-initiat...

p.kasianov’s picture

p.kasianov commented
StatusFileSize
new we_megamenu-fix-drupal-10.patch20.82 KB
new we_megamenu-fix-php8-deprecations.patch2.08 KB

Hi,

I did update the patch for D10 moving (8.13.0). Did remove some mess from js - lines that marked as updated but in fact that wasnt updated.
+ added patch for php8 fix issues.

So, I wanna ask - can I help you with update module to D10 core vestion?
Maybe, ..make new fork...?
7 months no updates..

Lemme know if you need help for that.
Thanks.

t1mm1 opened merge request !23

rjjakes’s picture

rjjakes commented
Status: Needs review » Reviewed & tested by the community

Patches provided by t1mm1 look the cleanest of the D10/PHP8 patches. I will merge this one and resolve conflicts for others and merge.

  • rjjakes committed 1f9da00a on 8.x-1.x authored by t1mm1 
    Issue #3300703 by t1mm1, fathima.asmat, AkashKumar07, ameymudras,...
rjjakes’s picture

rjjakes commented
Status: Reviewed & tested by the community » Fixed
rjjakes’s picture

rjjakes commented

Status: Fixed » Closed (fixed)

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

n.ghunaim’s picture

n.ghunaim commented

I'm using Drupal 9.5 and the we_megamenu backend it's not working for me.

rjjakes’s picture

rjjakes commented

Hi @n.ghunaim - can you open a new issue with details please.

heddn’s picture

heddn
Primary language English
Location Illinois
commented

Can we get that promised D10 tag release? It still isn't clear if this module supports D10 unless I visit the git repo and review composer.json and .info.yml file. Having a tag makes that so much more obvious (I assume it does support D10, right?). :pray:

pgrandeg’s picture

pgrandeg commented

I agree with @heddn, there was no new releases for more than 2 years and having a release will help to D10 implementation.
Maybe it would be a good idea to create a new branch 2.x and give support to D10 there to keep retrocompatibility with <9.5 in 1.x.
Thank you all

rjjakes’s picture

rjjakes commented

I have created a tag in git. I don't have permission to create a release on the page, but there is a D10 tag.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

@rjjakes Which tag did you create?