Support 9.x version of Mmenu

Created on 28 January 2022, over 2 years ago
Updated 5 June 2023, about 1 year ago

I was reading through the changelog in 9.0 version of Mmenu and it seems really interesting: https://mmenujs.com/whats-new.html

There are accessibility related fixes for example and it now supports translating the menu texts.

Has there been any plans to make the current system compatible with this?

I tried to replace the currently used 8.x branch with the new one. It seemed to mostly work but for example styles are a bit different than in responsive_menu.css. The style changes seem to be mostly related to the fact that Mmenu is now using BEM styled class names to change the state styles.

✨ Feature request
Status

Active

Version

5.0

Component

Code

Created by

🇫🇮Finland HeikkiY Oulu

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States mortona2k Seattle

    There are style changes in patch #6, looks like they're related to fixing issues in Gin.

    One of them is pretty heavy handed:

    html {
      overflow: hidden;
    }
    
  • 🇺🇸United States mortona2k Seattle

    I have this up and running. With the 4.x branch, the toolbar moved when the menu opens. I adjusted it so that the toolbar is static and the responsive menu is under it.

    Here's the changes I made to config and options. I don't see a change record for this, so I suspect that it was never working properly!

    Here's the docs on config/options. We were missing the 'page' key.
    https://mmenujs.com/docs/core/off-canvas.html

    In addition to that, these are config, not options.

    -        if (settings.pageWrapper) {
    -          options['offCanvas']['selector'] = '.responsive-menu-page-wrapper';
    -        }
    -        else {
    -          options['offCanvas']['selector'] = settings.offCanvasSelector;
    -        };
    -
             const config = {
               classNames: {
                 selected: 'menu-item--active-trail'
    +          },
    +          offCanvas: {
    +            page: {
    +              nodetype: 'div'
    +            }
               }
             };
     
    +        if (settings.pageWrapper) {
    +          config['offCanvas']['page']['selector'] = '.responsive-menu-page-wrapper';
    +        }
    +        else {
    +          config['offCanvas']['page']['selector'] = settings.offCanvasSelector;
    +        }
    

    As I mentioned in https://www.drupal.org/project/responsive_menu/issues/3246904 ✨ Specify the page wrapper and override output Needs review , we should probably be using .dialog-off-canvas-main-canvas. Somewhat related is how we adjust the toolbar height, since it all has to work together. I changed ToolbarVisualView.js to also set padding-top on .mm-menu. .mm-menu has a fixed position, so it needs the padding set on it explicitly.

         $('.mm-page').css({
           'padding-top': this.model.get('height')
         });
    +    $('.mm-menu').css({
    +      'padding-top': this.model.get('height')
    +    });
    

    The original toolbar function sets the padding top on html and body, which still works the same as setting it on .mm-page. I think we have to decide how elements should be nested by default so it works out of the box. By using .dialog-off-canvas-main-canvas as the page selector (or .responsive-menu-page-wrapper) the admin toolbar is NOT wrapped in the .mm-page, which I think is correct. But it is fixed position, so doesn't matter so much visually.

  • 🇺🇸United States mortona2k Seattle

    Remaining work is to check the BEM classes and removed open:start/close:start hooks in responsive_menu.config.js.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    Build Successful
  • @mortona2k opened merge request.
  • 🇫🇮Finland HeikkiY Oulu

    I tested this.

    Updating our module from 4.4.2 to 5.0.x branch and applying the patch works fine.

    Also the actual functionality seems to work fine. Of course all our overridden styles need to be remade and also we had some JS functionality which doesn't anymore work with 9.x branch but that is to be expected.

    To me it seems like the actual mmenu.js works fine after the update and just needs the customizations to be redone to work with the new codebase.

Production build 0.69.0 2024