Wesel
Account created on 7 September 2011, about 13 years ago
  • Consultant at KRZN 
#

Merge Requests

More

Recent comments

🇩🇪Germany sunlix Wesel

@aurm

The update hook also uses to entityFieldUpdateManager to uninstall the field storage.
I think this part of the update hook have to altered to the database connection.

🇩🇪Germany sunlix Wesel

The only uncertainty I have is on the isset check.
Should I change this to:

before:
if (!isset($path_should_be_tracked))

after:
if (!isset($path_should_be_tracked[$path]))

I have researched on other contrib modules like paragraphs permissions.
There is a direct check on if (!isset($path_should_be_tracked[$path])) instead the general one.

🇩🇪Germany sunlix Wesel

Hmm this is my very first contact with drupal_static.
I have adjusted the code to store the boolean in the $path wise manner.

I hope anyone can review and help here out. I am unsure if I got it right. :)

🇩🇪Germany sunlix Wesel

I close this issue due to the release of 2.0.0 right now.
There we dropped the Drupal.behavior implementation because it is not needed for third party integrations like ReadSpeaker.

🇩🇪Germany sunlix Wesel

I close this issue due to the stable release of 2.0.0 right now.

The migration path is there for the Drupal internals except the need of checking the used template.

🇩🇪Germany sunlix Wesel

The docReader feature can be added in a new 2.x minor version.

🇩🇪Germany sunlix Wesel

I tested successfully a default migration from 1.x to 2.x.

Made some minor changes to the settings form so we are good to go I guess.

🇩🇪Germany sunlix Wesel

@Anybody
I should not have a high impact. The default Drupal js library behavior is already "load the scripts and the end of the <body>" So it is nearly at DOMContentLoaded. In default Drupal installation with olivero I get in both cases a Lighthouse Performance of 100%.
In comparision with matomo, they load their event tracking without defer too.

🇩🇪Germany sunlix Wesel

Okay got it. We extended the library definition of etracker/etracker.js if event tracking is enabled in the settings.
This adds the etracker.js to our library. But it is affected by the "force moving external scripts" settings from advagg.
The source of the problem is not the advagg setting but the more the interaction with all library definitions and the removal of the defer attribute, which is mandatory.

The solution

We split the library definition of the vendor script and our default event tracking into two seperate one with dependency definition.
So we can guarantee to script order will be stable and the script will work without defer

🇩🇪Germany sunlix Wesel

Hey there,

I just came back to this because I was thinking if this is outdated.

I applied all settings from the screenshot and now I can reproduce this error.
The error occurs by set the config "Move all external scripts to the top of the execution order".
If you disable this kind of forcing the script order no error occurs.

Do you have some insights why do you have this setting enabled and by what parameter a script is considered as "external"?
I try to understand in what way we could manage the script to be "unbreakable" if there is some external forces involved.

🇩🇪Germany sunlix Wesel

This is already fixed by 📌 Automated Drupal 11 compatibility fixes for etracker Fixed and part of the current 3.0 stable release.

Thank you!

🇩🇪Germany sunlix Wesel

I added that last trailing comma because I wanted to see the test green.
It feels unusual for me. But it is allowed: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Traili...

I will find my way dealing with this :D

Ready for a merge if no one disagree. :)

🇩🇪Germany sunlix Wesel

I did a manual test on the admin-js. The cookies related is covered by the fantastic functional test suite from @Grevil.
So I think this should be good. But we can leave it here for feedback from @Grevil.

If I lint the silly string in to one line eslint want to make it multiline because of the line length. :D
The only one we can do is to introduce constants for the long selectors.

🇩🇪Germany sunlix Wesel

Hey there,

as discussed latlely with @Anybody I have adjusted the suggested solution from @Grevil a bit.
It makes no sense to have a different behavior only if there is one language installed in the system.
So we streamline the returned array for all languages.

There as in general a lot of confusing stuff in there :D
The page title get the language code as a suffix if page title suffix not empty.
But that key is never set and we do not have any kind of configuration for that.

By time the whole .module hooks should get a meaningful refactorisation. :)

🇩🇪Germany sunlix Wesel

Finished some refactoring and solving the phpstan, phpcs hints.
eslint> is the only one with some complainings but from my point of view this looks like a false-positive from eslint.
These are all multiline querySelector calls and for me it makes no sense to a comma at the end of the parameter list, because there is no further parameters to be set.

Maybe @Grevil and @Anybody has a opinion on that?

/builds/project/etracker/web/modules/custom/etracker/js/etracker.admin.js
  19:59  error  Insert `,`  prettier/prettier
  47:62  error  Insert `,`  prettier/prettier
  58:54  error  Insert `,`  prettier/prettier
  74:80  error  Insert `,`  prettier/prettier
  81:90  error  Insert `,`  prettier/prettier
/builds/project/etracker/web/modules/custom/etracker/js/etracker.js
  61:75  error  Insert `,`  prettier/prettier
🇩🇪Germany sunlix Wesel

sunlix changed the visibility of the branch 3302882-review-and-fix to active.

🇩🇪Germany sunlix Wesel

sunlix changed the visibility of the branch 3302882-review-and-fix to hidden.

🇩🇪Germany sunlix Wesel

Hey together,

I have added a draft MR to review the block entity migration to the new block plugin and settings.
Can anyone review this to see if anything works as expected?

Any feedback would be awesome.

🇩🇪Germany sunlix Wesel

I am good with the state of the config schema.
This is merged to 2.x. I will release a alpha-verison to test that out.

Currently there is no migration path for the Block-Plugin changes (replaced with another plugin ID)
That will be a follow up on the road to stable.

🇩🇪Germany sunlix Wesel

@mcaddz

Thank you for your feedback. I have discussed this with our partnered agency and considered your response.
I am fully with you. Our assesment was similiar to yours.
We leave it as it is. I think there is no full separation of the user groups with the differrent accessibility needs.
So let them decide on their own.

But nevertheless, thank you for bringing that up.

🇩🇪Germany sunlix Wesel

I think the assertions in hook_options_list_alter are not enough to make it fail proof during runtime.

By documentation it should be only a debugging/testing feature.
https://www.php.net/manual/en/function.assert.php

I opened a merge request with early return's.

🇩🇪Germany sunlix Wesel

Ah nope, I think it's worng. I am sorry. aria-disabled does not change focusability. But it must excluded.

🇩🇪Germany sunlix Wesel

Maybe aria-disabled="true" is what we are looking for in this case?
aria-disabled="true" only indicates semantically the state, but it's not by function.
This would have to manually controled for custom elements but not needed here.

Do you have an opinion on this?

🇩🇪Germany sunlix Wesel

Thank you for your investigation.
I agree with the aria-hidden due to the redundant content argument.
But disabled is not an allowed attribute for anchor elements. So I think we should go with tabindex="-1".
But the current template has an accesskey="L". I think this could be a discrepancy or?

The original state of the template come direct from the vendor documentation.

🇩🇪Germany sunlix Wesel

We have re-add authorization_drupal_roles_entity_base_field_info for the migration update hook 8004.
In the update hook 8004 the base field is queried but it will not found due to missing metadata for the field API. (the database table is there, but not the field metadata).
So to make the migration the module need to know the old base field to make the query and fetch the data for the migration.
The module have to make the migration and in a later major version it can safely removed.

🇩🇪Germany sunlix Wesel

Hey marty2081,

thank you for this hint. I think this only applies for 1.x.

In 2.x you can use a hook_preprocess_block() to alter the drupalSettings

// OpenReadspeakerBlock.php
'#attached' => [
  'library' => $library,
  'drupalSettings' => [
    'open_readspeaker' => ['rsConf' => $config->get('rsConf')]
  ]
]

In 1.x you could also use the libraries-extend / libraries-override mechanic described here: Overriding and extending libraries

🇩🇪Germany sunlix Wesel

I have Cherry-Picked from 📌 deprecation dynamic property Fixed .
@cbccharlie was totally right, it's kind of a duplicate of 3384235. The solution does apply to 1.x Branch also.

For the GitLab CI there will be a dedicated issue. This is out of scope of this issue.

🇩🇪Germany sunlix Wesel

I have re-add the hook_entity_base_field_info to make the field authorization_drupal_roles_roles available again via Drupal::entityDefinitionUpdateManager.

🇩🇪Germany sunlix Wesel

Added a review for proposing null coalescing operator. So the change will be just 1 character. :-P
Thank you @pooja_sharma for your fast interaction on this!

🇩🇪Germany sunlix Wesel

Can confirm that the change works.
It's just the missing member variable for the dependency injection introduced in 📌 Preview all image styles based on FocalPointEffectBase Fixed :-)

🇩🇪Germany sunlix Wesel

I can confirm that this update works as intended. The error message in status report belonging to entity and field definition update disappears after that update-hook.

From my point of view this could be RTBC. A new release would be very helpful. :)

🇩🇪Germany sunlix Wesel

sunlix changed the visibility of the branch 3442694-typeerror-drupalschedulercontentmoderationintegrationeventsubscriberschedulereventsubscriberpublishimmediately to hidden.

🇩🇪Germany sunlix Wesel

Hey together,

this got my attention due to our addtion of this module in some customer projects.
I think this is already fixed by #3444174: Tests fail with "Call to a member function isPublished() on bool" and we just need a new release.

So I will close this as outdated and add a issue relation.

🇩🇪Germany sunlix Wesel

mea culpa, I did make an understanding failure. Thought it is defaulting to the ImageItem default settings, which is not, because here is no field api involved in the test szenario. :-)
Thank you for pointing that out, did learn something new with testing! :)
Should be good to go.

🇩🇪Germany sunlix Wesel

@pooja_sharma

Thank you a lot for pushing this forwars with a functional test. :-)
I have changed the test cases a bit to have to specific test case for given strings and a test case with no given alt attribute.
Both cases have to result in having a rendered alt attribute.

I am unsure if we have to add a specific alt = NULL testcase, because this is not intended by the used field API here. ImageItem have default values for it's properties which should not altered or?

🇩🇪Germany sunlix Wesel

Hey together,

I have research a little bit on this and I am sure, that the patch in die issue fork solve this.
The related property here is thumbnail__alt (Database column in media_field_data which comes from the BaseFieldDefinition in the Media Entity itself.
This property/column stay's null if you upload an image to a media entity based on the image source plugin and left the alt field empty.
But the ImageItem field plugin already has a default value for alt which is an empty string.
see: https://git.drupalcode.org/issue/drupal-3319601/-/blob/3319601-media-ima...

For remote video media entities based on the oembed source plugin this issue doesn't come up because the default value is set on the updateThumbnail method in the Media entity.
see: https://git.drupalcode.org/issue/drupal-3319601/-/blob/3319601-media-ima...

So the issue is "only" related to media entities which are based on media source plugins that have $thumbnail_alt_metadata_attribute defined.

🇩🇪Germany sunlix Wesel

Hey,

we have the same behavior in our installations. New installation with drush site:install --existing-config are not successfull if we have a content type configured to a content_moderation workflow.
Beside this patch we have to apply the path from 🐛 BaseFieldOverride fails to take into account ContentEntityInterface::bundleFieldDefinitions() when invoking onFieldDefinitionUpdate() Needs work to be able to successfull install from existing config.
Maybe that could related to each other. The error message differs but I think it is some kind of the same source issue.

🇩🇪Germany sunlix Wesel

I can confirm this.
I had multiple phone calls from customer on this topic today. But unfortunatelly did not found a reason yet.

🇩🇪Germany sunlix Wesel

Hey,

I have made a first shot so solve this from here.
We could change to module weight to be after the metatag_extended_perms_field_widget_single_element_form_alter and change back the #access
I think we can't proactive disable the elements display?

🇩🇪Germany sunlix Wesel

Hey, I also can confirm that the new sticky actions implementation solved this behavior described here. :-)
Thank you for your work! :) I like the new sticky actions.

🇩🇪Germany sunlix Wesel

Yes, it breakes other implementations, too, like eTracker as mentioned here: https://www.drupal.org/project/cookies/issues/3422880#comment-15477412 Convert CookiesKnockOutService in service Needs review

🇩🇪Germany sunlix Wesel

Hey Julian,
Hey Joshua,

this change is a breaking change cause of the deletion of CookiesKnockOutService::getInstance()
With the latest release of COOKiES you broke all remote implementations like eTracker's COOKiES support.

This was the default call to knock-out the scripts by server response.
CookiesKnockOutService::getInstance()->doKnockOut()

see: https://git.drupalcode.org/project/etracker/-/blob/8.x-3.x/modules/cooki...

Do you see any chance to get back the deleted method? Otherwise all remote implementations stay broke.

🇩🇪Germany sunlix Wesel

The error occurs only with Drupal 10.2.x.
Its not media library related only. Its happen also with entity_browser.
I think its more a modal/ajax related bug.

🇩🇪Germany sunlix Wesel

Yes we have the same problem. I think maybe there is a think in Drupal 10.2.2 but cant verify it at the moment.

Before you are open any media library modal Drupal.FieldGroup and Drupal.behaviors.fieldGroup are defined.
After executing any filter oder pagination the JavaScript error occurs in the browser console and the Drupal-Objects are cleared and set undefined.

🇩🇪Germany sunlix Wesel

Hey Julian,

I am sorry wanted to have a look today, but a full bloat meeting schedule didn’t made any „work“ done.
Added it tomorrow, hit me on Slack before 10h so I think we can spin off beta5

🇩🇪Germany sunlix Wesel

@acbramley

I am aiming for the 📌 Rework config keys Active to make the config more speakable and meaningful against Readspeakers product.
If that is done and we have an upgrade path I will spin-off a release.
Unfortunally my daily job has currently no spare time for this until end of march I guess.

🇩🇪Germany sunlix Wesel

Hey,

I close this issue, because it is already merged by Sascha. :-) Thanks for that!

🇩🇪Germany sunlix Wesel

Yeah you are right. I did not know the function before I saw it here in Twig Tweak.
I think my expectation was mislead by the documentation, where only {{ drupal_view_result('who_s_new', 'block_1') }} was written down without any further clarification.
So sorry for that. Now it is more clear to me.

🇩🇪Germany sunlix Wesel

I have the same question. :)
The Twig-Function use the Drupal callable views_get_view_result()
I have attached a Screenshot with the Details from xdebug.

It cant render correctly by Twig. Do you have any advice?

The error is for "0" and "1" the same (In my case there a two view rows)

User error: "0" is an invalid render array key in Drupal\Core\Render\Element::children() (line 98 of core\lib\Drupal\Core\Render\Element.php)

The Result of Element::children(views_get_view_result('view_name', 'view_display')) is empty.

🇩🇪Germany sunlix Wesel

I found the issue in our distribution. :-)
We have a library which depends on the gin/edit_form library.
Due to that, we load the js.

But I think the question remains. Should the gin libraries declare its dependencies (correctly/explicity)?
The edit_form.js depends on the core/once library but do not declare it as a dependency explicity.

🇩🇪Germany sunlix Wesel

Hey sure,

I dived a little bit deeper into it.
I cant allocate the source of the dependency tree. By any reason edit_form.js is in my depedency tree.
But I am logged out on my default frontpage. The edit_form.js should not have in.

🇩🇪Germany sunlix Wesel

Uff I am so blind. Mea culpa :D

Can you tell me how I can add the dev release to the project page?
It is only possible with a release tag?

Production build 0.71.5 2024