🇦🇺Australia @darvanen

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.

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

I got up to level 5 without having to add too many ignores due to external code. The 'next major' phpstan job is failing due to changes introduced in Drupal 11, but the tick on the module page doesn't take the next major jobs into account so I'm happy to leave those in a warning state, especially since the phpunit next major job entered a warning state while I was working on this. Drupal 11 is a bit of a moving feast.

darvanen → created an issue.

Done.

I want to get these two done before cutting a 2.x release:

• 📌 PHPStan - enable and fix Active
• 📌 Change rules config from strings to booleans Active

Just because the constraint is used by the module for "automatically applies the configured rule-set to user email addresses" doesnt mean people won't want to use it for other purposes. I'm trying to makes something extensible, there's already a webform validation handler for instance.

Yeah we could go nuts with the tests but I think you're right, as-is is fine for now.

Really appreciate your feedback, and some of your improvements stayed so it was worth writing them so I could have a different perspective on the file. Thanks again :)
Agree this is ready to merge.

I've had a bit more of a think and realised that you reduced the scope of the 'changed' validator so it will only work on user entities, and only on the primary email field. This module is intended to be as much developer tool as an easy-to-use validator for user accounts. I've changed that particular alteration back.

I'm still pondering the tests.

Ok, tests are all green on MR 21 which is against branch 2.x.

I recommend skipping the ESLint check for now and creating a follow-up issue if we want to have that test, I think it may needlessly delay getting gitlab CI up and running on this project. I've put that skip in the pipeline file.

I've implemented a slimmer version of the template pioneered by @sime to help ensure the documentation doesn't become stale, feel free to change it back though, of course.

This is ready for review.

Sorry about the noise, I know this is a mess of my own making, working to unravel.

The MR may still work if it's edited to be against project's 2.x branch. I just can't work on it after pulling 2.x from the project, at least, not using Drupal's provided git commands.

Found a way to view it without being able to edit the target of the MR, looks like it's just the template by itself: https://git.drupalcode.org/issue/antibot-3433053/-/compare/2.0.x...2.0.x...

I'll see if I can find some time later to make yet another branch to simplify the process.

Thanks very much @Grevil, I was convinced DI doesn't work in form classes, that must be something else, like inside an Entity. I agree with all the changes, I went through so many iterations getting it to work that I missed all of those opportunities, thank you.

Looks like it's passing testing. Do you think we need to adjust the tests that were created in 🐛 Add option to only validate new registrations and address changes, exclude existing addresses (do not block user profile save) Fixed now the scope of the settings is a bit different?

darvanen → changed the visibility of the branch 2.0.x to active.

darvanen → changed the visibility of the branch 2.0.x to hidden.

The 2.0.x branch was created in the fork and now conflicts with 2.0.x in the project.

Here's the cause of one failure: 📌 When using drupalGet(), provide an associative array for $headers Needs work

Fixed this in 📌 Automated Drupal 11 compatibility fixes for multivalue_form_element Needs review because I was using a Drupal 11 test rig.