- 🇺🇸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.htmlIn 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.
- last update
almost 2 years 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.