Sydney, Australia
Account created on 14 December 2010, over 13 years ago
#

Merge Requests

More

Recent comments

🇦🇺Australia darvanen Sydney, Australia

Removing out of date tag

🇦🇺Australia darvanen Sydney, Australia

+1 RTBC

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

🇦🇺Australia darvanen Sydney, Australia

Moving to project ownership queue per standard procedure.

🇦🇺Australia darvanen Sydney, Australia

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

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

🇦🇺Australia darvanen Sydney, Australia

@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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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 ☺️

🇦🇺Australia darvanen Sydney, Australia

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 .

🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

Updating my attribution

🇦🇺Australia darvanen Sydney, Australia

Updating my attribution

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

Brilliant work mate, thank you.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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 :)

🇦🇺Australia darvanen Sydney, Australia

@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

🇦🇺Australia darvanen Sydney, Australia

The latest test agains Drupal 11 did not fail.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

@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.

🇦🇺Australia darvanen Sydney, Australia

darvanen created an issue.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

darvanen created an issue.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

This is *fantastic*, thank you.

I look forward to seeing it in a release <3

🇦🇺Australia darvanen Sydney, Australia

Might be time to switch to vite.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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)
🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

Left one small comment/query :)

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

darvanen created an issue.

🇦🇺Australia darvanen Sydney, Australia

@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.

🇦🇺Australia darvanen Sydney, Australia

Brilliant, thanks @ptmkenny!

Looks good to me.

🇦🇺Australia darvanen Sydney, Australia

@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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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 :)

🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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!

🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

Thanks @chandu7929, I appreciate the update.

🇦🇺Australia darvanen Sydney, Australia

@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
🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

@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.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

@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.

🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

darvanen created an issue.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

Production build 0.69.0 2024