🇮🇹Italy @tanc

Created a new branch and MR as the old one doesn't apply cleanly. I don't think this method works any more as Gin has a dialog.css component which explicitly sets the background colour of the off-canvas ui dialog. It's not even using a variable, its just set to #444.

.ui-widget.ui-dialog.ui-dialog-off-canvas {
  --gin-offcanvas-active: var(--gin-color-primary);
  background: #444;
  border: 0 none;
  box-shadow: 0 0 48px rgba(0, 0, 0, .075);

So I think the solution will need to be ✨ Create an off-canvas style library Active

The new MR contains the correct overrides to ignore the base off-canvas CSS but the result isn't pretty.

Made a MR from @beatrizpais branch so it can be easily tested as a patch

I've made a small fix (newline at end of readme) and merged the current HEAD and the tests are passing. It's showing as failed due to not finding artifacts to upload but it might be because the issue fork was creating before the gitlab ci fix was introduced?

Anyway, in theory it should all be fine. Any chance of wrapping this up or does it need more work?

Adding patch for composer ref

tanc → created an issue.

No problem! Thanks for reviewing

I've added the requested documentation and marked as needs review.

Thanks @brianperry. Feel free to contribute some docs, hopefully it's fresher in your mind as you've recently done the steps so you may be better placed to write something. Even just an outline and I can help fill in the details.

Thank you! Insane how hard this is.

https://github.com/mundschenk-at/php-typography/blob/main/src/class-sett...

Feel free to add a PR with the change

You could modify the template for the off canvas menu and put in a condition so it only renders levels less than X

{% if menu_level < 4 %} for example, inserting it on line 35

Agreed, the patch is needed to make the dependencies work without the console error. As the module attribute is already added automatically to the main script I think adding it to the dependencies makes sense.

I was able to replicate the issue with Chrome on Android on your site. But I can't replicate the issue with the same browser on the menu.js site which makes me think it's something with the Drupal markup that the browser doesn't like.

It'll need some investigation but I rarely have time I'm afraid. If you want to investigate the markup and try to get it as close to the menu.js recommended markup that would be good.

Hi Jen, that would be great, thanks very much. I've assigned you as a maintainer now with full rights. Feel free to add others as needed.

I've got this working and added a MR so you can review. This basically does as @wotnak suggested, it adds a devDependencies option under the vite key. These library entries are then added as dependencies of the vite controlled library. It also adds the dev server url as a data attribute to those dev dependencies so that it can be fetched and used as suggested.

An example mymodule.libraries.yml file showing a deep file path for the dev server url:

reactapp.devmode:
  js:
    js/viteDevMode.js: {}

reactapp:
  vite:
    manifest: app/dist/.vite/manifest.json
    baseUrl: '/profiles/myprofile/modules/mymodule/modules/mydeepermodule/app/dist/'
    devServerUrl: 'http://host.docker.internal:5173/profiles/myprofile/modules/mymodule/modules/mydeepermodule/app'
    devDependencies:
      - mymodule/reactapp.devmode
  js:
    src/main.tsx: {}
  dependencies:
    - core/drupal
    - core/drupalSettings
    - locale/translations

The contents of js/viteDevMode.js:

const thisScript = document.currentScript;
const devServerUrl = thisScript.dataset.viteDevServer;

import(`${devServerUrl}/@react-refresh`)
  .then((RefreshRuntime) => {
    const injectIntoGlobalHook = RefreshRuntime.default.injectIntoGlobalHook;
    console.log(injectIntoGlobalHook);
    injectIntoGlobalHook(window);
    window.$RefreshReg$ = () => {};
    window.$RefreashSig$ = () => (type) => type;
    window.__vite_plugin_react_preamble_installed__ = true;
  })
  .catch(() => {
    console.log('Could not load RefreshRuntime from the vite dev server');
  });

I'm also wrestling with this problem at the moment. I'd really like a way for the Vite module to be able to load a library when in dev mode so that I can add the needed preamble for @vitejs/plugin-react.

@darvanen have you had any further thoughts about how to do it?

Thanks both, I've assigned JayVisaria as a maintainer

No, definitely just D8 and beyond. I guess I'll need to try again to see whether things are working after all this time. I'll put this back to postponed until I have a chance.

Just to be clear, the dependencies are JavaScript libraries. I don't suppose you have an example of a module where this is working so I can follow along?

Hi Rachel, the question is still valid, it would still be nice to be able to install dependencies of a module in some way so that it is possible to provide a demonstration in simplytestme of that module.

But it's not something I can't live without (I've managed without for the last 6 years lol!)

tanc → created an issue.

tanc → created an issue.

Thanks for testing all. I've cut a new 8.x-1.5 release

Thanks for the feedback @scott_euser

I've updated it to make things a bit simpler and just return an array of all associated entities for a given site setting. I think this approach is easier to understand and fits better with the current code (changes less). I've updated the MR.

If you're happy with this approach I'll write a test and update the README

One thing I haven't considered is that I think you can have multiple entities for a single site setting. So maybe this function should load and render all of them associated to that machine name?

tanc → created an issue.

I'm moving this over to the shield module and re-opening as it doesn't sound like a core issue and more of a shield issue. Feel free to move it back if I'm mistaken.

I guess the next thing to do is have a set of reproduction steps so it's easier to start debugging from a fresh install.

@timfletcher we're also seeing this same error. For me it consistently occurs when creating a new paragraph type.
I've narrowed it down to an incompatibility between the basic_auth core module and the contrib shield module.

Disabling one of those two modules 'fixes' the issue in my testing. From your backtrace it looks like you're also using both the shield module and basic_auth. Can you try uninstalling one at a time and confirm whether the error still occurs? Most likely this is a fault of the shield module as they have two open issues about compatibility with basic_auth module.

Here is one example: https://www.drupal.org/pift-ci-job/2684261 →

Also:

https://dispatcher.drupalci.org/job/drupal_contrib/697626/testReport/jun...

All full projects on Drupal.org can now use GitLab CI!

Can you explain how to activate it? I've added a .gitlab-ci.yml file in an open issue fork on a full project. The yml validates when checked in the Pipeline Editor in gitlab but when trying to run it with the "Validate" tab I see "Pipelines are disabled!". I also don't see a CI tab on the left menu anywhere. According to the docs page it says:

When you first commit your .gitlab-ci.yml file the pipeline should be triggered automatically.

1. Navigate to the sidebar
2. Click on 'CI/CD'
3. Click on 'Pipelines'

I don't see CI/CD anywhere or Pipelines in the sidebar.

tanc → created an issue.

Thanks everyone for your help. I've committed the patch to 4.4.x branch and created this issue to fix the failing test 📌 Fix failing test Needs review

As soon as that test is fixed we can look at a release.

tanc → created an issue.

Drupal.org's tests are very confusing... They pass locally but fail on d.o's infrastructure. I'm tempted to commit this as I've tested locally and then open another issue to fix the tests

As this module is wholly built around a 3rd party library called mmenu there definitely aren't the resources and/or desire to port any of that code to Drupal's off-canvas system.

However the issue of Drupal's off-canvas getting clobbered by our preprocess is definitely an issue to be resolved.

Tests are failing due to bartik and stable no longer being included. Updated tests and will hopefully pass.

Patch in #8 applies cleanly against 2.0.6 and fixes the issue I was seeing that is similar or the same as the issue in #5 with embedded facets and view with ajax enabled.

The page's path was being added to the url every time a facet was checked or unchecked. For example, on a node page containing embedded view and facets blocks with a url of /kalender, after checking and unchecking the events_relevant_for facet checkbox the url would end up looking like:

/kalender?q=/kalender%3Fq%3D/kalender%3Fq%3D/kalender%3Fq%3D/kalender%3Fq%3D/kalender%3Fq%3D/kalender%3Fq%3D/kalender%3Fq%3D/kalender%3Fq%3D/kalender%3Fq%3D/kalender%3Ff%5B0%5D%3Devents_relevant_for%3A101&f%5B0%5D=events_relevant_for%3A100

With this patch the url looks like this after checking and unchecking the events_relevant_for facet checkbox a few times:

/kalender?f%5B0%5D=events_relevant_for%3A114&keywords=

If you add the user cache context then the rendered_tree cache will vary per user which might not be what you want. It pretty much makes the page uncacheable for logged in users.

Thanks for these patches and your time! It looks like the tests are failing on the polyfills inclusion. I wonder whether with release 5.x we can drop IE support? Considering Drupal 10 doesn't support IE I think it's safe to remove it. It will mean removing the test, the form item and the config.

Hi dww, it would be a pleasure to have you aboard as maintainer. I've been struggling to keep up with the module as I'm no longer sponsored to work on it.

There is a work in progress branch which brings compatibility with the latest mmenu library. It would be great to get that working as well as the D10 release.

I'm currently on holiday but back next week so will pick things up with you then.