🇩🇪Germany @marcoliver

Hi @mattea.turnbull!

Thanks for your input! I finally got around to looking into it. Would you kindly take a look at the issue fork? I have added a number-input for the delay to the settings form (under Visibility). The value you enter there is then handed over to the front end script.

Looking forward to your feedback!

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

Tested the issue fork with the change provided by @hosterholz. It fixes the error on the form.

I also fixed a small phpcs issue introduced through the last commit.

Marking RTBC.

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

Just chiming in real quick to let you know that I just spent about an hour with the new release and it looks good!

I did not encounter anything unexpected on either D10 or D11! Thanks!

Hi! I just updated the issue for against the latest state of 3.1.x-dev.

Can someone involved with this issue please verify that the issue fork still works for them?

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

Hi @uccio! Sorry for the late reply! Merged the changes. Will tag a patch release soon.

Thanks for the MR, @alyaj2a!

I decided to change the order of use statements back to the previous version, because Coder 8.3.26 has removed the use-order-sniff and I did not see any problems reported with the original order.

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

Can confirm, that the issue fork unbreaks the current 2.0.x state of the module.

Marking RTBC.

Hi @itamair,

I removed my patch to restore the original problem and then applied your patch. It also solves the problem just fine! Thanks!

Opened an issue fork with MR.

marcoliver → created an issue.

@uccio I have created an issue fork that includes npm audit and updates our node dependencies.

Since you originally raised this issue, could you please take a look if

• this branch solves the problems you encountered in your own CI
• test if PbT still works as expected on your end

marcoliver → created an issue.

Just for the record, I just tested the issue fork from https://www.drupal.org/project/gin/issues/3463796 🐛 Ensure sticky action buttons to work with modals and ajax refresh-calls Needs review and with those changes the buttons are available again.

So I'm closing this issue, since there is nothing to do in Layout Paragraphs, there is a workaround, and an upcoming fix in Gin.

Quick update: this may be addressed by this Gin issue: https://www.drupal.org/project/gin/issues/3463796 🐛 Ensure sticky action buttons to work with modals and ajax refresh-calls Needs review

Hi folks,

I looked into this a bit just now. Like @tirupati_singh I didn't have the problem when I reproduced the steps in isolation.

Then I looked at the actual project @everton137 is working on and the culprit seems to be the Gin admin theme! More specifically that new beta feature they have to Enable sticky action buttons. When that is on, the buttons for this module's modal break.

Not sure if this is an issue of this module then, or if that is something that needs to be addressed within Gin…? (tending towards the latter, though)

marcoliver → created an issue.

Never mind my last comment about the versions, it was a long day.

Reverted the last commit about the request constants. This will cause problems only when we reach Drupal 11, in which case we should probably spin-off a new major version of the module anyway.

Hi @hkirsman!

I finally found the time to look into this.

I played around with the failing tests a bit.

First of all, I decided to try and disable testing for previous Core versions.

Then I updated the failing tests. They're passing now. I also updated two constant uses again, though I'm not sure about this. Basically, that change would render this issue fork only compatible with Drupal >= 10.3.

So I might roll that back again so we can stay compatible with 10.2.

It would be cool if you could give the version from the issue fork a quick testing spin to see if I accidentally broke anything.

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

Oops, my bad! MR 8277 now targets 11.x

marcoliver → changed the visibility of the branch 11.x to hidden.

Sure thing! I created a new MR (8257) targeting 11.0.x.

Thanks @Amandeep123. Merged. Will tag a new version shortly.

Hi @mattea.turnbull! I created an issue fork and a merge request. The change seems pretty straight forward, but could I still bother you to take a look, whether your need is actually addressed?

If you're happy with the change and do not encounter any issues, I'll be happy to tag a new release shortly!

Sounds like a good enhancement. I'll take a look at the next opportunity!

Also gave the latest commit another look and I share divya.sejekan's confusion.

Ideally, shouldn't the entries in the menu be in the same order the entities would be on the entity list page? A.k.a. using the NodeType sort() function?

See MR.

Basically just Ctrl-Shift-F'ed Core for drupalGet\(.*,.*,.* and then manually combed through the results to find any offending calls.

marcoliver → created an issue.

Looks good. RTBC.

Looks good to me, even though I'm now seeing a depressingly large number of errors, but I guess this is something that could/should be addressed in something like https://www.drupal.org/project/tome/issues/3406561 ✨ Improve error messages to be clearer about the path causing the problem Active

Marking as RTBC.

Merged-in the latest 8.x-1.x commits.

MR looks good to me, since 8.x-1.12 dropped support for D8, so the core key can be dropped. Marking as RTBC.

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

Looks correct now. Marking RTBC.

Fixed one failing functional test by adding another permission so sub-menu-items would show up.

Not sure how we should deal with the remaining issue. AdminToolbarAdminMenuTest extends ToolbarAdminMenuTest. ToolbarAdminMenuTest (and a bunch of other tests in Core) use drupalGet() and set HTTP headers by sending them as a complete string ("header-name: header-value"), for example here. But drupalGet() expects the headers to be sent with the header name as the key and the value as, well, the value.

This should probably be addressed as a Core issue, no?

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

Reverted back to using the older MASTER_REQUEST constant, which is deprecated but still usable in both D9 and D10.

Will tag 3.1.33 in a moment.

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

FYC, here's an MR.

"Clear/recommended filters" buttons had no type attribute set, making them submit buttons by default. So they'd implicitly be "clicked" whenever the form was submitted, toggling the default filters.

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

Rebased MR looks good to me. Current pipeline failures seem to be unrelated and out of scope for this task, so marking as RTBC.

@enchufe Even without your patch I am seeing the sitemap.xml file being created in the static folder.

I partially agree with @samuel.mortenson insofar that the "empty" display of the XML seems to be a server issue, not necessarily Tome-related.

But the module is nevertheless lacking a way to discover and save the XSL stylesheet for the sitemap.

I have created a first attempt at a solution for this in MR 24, kindly asking for review / feedback.

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

I have created a MR with a fix similar to what @dineshkumarbollu proposed, but also avoiding the warning @jrochate mentioned.

I believe providing an empty fallback value here in case there are no target_bundles defined is fine. $field_target_bundles is used for one specific check only, which we're not going to pass anyway if $field_target_bundles has an empty value. Afterwards it is not used again.

I played with the scenario a bit and the change does fix the error but does not seem to introduce any new issues.

I'd greatly appreciate it if somebody else could give the MR a quick look.

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

Version 2.2.0: Removed the library-dependency on drupal/js-cookie and required it as a JS build dependency.

Fixed in version 2.2.0.

Fixed in version 2.2.0.

marcoliver → created an issue.

marcoliver → created an issue.

Okay, marking this as fixed. Will tag version 3.0.0 in a moment.

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

Never mind! I was missing the "Simplenews Newsletter" overrides! After importing and enabling those, I began to see the per-newsletter policy settings!

Closing this.

marcoliver → created an issue.

Moin @avogler! I took some time yesterday and opened a 3.0.x branch to specifically work with Simplenews 4.

https://git.drupalcode.org/project/degov_simplenews/-/tree/3.0.x?ref_typ...

This version of the module is aware of the difference between the signup form block and the manage form page. I also had to make some other minor changes to a few form hooks. In the end that's why I opted for a separate branch for SN4 support.

I've already asked a colleague of mine to give that branch a spin to see if there are any problems I missed or introduced in this version. I'd greatly appreciate it if you could also take a look from your side!
Thanks in advance!

Thanks for testing, and thanks for José Trindade to raising this issue! Merged. Will tag a new release shortly.

Thanks for testing! Merged. Will tag a new release shortly.

marcoliver → created an issue.

@fathima.asmat Good catch, thanks! No particular reason, I just didn't see that there was an update available.

I bumped the versions once more. The JS tests are now run in the pipeline as well.

marcoliver → created an issue.

Ah okay, I can see it now! When I am on the page to manage my subscriptions, it loads the page form, which then explodes. Okay, your change makes sense here!

But I think we'll need to do some more work on this issue then. Because right now I can be an anonymous user with subscriptions and Simplenews' manage feature allows me to view the form, but degov_simplenews has no concept of this pseudo-authenticated user yet. Thus the name fields are empty and the Terms-and-conditions checkbox appears again.

I'll see if I can spend some more time on this next week!

marcoliver → created an issue.

Hi avogler!

Thanks for your patch! I just had a look at it and I'm not sure what motivated this change: https://git.drupalcode.org/project/degov_simplenews/-/merge_requests/2/d...

From what I can tell, the function you removed is still present in the 4.0.0 branch of simplenews.

Can you please let me know your reasoning behind the change?

Updated against 1.0.x, MR is mergeable again.

I updated the MR from #12, incorporated the changes from #8 and worked on the rest of the reported issues.

drupal-check now shows me no errors!

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

Created an issue fork from the patch. Can confirm that only enabled views show up in the menu now.

Marking RTBC.

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

Synced 3.x and 3420788-search-links-always in the issue fork.

Tested the behavior and it looks fine (video attached).

Marking RTBC.

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

Looks good to me! The issue fork solves the problem described in the issue body.

Comments #7 and #8 may be valid, but are out of scope for this task.

Marking RTBC.

Kristen Pol → credited marcoliver → .

Oops, my bad, should have waited for the pipeline!

Removed the offending pager, pipeline's green now!

Okay, I updated the issue fork against the latest 2.0.x-dev and made the following changes to address the pipeline issues:

• changed the format of the deprecation error message in AddressDefaultFormatter.php (pulled the version numbers from thin air, feel free to change them!)
• phpstan-ignored the fallback \Drupal call to assign the ConfigFactory if it is missing from params, as I don't see how this could be solved via DI

MR is up-to-date again

Hi!

I created an issue fork. I took the idea from the patch and slightly expanded upon it. Mainly, instead of just not breaking the page, the widget now displays a message that the content type is not managed by a workflow.

I then added a functional test that replicates the scenario outlined in #6 🐛 Call to a member function getTypePlugin() on null in Drupal\scheduled_publish\Plugin\Field\FieldWidget\ScheduledPublishWidget->formElement() Needs work and checks for part of that explainer text.

I'm currently seeing an issue with the testing step phpunit (next major). It seems to me to be unrelated to the module itself, but maybe someone else has an idea here.

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

Was able to reproduce the warning & the patch fixes the issue. Marking RTBC.

Looks good! The level 2 parent of the current page is now highlighted as expected. RTBC.

Hi! I updated the Issue Fork against the current state of 2.0.x again (Commit). I also removed a line in the new code that changed the output and caused some unit tests to fail (Commit).

What I'm unsure of though is: we've introduced some changes that cause warnings in phpcs and phpstan (see pipeline). How do we deal with those? Fix? Or just ignore them for now, as we'll remove the deprecated stuff in a later version anyway?

Oh, otherwise the functionality itself still looks good!

With the lack of info in the issue body and @kufliievskyi's work trying to reproduce the problem, I'll close this issue.

I agree with @kufliievskyi. This seems to be solely an issue with the version constrained used to require the module into the project. Closing.

Hey, no problem at all! Thanks for voicing your concerns!