🇨🇦Canada @teknocat

teknocat → changed the visibility of the branch 3456062-the-coreversionrequirement-constraint to hidden.

teknocat → created an issue.

teknocat → created an issue.

No problem. I figured there might be other code like that, but didn't have time yet to go through the rest of the code. If I do, I'll submit additional issues and merge requests.

Sounds good all. In the meantime, I found the standard Gin sidebar works great until the new Navigation is available in core.

I was going to create a new issue for this if you hadn't already.

Yes, this is very frustrating, actually. I've enabled the experimental nav on a couple of sites to see how well it works and this is one of the first things my colleagues pointed out.

Another example is that I frequently need to go to the top-level Views admin page. It's nice that all the views appear to click on when expanded, but that can get really long, so it's more convenient (and also a bit of a force of habit for me) to just go to the top level and look for the view from there. Part of the problem is because I use the admin toolbar module and admin toolbar tools to generate additional admin menu links that we like to have there as developers.

I don't seem to find any issues with the new Drupal Navigation in the Starshot protytpe project using core 10.3 beta, but it's not using the admin toolbar tools and thus only generating the standard core links for the admin menu. Will have to see how things work out with our sites once 10.3 is fully released and we start using the final version of the navigation sidebar.

teknocat → created an issue.

teknocat → created an issue.

Yes we also figured out the license thing and made sure that we are using a different license key for each site.

I could also see various ways this code could be refactored to shorten and simplify it, so if you're interested I could create a separate issue for that and commit some changes for consideration. It would help make the code more maintainable in the long run.

teknocat → created an issue.

@djsagar if you give me push access to your forked repo I'll sort out the merge conflict. I have it fixed up on my local already.

teknocat → created an issue.

In my Drupal projects I've got "bower-asset" and "npm-asset" configured in "installer-types" and added to "installer-paths" in composer.json under [webroot]/libraries/{$name} along with with "type:drupal-library". Thus I can simply do "composer require npm-asset/fontawesome-iconpicker" and Bob's your uncle.

Actually the ->toString() isn't needed and it doesn't need to be set in a variable. It can just follow the same pattern as for media and blocks:

foreach ($content_types as $item) {
  $content_type_items[] = [
    'title' => $item->label(),
    'class' => $item->id(),
    'url' => Url::fromRoute('node.add', ['node_type' => $item->id()]),
  ];
}

teknocat → created an issue.

Why doesn't this work at all when applied as a patch to Drupal 10?

These patches are not the correct solution to the problem. The issue is that "single" and "single_all_day" options were added to the settings form, but not defined in the schema YML or provided with the default options. They also weren't displayed in the summary.

I'll create another issue fork that makes those corrections.

The last several commits to my issue fork seem to have this now working fully. Some things from patch #4 were correct and thus incorporated.

I have tested a form using test values, then edited the submission and the widget seems to be working so far.

So it seems that this widget simply doesn't function fully, with either patch #4 or the version I've put together in my branch. The default value can never seem to be set, nor does it return the selected value(s) correctly in the webform.

Inspecting the rendered form reveals why - the field name has some complexities to it that will require the implementation of setValueCallback and getValueCallback in the webform element class. This is taken care of in the Term Reference Tree widget for use with fieldable entities, so that needs to be replicated for the webform module.

I'll try to get this figured out and comment when I've got something that actually works.

Damn, I just did my own thing from scratch in my own custom module before seeing this. However, I went a slightly different way by extending the OptionsBase webform field type and using WebformEntityReferenceTrait and WebformEntityOptionsTrait. I started by looking at what the Webform Entity Checkboxes field type did, then figured out how to use the bits of that that made the most sense to turn it into a checkbox tree using this module.

I'll create an issue fork and submit a merge request with my idea so everyone can review and figure out what makes the most sense.

teknocat → created an issue.

teknocat → created an issue.

teknocat → created an issue.

Ok I'm confused now because after reviewing the code and "fixing" it in a couple of different ways, as well as re-saving some form components, it started behaving as expected and the weird issues went away. I saw how the getFormElementAccess() method works if the component is not found at the top level of the elements array and that takes care of the recursive search, so I'm not sure why it didn't seem to be finding the right elements at first and gave an error about a component that wasn't even in the list to be compared for equality.

Seems it was just something wonky about the webform config on my side, so this issue can likely be closed.

teknocat → created an issue.

I'm really glad somebody posted this as an issue, because I ran into the same thing because of some custom code I didn't realise needed updating for Symfony 6. It's not really a deprecation so much as how you need to use the request object and its functions. Very hard to otherwise find an answer on.

teknocat → made their first commit to this issue’s fork.

This really ought to be included as part of issue #3390107.

teknocat → made their first commit to this issue’s fork.

Can somebody explain to me how you apply the patch using composer and then get composer to actually install the newer square SDK dependency without using some weird trickery?

When can we expect a new release of this module with the dependency updated?

The provided patch also makes sense, given the 2 core issues have since been fixed. However, since this module is included with lightning_core, it's not possible to apply the patch as is using composer. This issues should be moved to the lightning core project and a patch for that setup accordingly.

Yes, simply running on PHP 8.2 triggers this warning. I have tested both versions and can verify it.

Plus 1 on this! I have a client with many rows to export and it canney handle it.

After reviewing the source code, I'm not even sure how the action could be updated to work with Views Bulk Operations properly. The action just wants to stream it directly out.

The patch in #5 is not really the correct solution. It prevents the error, but it doesn't make the code work correctly, which is to ensure that the live preview is disabled for the PDF display. I don't quite know why it still successfully disables the live preview, but the correct syntax should be as follows:

if ('pdf' === $view->getDisplay()->getPluginId()) {

I will create an issue fork and submit a merge request for this.

teknocat → created an issue.

I've had this module running on a site for some time now and it just started producing this error the other day.

It seems to me that it would be prudent for this module to catch and handle any type of guzzlehttp errors more gracefully, especially if it's preventing users from being able to login to their site. This error may not be a fault of this module at all. This particular cURL error could be an issue with the server environment, or a problem on Google's side, or various other things. Regardless, it does not seem acceptable from a user standpoint to just get a white page with the generic error message and have to contact the web developer to sort it out. The short term solution for the site I'm working with currently was to just disable CAPTCHA altogether on the login and password reset forms, at least in the short-term.

I think the ideal solution would be to add some configuration for what to do in case the reCAPTCHA service fails in some way that prevents it from validating the CAPTCHA in the first place. One option could be to fallback on another configured CAPTCHA, or simply ignore the CAPTCHA and just allow the form to be submitted. That way the developer can choose the most desirable behaviour for their client.

It would be even better if a test could be done against the reCAPTCHA service to determine if it's working and available and working BEFORE the form loads and displays to the user to make the user experience even smoother. Either way, regular users should not have a bad experience if there's a way to handle it gracefully and ensure the UX remains smooth.

If I have time to come up with a decent solution to this I will submit a merge request.

Oh wait I think that's just me being dumb. Should use PNG rather than JPEG to leave backgrounds transparent.

Image format selection doesn't solve the problem. I have PDFs that have transparent background, using JPEG format for thumbnails, they still have a black background. Perhaps it would be prudent to add a config option to specify a default background colour to use?

By the way, if this is ONLY an issue with the very latest Drupal core 10.1 version and the change does not work in previous versions, then it might be prudent to do a new release version of this theme that is only compatible with the latest release and update the version compatibility of the previous release.

This is not the actual problem nor is this the provided patch (#5) the correct fix for the problem.

Drupal core no longer includes the jQuery once library because it is trying to move away from the dependency on jQuery altogether in the long run. jQuery once was replaced by a new core/once library that is independent of jQuery or other third-party libraries. If switching affix.js back to using jQuery once works for you then that library must be getting included in your particular Drupal site, either directly by you because you chose to include it or because another module still depends on it and has it as a composer dependency and thus includes it.

I found that affix.js stopped working for me as of the upgrade to Drupal 10.1.2 (from 10.1.0, so it's possible that 10.1.1 breaks it as well). After some investigation, I found that in this version of core, the once library has changed a bit, such that it is no longer possible to get the matched element using "this" within the foreach function. Instead, you have to update use of once to get the element passed into the function.

Lines 29 and 30 of affix.js just need to be changed to this:

      once('affixed', '[data-toggle="affix"]', context).forEach((element) => {
        var ele = $(element),

And Bob's your uncle. I will try to submit a merge request with the changes so the diff from that can be used as a patch.

OK, apologies for making a fuss, seems it had something to do with caching - browser and/or Drupal.

What's even more weird is that I have 3 different sites I can see this problem with, but one where I installed all the latest updates and it works just fine. I will have to look into what's different between them to see if I can narrow it down and determine whether it's actually still a bug in this module, or a conflict with something else.

Oh and I'm using version 2.2.4 of this module.

This issue still does not seem to be fixed.

Testing on the latest Drupal 9.5 as well as Drupal 10.1 and I get:

Drupal 9.5.9:
TypeError: this.editor.plugins.get('LinkUI')._createViews is not a function. (In 'this.editor.plugins.get('LinkUI')._createViews()', 'this.editor.plugins.get('LinkUI')._createViews' is undefined) (anonymous function) (ckeditor5.js:201)

Drupal 10.1:
TypeError: null is not an object (evaluating 'this.editor.plugins.get("LinkUI").formView.extendTemplate') - editorAdvancedLink.js:1:3969

Disabling the module is the only thing that fixes the problem.

While the PNG option worked fine on my local machine on macOS, I found I still got a black background on my web server. This could just be due to the version of ImageMagick, so if that's the case then the first patch on this issue is better and it could be expanded on later if needed.

The only problem I see with this patch is that it does away with the use of the Spatie PDF to Image library, which accounts for various other things and additional options during the process of generating thumbnails, including an allowance for generating thumbnails for remote images.

It is advantageous to keep using the spatie pdf library, but there isn't an easy way to use that and composite it onto a white background outside of that library (I tried that). However, if it uses the PNG format then transparency is allowed and you don't get a white background. This introduces a different issue, in that the thumbnails might not look good against whatever background colour they are on, but that problem can be easily resolved with CSS.

It's a very simple solution and, if desired, the settings form could be updated to allow the developer to choose the desired format for any given thumbnail format. However, that means you'd then have to make sure you set it consistently in all places.

As such, I suggest simply changing it to use PNG format as a default and I will submit a merge request with that solution for people to try out.

Ah yes thank you! This is very helpful and hopefully helpful to others as well.

Yes I have the global minimum stability set to "dev" because I sometimes have to use dev versions of modules for one reason or another. I didn't know you could still specify the desired stability for individual packages. So now I've done that and it's solved the problem.

teknocat → created an issue.

Yes I agree there is no reason for this module to wipe out all the submit handlers just to be able to do it's thing. @prudloff's solution looks great. I don't think I would have thought of that.

Ah yes, indeed I was using the patch from that related core issue.

If I remove that patch, then I can use the latest version of this module without issue and that provides the reset permissions and it all works.

So options for anyone until the release of 10.1.x and the appropriate update to this module to go along with that are either:

a) Use the core patch and use a workaround to continue using version 3.3 of this module, or
b) Don't use the core patch and use the latest version of this module

While there are other related issues that would be ideal to solve in core all with one solution, in the meantime I think the Media Library Edit contrib module is a great solution to this particular problem. It works very well, supports D10 and appears to be fairly actively maintained.

I'm sorry I don't know how to update the automated tests, so that will likely need work, plus some other tweaks before these changes can be accepted. However, upon apply my second patch to my current live D10 installation, it does work and the status report page does indeed show it as correctly configured. It says it's using Libraries API files even though I'm using the composer merge plugin, so that's one of the minor things that might be prudent to update as well.

But suffice it to say, the patch works to eliminate errors and get it working fully in D10 for now.

My previous patch would not apply against the 8.x-2.4 release of the module, only against the current 8.x-2.x branch.

Here is a patch that will apply against the 8.x-2.4 release.

I have pushed a commit to my issue fork and attached a patch for use in the short term.

These changes seem to work on Drupal 10 so far, but will need testing on other Drupal versions, as well as a review by the community to determine if this seems like an appropriate solution for optimal compatibility.

I think it might be worth creating a new D10-only branch, however, so that dependency injection can be used properly in the form class instead of \Drupal::service() to load the libraries directory file finder service available in newer Drupal versions.

So after investigating further I understand that in order to continue to use the libraries API the cropper library would really need to be committed to the libraries registry. As that doesn't appear to have been done yet, using libraries doesn't seem like the best short-term option.

For maximum compatibility and to allow developers to choose their preferred option, I would suggest allowing for the use of the composer merge plugin to install the library locally from bower-assets. That can be one option, with libraries as a second, falling back on the CDN if it's not available locally.

I'm suggesting libraries as the second option to prevent the library definition not found error if it tries to use that method first. I have looked at how the Dropzone module checks for it's requirement of the dropzone js library in it's hook_requirements, which provides optimum compatibility (though it is still using some deprecated functions).

I will submit a patch once I've found a decent solution, though it won't guarantee that it'll work with libraries until somebody else can fully test it that way.

After installing this patch it looks like it's still not enough to have it fully working in Drupal 10.

The next error is: Drupal\libraries\ExternalLibrary\Exception\LibraryDefinitionNotFoundException: The library definition for the library 'cropper' could not be found.

I will try to submit another patch once I can figure this out.

It looks like this is now removed in Drupal 10, yet this module says it is D10 compatible. It did seem to be fine in Drupal 9, right up to 9.5, so I guess they haven't removed the deprecated functions until D10.

The patch is fine for now, but this should ideally be addressed quickly with a new release that drops D8 support and applies the changes for D10.

It would be really great if this could be merged and deployed as a new minor release version, as the ONLY thing needed is a change to the .info file.

I created an issue fork, but I'm not sure if I can do anything with it from there.

teknocat → made their first commit to this issue’s fork.

Would it also make sense to take this opportunity to switch over to semver for D10 and perhaps release it as 2.0-beta1? It would be nice to see all modules do away with the 8.x- prefix on their version numbers, but there does of course need to be an appropriate point at which to do that.

Just a thought. I've seen some other modules do this, but then the new version is D10 only while they still maintain a separate branch for continued D9 support. I don't see that as necessary unless there's a desire or need to offer a greater range of backwards compatibility, such as D8 versions that don't support the latest core APIs.

Patch file for use until a new release is available.

I find that this module works sufficiently well in it's current beta state in D9 (no issues for my company's use cases so far) and using the patch for D10 also has it working no differently than the D9 version, at least as far as I can tell.

This module is currently in a beta release only, so there's an expectation that it still has issues to work out, these issues are known and being worked on and have been discussed here.

As such, why block a D10 compatibility release just because of these known issues? Why not do another beta release for D10, with these known issues still in progress?

It doesn't seem necessary to block that until everything is fully resolved and you have a non-beta release ready to go.

So if this is closed and fixed, when will it be published as a new version?

If you're concerned about continuing with older D8 support, then just roll a whole new version for D10 and keep the older one around as well.

Ok, it seems I may have misunderstood what changed, as looking back it seems the bulk upload has always needed the "create media" and "dropzone upload files" permission combo. I'll have to dig around a little bit more, it seems.

The permissions provided by this module should also account for the fact that the core taxonomy module now provides a permission to reset any. As such, maybe this should actually be removed altogether from this module?

teknocat → created an issue.

Here's a new patch for version 1.7 of the module that allows styling according to https://developer.squareup.com/docs/web-payments/customize-styles.

I still found that it wouldn't respect certain styles, but this is because of a stylesheet loaded in by Square that has !important, like for the border colour of the input container. However, it does seem to be setting everything correctly according to the API documentation, so that's something that'll have to be fixed on the Square side of things.

I see this is no good against the latest version, which is using the newer API.

I'll work on a new patch that works with that version.

teknocat → created an issue.

By the way, I think a fix for this will go very nicely alongside a solution to https://www.drupal.org/project/lightning_media/issues/3097516 ✨ Bulk media upload should have an option to select the target media entity bundle Needs work .

I noticed the same problem and then discovered why it had disappeared.

My all powerful role could see it, but not any other users. I also noticed that the old permission for lightning media bulk upload was removed, so I took a closer look at the module's routing file. I saw that it now requires the user to have both "create media" and "dropzone upload files" permissions.

This is no good for us because we do not want to necessarily grant general "create media" permissions to all users. Yet we still want those users to be able to bulk upload media for the types they do have permission to create. The old permission did the trick nicely for this.

I can see the value in having a more robust permission check, but I think it needs to have a permission handler function, rather than just a list of permissions, so that it can check for all the individual media type create permissions as well as the general "create media". If I can come up with a patch to take care of that I'll submit it when I have time.

Upon reviewing the test failures of my patch, it looks like that's nothing to do with the patch's functionality itself. Seems that the test environment being used doesn't include dropzone for testing bulk upload.

As such, I don't think that failure necessarily means there's an issue with the patch nor should it hold up further review of this to determine if it's something that should be incorporated fully.

That latest patch won't apply against the 5.x branch, plus it doesn't strictly comply with Drupal coding standards.

Attached is one that should work for the 5.x branch.