🇦🇺Australia @darvanen

Fantastic, thanks @FloGe.

Code looks good to me, just want another report of manual testing and I'll push it out.

I'll leave this here for a day or two to allow for feedback if need be as I've only just been added as a maintainer, but only that long if there are no comments because I'm planning to make a release as soon as this is in.

I have tested this manually in Drupal 11, basic behaviour remains the same and there are no errors, warnings or messages in watchdog or container logs.

darvanen → created an issue.

That sounds like config validation might be the cause. I'm not 100% sure how that works with text filters, the setting is mentioned in the Annotation of the text filter plugin but maybe it needs to be in the schema file too?

If it's blocking changes then I reckon this is Major.

Yeah sure, looks good to me and happy to support that. If you could just document it in the readme please?

Ideally this would have a test too now that we're finally starting to build out a suite of them.

Also patches are becoming a thing of the past, please feel free to make a merge request :)

Alright let's close this, feel free to reach out in the #frontend channel of Drupal Slack, I'll keep an eye out :)

My thanks @marco-s and @apaderno 🙇‍♂️

Nice ideas @bobi-mel, on the whole I think that sounds good, the only thing I would add is to use form states to make the text field only visible if the checkbox is selected.

I don't see why we can't test it with an admin theme?

@mortona2k did you end up solving this? The issue queue may be slow but at least it doesn't require timezone alignment :)

If you haven't solved it maybe you could share your libraries.yml snippet and vite config?

I suggest this ticket can be marked 'fixed' and @segovia94 credited

As of 4.2.0 the slide-element library is now bundled into the built code. It is no longer necessary to mess around with CDNs or downloaded libraries. I'm updating the readme to reflect this.

Having trouble with the pipeline for 4.x.

I can cut a release for 4.2.1 in the interim though.

@larowlan came up with the solution for this in Slack:

you want this
theme/dist/collapsiblock.js: { minified: true, attributes: {type: module} }
which causes it to render as <script type="module">

Thanks for the detailed report @cainaru, I'm working on finding a fix for this now. It was caused by the move to vite from gulp and my less-than-perfect understanding of vite config.

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

Removing out of date tag

+1 RTBC

We installed the patch, followed the readme and were up and running in no time! Thanks for the great work on this

larowlan → credited darvanen → .

Moving to project ownership queue per standard procedure.

Forgot to unassign

@bobi-mel no apology needed, I can see it was just lost in translation.

2: Great, I look forward to hearing your ideas.

@bobi-mel, answers to your questions:

1. I'm sorry but I don't understand what you mean by "chart" here?
2. Same issue.
3. I think Gin theme has a dark mode and is quite popular, let's see if that meets our needs.
4. I don't have any specific points to make, just try to uphold Drupal's values.

As for 5, here's my current vision:

• Replace the current .png images with .svg images with fill="currentColor".
• Provide an interface where the admin can set the color of the indicator.
• Write a test for the new feature.

Bonus points for:

• Provide additional built-in glyph options commonly used for dropdowns.
• Create optional integrations with fontawesome → and/or material_icons → (probably better as a follow-up).

I am very happy to discuss if you have different ideas.

No worries mate, glad it's working for you :)

Gladly.

However I am on vacation at the moment, I do not know if I will have the attention for it. Please give me a couple of weeks grace just in case ☺️

darvanen → created an issue.

Hi folks,

This looks like the kind of issue that could reach well over 500 comments before we come to an agreement so with the frontend framework manager's support (@nod_) and help from a core committer (@larowlan) I've kicked off a community initiative because I would *really* like to see this happen.

I've built a prototype using php-forge/foxy and a new contrib module → but I'm not at all wedded to seeing it happen that way. It's just a prototype but so far I *think* it covers all of the pitfalls of previous attempts, I'd really like people to poke holes in it.

If you're still interested in this issue please accept my humble invitation to join us over at #frontend-bundler-intitiative in Drupal Slack → .

I found a moment to check this and followed these steps:

• Open my D10 site that has the token_filter installed on the 'Advanced HTML' text filter
• Checkout 2.2.0
• Go to the text filter edit page
• drush cr
• Refresh the text filter edit page
• Checkout 2.1.0
• refresh the text filter edit page
• drush cr
• refresh the text filter edit page
• Checkout 2.2.0
• drush cr
• Refresh the text filter edit page
• Save the text filter
• Go to an article edit page that uses the text filter
• Save the article

I didn't see any errors at any point.

Can you provide steps to reproduce from a fresh Drupal install please?

Thanks for reporting this @darkodev. I’m in transit right now, I’ll look at this as soon as I can if someone else doesn’t get there first.

We did update the JS dependencies and rebuild the script, and “it worked on my machine” 😅

Really gotta get some more tests on this module.

As it’s a plugin issue can I just double check that you’ve cleared your cache?

Updating my attribution

Updating my attribution

darvanen → created an issue.

Ok, had a look at https://github.com/ckeditor/ckeditor5-dev/releases/tag/v32.0.0 and decided to remove @ckeditor/ckeditor5-dev-utils as I could not see anywhere it was being used in scripts or imports. Also updated all dependencies and audit now shows zero deprecations. Building the JS library makes no changes to the built code.

Brilliant work mate, thank you.

Looks like we just needed to compile the JS code. Manual testing passes now.

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

I will definitely consider co-maintainership if you can solve #936984: New graphic for open and close → in a robust and flexible way.

If this issue was a way to ask me for more support, please feel free to close it. I will do so after a few months if there's no further action here :)

@anand.tshniwal93 I've come back to review this and I have to ask - how are you using composer to install an npm dependency? That is not something supported by composer.

I'm going to mark this postponed for a minimum of 1 month for further feedback, and potentially close as "works as designed" after that.

I'd encourage anyone having issues with JS dependencies to check out a work-in-progress solution to this problem at https://github.com/darvanen/drupal-js

The latest test agains Drupal 11 did not fail.

Closing as "works as designed" per previous comment. Feel free to re-open if you have new information to share.

Closing as "works as designed" per previous comment. Feel free to re-open if you have new information to share.

@Yan2020 when a theme doesn't work with this module it is typically because their block templates don't include the standard title prefix/suffix as seen in the core block module's base template:

  {{ title_prefix }}
  {% if label %}
    <h2{{ title_attributes }}>{{ label }}</h2>
  {% endif %}
  {{ title_suffix }}

This module relies on those inclusions, which was a very big improvement on the previous method the module relied on to target blocks using CSS targets. You have two options:

1. Create a sub-theme of the druadmin theme and create your own block template(s) that implement the title prefix/suffix.
2. Contribute a patch to the druadmin theme that implements the title prefix/suffix in the block template(s) in that theme and use a patch of that contribution in your own project (recommended).

Marking this issues as Support Request since the issue lies outside this module. I will keep this open for at least a month to allow for further discussion.

darvanen → created an issue.

Also took a moment to reduce the test footprint of the module to fall in with emerging advice about when to run 'next major' etc test.

darvanen → created an issue.

That's good enough for me, we probably could have done better test coverage for those last problems, but I'm happy with where it is for now. If they break in a later release I will insist on tests.

Going to make this a minor RC.

This is *fantastic*, thank you.

I look forward to seeing it in a release <3

Might be time to switch to vite.

@bgustafson do you think the change to the test in that commit provides sufficient coverage?

Alrighty, everything worked until I rebuilt the code with the altered JS then the CKEditor instance failed to initialise and the browser reported this:

CKEditorError: plugincollection-plugin-not-found {"plugin":null}
Read more: https://ckeditor.com/docs/ckeditor5/latest/support/error-codes.html#error-plugincollection-plugin-not-found
    at ckeditor5-dll.js?v=40.2.0:5:110188
    at ckeditor5-dll.js?v=40.2.0:5:110247
    at Array.forEach (<anonymous>)
    at u (ckeditor5-dll.js?v=40.2.0:5:110049)
    at l.init (ckeditor5-dll.js?v=40.2.0:5:108537)
    at D.initPlugins (ckeditor5-dll.js?v=40.2.0:5:115287)
    at js_SiUtCm46YLOYMNhHO4sPUmdx9_ZqVt-kXQc_l3wcO6E.js?scope=footer&delta=2&language=en&theme=claro&include=eJyNU9tuwyAM_SGaPO0DNql9mrRPiBziJKyAKZhu7dcPktAurVTlJfHl-GDsgyTL-MsRdN356EBX8hbZaWWPQUjyWFvyBrS6opAaPNWDpjYhAl8SaJgxCwFIVmecihewpQ53fWIQ2Sq4bK8KJWkNLqyDEJkkGaeRUTjgsSSynZDGoOU7w-SK9IlNVCW8uCvewMAYRK80oy_B2RPyiJ1i8m-1SrPwNmXKASWzBVOhcSMEFTaBRzZ6bwdlcRNcGRiWSd2DeVh5zsCPmVaTPJ4i8VPNvKfHWHhicBAYD57MV98r-UQTKHqJ--RNethwA5n2_5Hb2oRmaPXToSN5dU16Bf05zW2d9WjojId5HkxHtM2y7tlpPf2EtO4s96KAbGdR4D9JLMS33r5PEf2lmgQ9C7woGs671GcQITXGMt5Yi5-6IN1CbmD6ixju4st2pdLzy0_uxasstdAZZZsHxoo94otUGtk5X_khhUGCw_dcJlo1NE45rIvxBxRnl5Y:5:9630
    at new Promise (<anonymous>)
    at D.create (js_SiUtCm46YLOYMNhHO4sPUmdx9_ZqVt-kXQc_l3wcO6E.js?scope=footer&delta=2&language=en&theme=claro&include=eJyNU9tuwyAM_SGaPO0DNql9mrRPiBziJKyAKZhu7dcPktAurVTlJfHl-GDsgyTL-MsRdN356EBX8hbZaWWPQUjyWFvyBrS6opAaPNWDpjYhAl8SaJgxCwFIVmecihewpQ53fWIQ2Sq4bK8KJWkNLqyDEJkkGaeRUTjgsSSynZDGoOU7w-SK9IlNVCW8uCvewMAYRK80oy_B2RPyiJ1i8m-1SrPwNmXKASWzBVOhcSMEFTaBRzZ6bwdlcRNcGRiWSd2DeVh5zsCPmVaTPJ4i8VPNvKfHWHhicBAYD57MV98r-UQTKHqJ--RNethwA5n2_5Hb2oRmaPXToSN5dU16Bf05zW2d9WjojId5HkxHtM2y7tlpPf2EtO4s96KAbGdR4D9JLMS33r5PEf2lmgQ9C7woGs671GcQITXGMt5Yi5-6IN1CbmD6ixju4st2pdLzy0_uxasstdAZZZsHxoo94otUGtk5X_khhUGCw_dcJlo1NE45rIvxBxRnl5Y:5:9587)
    at Object.attach (js_SiUtCm46YLOYMNhHO4sPUmdx9_ZqVt-kXQc_l3wcO6E.js?scope=footer&delta=2&language=en&theme=claro&include=eJyNU9tuwyAM_SGaPO0DNql9mrRPiBziJKyAKZhu7dcPktAurVTlJfHl-GDsgyTL-MsRdN356EBX8hbZaWWPQUjyWFvyBrS6opAaPNWDpjYhAl8SaJgxCwFIVmecihewpQ53fWIQ2Sq4bK8KJWkNLqyDEJkkGaeRUTjgsSSynZDGoOU7w-SK9IlNVCW8uCvewMAYRK80oy_B2RPyiJ1i8m-1SrPwNmXKASWzBVOhcSMEFTaBRzZ6bwdlcRNcGRiWSd2DeVh5zsCPmVaTPJ4i8VPNvKfHWHhicBAYD57MV98r-UQTKHqJ--RNethwA5n2_5Hb2oRmaPXToSN5dU16Bf05zW2d9WjojId5HkxHtM2y7tlpPf2EtO4s96KAbGdR4D9JLMS33r5PEf2lmgQ9C7woGs671GcQITXGMt5Yi5-6IN1CbmD6ixju4st2pdLzy0_uxasstdAZZZsHxoo94otUGtk5X_khhUGCw_dcJlo1NE45rIvxBxRnl5Y:22:3962)

Nice one! Gonna do the manual testing now. One question for my ongoing education: what purpose does making ckeditor5 a dependency instead of a dev-dependency serve?

Left one small comment/query :)

I have contacted the most recently active maintainer → via their contact form asking for a comment on this issue.

The owner → 's contact form is not enabled.

darvanen → created an issue.

darvanen → created an issue.

@ptmkenny I've merged the changes from 📌 Require composer-lint, cspell, phpcs, and stylelint to pass Fixed , you may wish to rebase this branch.

Yes please to ignoring compiled files.

Brilliant, thanks @ptmkenny!

Looks good to me.

@bobi-mel let's wait to fix the test until we've resolved the issue properly.

I suspect that weight alone won't be enough, we might need the delta or ID for section and column as well.

@bobi-mel by any chance did you try adding the same block to two different sections on the same node?

Thanks @bobi-mel, did the change work as intended? If so perhaps you could set it to NR and we'll see if @jurgenhaas or someone else has the bandwidth to try it out?

If it is satisfactory the last thing required will be a test to lock in that behaviour.

Opened a new 2.x branch, this is now out of date.

I think we can close this now :)

griffynh → credited darvanen → .

For what it's worth, I'm with @acbramley on this.

When I was contributing to this earlier I was yet to come across any kind of guidance on how to approach adopting GitLab CI, but senior people in the community all seem to be coalescing around "just do it and fix the lint failures later".

That said, the final decision on how to approach this is down to the maintainers of this module of course, who are yet to comment.

#46: I like this idea a lot better than my idea of generating a UUID.

My thanks 🙇‍♂️

I would like to use this namespace to work on a module for a potential community initiative to standardise JS package management for Drupal projects.

You can see the beginnings of this effort at https://github.com/darvanen/drupal-js

#43 I found re-running a test didn't work but starting the pipeline from scratch did.

I have confirmed that the errors in this issue are now occurring in the main branch pipeline, whereas they previously weren't.

The pipeline for this issue is also now passing, so this is good to merge. Thank you very much :)

Wonderful, that's a big improvement, thanks @bobi-mel.

I think we still need to address #43 as we're adding an id to each block. Perhaps we could simply generate a uuid for each block?

Thanks very much, looks good. Happy to commit this without 'next_major' passing as we have an open issue to check Drupal 11 testing compatibility when the current infrastructure failure clears.

Thanks @vishalkhode, I understand that, but the Drupal 11 (next_major) PHPUnit test on the GitLab pipeline is failing:

I do not wish to commit this until the pipeline passes.

The pipeline on the MR is failing.

I believe this is due to #3444792: Prepare for PHPUnit 10 → so well see what happens once that is in. Postponing until then, I'm watching that issue.

darvanen → created an issue.

The 'next major' PHPUnit tests are failing due to #3444792: Prepare for PHPUnit 10 → which is not at all related to this work, so I'm happy to say this looks like it's done. Thank you!

It looks to me like we've missed removing core/js-cookie from 11.x so far... what are the steps to get that to happen, or is it too late?

Thanks @chandu7929, I appreciate the update.

@chandu7929 it would seem you ran your script against something other than 4.x-dev as the core_version_requirement has been up to date since the 23rd of March on the dev branch.

I acknowledge the other two failures, but could you please:

1. present the issue summary in a more human-readable form, and
2. actually target the development branch

Hi @rocketeerbkw,

Wondering if you've had any more thoughts about this? The offer still stands, especially since the Drupal 8+ version has now disappeared from the module home page.

Regarding deprecations I checked and the answer I got was a link to https://drupal.slack.com/archives/C03L6441E1W/p1713811922586309?thread_t...

Repeated here for convenience:

catch
17 days ago
Any actual API deprecations should be for removal in Drupal 12 already unless it's blocking Drupal 11. Internal and dead code sort of things until 10.3.0 beta1

Setting to NW as we'll need to keep the deprecation code in the 11.x MR

@joachim noted re stripos still being relevant in 10.x. I agree that's resolved.

I'm going to defer to @daffie now, my timezone is about to pass midnight.

Oh of course, I'm not accustomed to looking for two MRs. Just gotta remove that same comment in the other MR and then I reckon we're good.

I just read the change record, it says this is "deprecated" but there's no BC layer or deprecation error messages, I think we need those?

Responding to a callout on Slack to review this, I have only looked over the code. Overall

Raised one tiny nit that would probably see it sent back by a committer. Once that's in I think I would be happy to RTBC based on #68.

Oh I didn't answer the first question:

I disabled the phpcs so that a follow-up could be made to turn the test on and fix the errors in a separate issue. That way the test suite in use runs green by default making it easy to spot problems in MRs. If we don't fix the other tests in this issue I'd recommend doing the same thing with those.

@ptmkenny my experience with maintaners has been that they like to have all tests passing before committing anything, though that may be more prevalent in core than contrib.

Apologies for overriding your work so soon after it was submitted, I should have looked at the timestamp, I was just trying to help.

I'm happy to split out the the work into separate issues, I just don't want to be seen as credit-gaming.

Interesting that this isn't happening in the gitlab pipeline: https://git.drupalcode.org/project/rename_admin_paths/-/merge_requests/1...

Perhaps this issue is a bit out of date?

I have set CI to skip phpcs for now I recommend making that a follow-up task if desired as the volume of errors is quite large and would only serve to delay this issue.

I fixed the CSPell errors by adding ignored words for names and two adjustments to other words - I checked that changing the name of the admin menu item doesn't cause any regressions in positioning on a site, it's ok.

PHPStan only had one error, I put a default return in the help hook to satisfy that.

Tests are all green so I'd say this is ready for review.