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

Merge Requests

More

Recent comments

🇦🇺Australia darvanen Sydney, Australia

Fantastic, thanks @FloGe.

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

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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?

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

Having trouble with the pipeline for 4.x.

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

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺Australia darvanen Sydney, Australia

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.

🇦🇺Australia darvanen Sydney, Australia

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

🇦🇺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.

Production build 0.69.0 2024