🇩🇪Germany @mrshowerman

Tests are green again.

mrshowerman → changed the visibility of the branch 3048458-preload-attribute to active.

You have to tick the checkboxes below "Credit & committing" if you want to give credit 😉

Wow. No credit at all?

Wow. No credit at all?

Not sure which changes you're referring to, but I rebased anyway.

I've decided to create an MR in this case so we can better track updates to the branch.

There is no update script that enables the new module so far; I leave the decision of whether to add one to the maintainers.

mrshowerman → created an issue.

Attached patch fixes both issues.

mrshowerman → created an issue.

Rebased an addressed the @larowlan's comment.
Test fail seems unrelated.

Fixed phpcs, back to previous state.

mrshowerman → created an issue.

Rebased.

I noticed that in the HTML method, there were a few attributes added to the link that aren't valid HTML, like type="mailto". On the other hand, other valid attributes got lost in the pre and post keys that were not processed properly.

Created an MR that fixes both on top of the original patch.

LGTM.

+1 for this change.

There is a typo in the settings form description (missing comma), and the text could be optimised a little:

If checked, a button to rotate the image will appear next to each image.

Re #66: I agree that the 2 checkboxes should be moved, but since it concerns not only the new "Plays inline" box which this issue is about, but also the already existing "Muted", I'd vote for creating a separate issue for this to avoid scope creep.

We've been using this patch for a long time in various projects, and from my point of view, it is good to go.

Did a rebase. Now there's a few phpstan errors again, maybe @mohit_aghera can look into them.

Thanks a lot for the quick fix 🙏🏻

mrshowerman → created an issue.

Thanks for adding the target branch!
Rebased.

I rebased the MR.
@swentel, I added an example to the IS.

This should be targeted against 2.4.x-dev, but that's not yet available in the dropdown.

mrshowerman → created an issue.

Here's a fix.

mrshowerman → created an issue.

Re-roll.

Re-roll.

Rebased the MR and created a CR draft.
Also updated the deprecation target to 11.2.0 and the removal target to 12.0.0.
Please review.

Not sure if we need a followup for the removal of the deprecated interface.

Thanks for merging this, @finex!
But shouldn't this issue be fixed then?

Thanks for the feedback, @anybody. I applied your suggested changes.

Nice! Thanks for all the work, @mdranove. The new MR fixes my issue.
Having a closer look at the changes, I found a few things that could be improved, and took the liberty of changing them myself.

I'm also attaching a static patch that can be applied against 6.0.6 in case anyone needs it.

This looks good to go for me, thus setting in RTBC.

MR no longer applies, but it looks like the change from this issue has been included in 🐛 Create revision before changing values, set revision author and time Active , so this might be closed as a duplicate.

mrshowerman → created an issue.

mrshowerman → created an issue.

I was able to further reduce duplicate code by moving some parts of the viewElements() method into new trait methods. The slight differences between SvgImageFormatter and SvgResponsiveImageFormatter we noticed in 📌 Formatters shouldn't repeat core code Needs review today were respected, especially the cache tags handling 😉

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

Created a new MR that addresses all open issues.
Not sure what the right status should be now 🙈

Closing this as it was decided to work on this in the original issue.

@mrshowerman not sure we need to create a new issue for that, just open a new MR here with the fix.

Oops, missed that. I created 🐛 Fatal error when linking SVGs to file Active in the meantime, but I'll be happy to add the fix to this issue if that is preferred.

mrshowerman → created an issue.

@mably, I will also have a look into ✨ Use a trait to reduce code duplication in formatters Active .

Is there an open issue for this already?

Not yet. I had a quick look at this yesterday and was able to reproduce. I also have a fix ready.
Will create a new issue with that fix soon.

Addressed feedback by @grevil.

I made a few tests on several of our sites using different versions of Drupal (9.5.11, 10.2.0, 10.2.5, 10.2.7, and 10.3.6). All of them show the same error message when saving an unpublished node that has a URL alias. So I can confirm that 🐛 hook_node_grants implementations lead to a 'URL Alias' validation error when saving translated nodes. Fixed did not fix the issue.

I noticed that there is a second workaround: granting the "view any unpublished content" (provided by the module) also fixes the issue.

Updated the IS accordingly.

All feedback has been addressed, except for one thing where @mably requests input from @Grevil. Setting back to NR.

mrshowerman → created an issue.

mrshowerman → created an issue.

Rebased.
@sagesolutions, you should be able to test this now against 3.x-dev.

Fixed a bug in the "old" map, showing the wrong number of results and wrong pagination when using the new filter: the attachment view display needs to have the same exposed filters as the parent display.

@saschaeggi, see my comment on the MR. Leaving status change up to you.

Applied code suggestions and removed a line of code that I think is unrelated.
Back to review for @saschaeggi.

The current MR produced a fatal error when using the LinkitFormatter:

ValueError: func_get_arg(): Argument #1 ($position) must be less than the number of the arguments passed to the currently executed function in func_get_arg() (line 24 of modules/contrib/linkit/src/Plugin/Linkit/Substitution/Canonical.php).

That's because the formatter does not pass $options, so I quickly added sanity checks to all occurrences of func_get_arg(1).

Apart from that, I agree with @berdir. I don't understand why we are passing the options to the substitution plugin that creates the URL, and still process the fragment and query parameters afterwards. I also think this processing code has a bug (left a comment on the MR).

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

Rebased the MR onto the latest 3.x.
@mirakolous, you should now be able to apply the MR's plain diff to 3.x-dev again.

Not sure if I get what you mean by that last line, but how can this be by design?

If a site builder checks the option "Exclude textfields from autosubmit", they don't really care about those fields being nested in a fieldset or not. They just want all textfields in the form to not submit the form automatically.

mrshowerman → created an issue.

mrshowerman → created an issue.

Has there ever been an issue where we knew when it would get merged in core? 😉
I'd say we should be prepared for this getting in any time, and this module should be ready for it. As outlined in the IS, the patch supports both situations.

mrshowerman → created an issue.

mrshowerman → created an issue.

I will check this again after we have updated to 2.3.

You're totally right. That's a much better solution.

By the time I wrote that comment, the map was indeed based on the OC Map view, but we've changed that in the meantime due to the performance issues.

I've created a new page with the OC Map view: https://beta.kultino.de/node/10386
You will see that the title filter is working, even though the map does not initially show any results (again due to the aforementioned performance issues).

Rebased. Back to RTBC.

@mrinalini9, thanks for the MR.
Had a go at the issue summary. Not sure if we need a release note snippet, thus leaving in NW state.

Attaching patch that removes the DOM traversal and replaces scollTo() with scrollIntoView().
This is just a proof of concept and a base for discussion, thus no MR at this point.

mrshowerman → created an issue.

mrshowerman → created an issue.

mrshowerman → created an issue.

Now all tests are green.

Re-rolled.

Indeed.
As @ericdsd has already pointed out, let's try to keep the scope of this issue as small as possible. You can still create a follow-up issue for that special case.

I am not the maintainer of this module.
I'm just trying to push a few issues forward that are affecting us in some projects.

@selfirian, that looks very much like the bug fixed in 🐛 contents of dropdown button go to the left and can be off the visible page RTBC .
I've rebased the MR so we have the change here as well. Can you test again?

Needs re-roll against 3.x-dev