Berlin
Account created on 13 February 2013, over 12 years ago
  • Senior Full-Stack Developer at 1xINTERNETΒ  …
#

Merge Requests

More

Recent comments

leymannx Berlin

It's not been removed, just moved. The title of the issue and commit message is misleading.

leymannx Berlin

Instead of adding Drush to core we would built another Drush to add it to core. This is not DRY.

Eventually people then pick this approach up in their contrib modules and now have to provide 2 commands for the same thing, one for Drush, one for NotDRY.

Looking the other way instead of getting Drush closer to core, also doesn't sound fair to all the people who develop and maintain Drush. They do this for Drupal.

leymannx Berlin

I agree and would also vote for bug, given that there's even a place in the condition plugin's code where you provide a dynamic summary to be picked up in this exact vertical tabs place.

leymannx Berlin

If we make the title into a fixable task e.g. "Improve "The value you selected is not a valid choice" error message", then it's nothing about user module and instead becomes a feature request in the first place to give people a meaningful error message from core's AllowedValuesConstraintValidator.

Like:

  • What is a valid choice?
  • What is the current choice?
  • Which field/element triggered this error?
leymannx Berlin

Thank you @aortega1441 this looks very promising 🀩

We just should check again on the coding standards in that JS file, the comments in particular, please see https://www.drupal.org/docs/develop/standards/javascript-coding-standard... β†’ .

Non-JSDoc comments SHOULD use capitalized sentences with punctuation. Comments SHOULD be on a separate line, immediately before the code line or block they reference.

leymannx Berlin

The error is NOT coming directly from Drupal but from a validator that is extending Symfony's ChoiceValidator, the AllowedValuesConstraintValidator. You need to look for this string The value you selected is not a valid choice inside the project/vendor/ directory and not inside project/web/.

In the end after setting a few Xdebug breakpoint in the AllowedValuesConstraintValidator to understand what's passed around here, I had to set a breakpoint inside Symfonfy's ChoiceValidator.php in the last elseif to find out that the problem was an invalid timezone value for the user, I could then clean-up by exposing the timezone field via /admin/config/regional/settings to let users chose their timezone and also have the problematic value editable.

Took me 4h. Cheers 🍻

leymannx Berlin

Ah, you can simply pass context.location.origin to the URL constructor, to make it also work for relative or even protocol-relative URLs like //example.com/foobar, see https://developer.mozilla.org/en-US/docs/Web/API/URL/URL.

But there's another problem: data-href="javascript:alert()" is recognised as a valid URL. And this is exactly what we want to avoid.

leymannx Berlin

Maybe we could check first, if the string starts with a / character and if yes, temporarily prefix it with https://www.example.com to still be able to use this nice and clean URL method of the current MR. πŸ€”

leymannx Berlin

Thank you for your contribution @chhavi πŸ€— but @jan is right: We need to take both absolute and relative URLs into account.

leymannx Berlin

Provided new MR with info.yml changes done as well. Also removed composer.json which had differing version constraints. composer.json file will get created automatically from drupal.org infra pipelines. You only need to have it, if you have additional PHP dependencies.

Let's get this merged and released. πŸ™πŸ»

leymannx Berlin

True, now the hook is in src/Hook/ContentTranslationHooks.php?ref_type=tags#L382-415 and the 11.x MR still has it, while the 10.x patch has the hook removed.

But all tests are green πŸ€”

Are the tests wrong? Or could the hook still stay?

leymannx Berlin

The patch is completely unrelated to this issue.

leymannx Berlin

@tim - Please help us, checking out the current MR, reviewing if it is working for you, reviewing if it fixes any accessbility issues for you.

Maybe you can even help us solving the problem of the initial focus.

leymannx Berlin

Not sure how mirroring a third party JS library on drupal.org should be any good.

leymannx Berlin

Running composer require drupal/better_exposed_filters and pushing the updated composer.json and .lock file, my colleagues pull and upon running composer install the get an error:

- Required package "leongersen/nouislider" is not present in the lock file.
This usually happens when composer files are incorrectly merged or the composer.json file is manually edited.
Read more about correctly resolving merge conflicts https://getcomposer.org/doc/articles/resolving-merge-conflicts.md
and prefer using the "require" command over editing the composer.json file directly https://getcomposer.org/doc/03-cli.md#require-r
Composer [install -n] failed, composer command failed: exit status 4. stderr=

Re-running composer require drupal/better_exposed_filters then updates the .lock file again with leongersen/nouislider in it.

So we should either inform people about needing to run composer require drupal/better_exposed_filters twice or we follow the approach of either providing a libraries.composer.json or add the library into better_exposed_filters' composer.json like for example the Photoswipe contrib module does it: https://git.drupalcode.org/project/photoswipe/-/blob/5.x/composer.json?r...

I honestly don't understand why we would create a mirror of a third party library on drupal.org and try to pull it in like that.

leymannx Berlin

This is working really well so far. πŸ‘πŸ»

The only thing we would like to change is that we would like to prevent that there's a default focus upon opening the page.

Only after the first time pressing tab we would like the first element to be focused.

leymannx Berlin

The current patch did not account for manually entered URLs in config.

We must cover both absolute library URLs and relative URLs with base path prefixed manually already.

Applied same changes as in πŸ› Library path wrong for subsites (4.x) Active .

leymannx Berlin

The failing pipeline is unrelated to this issue and has already been addressed in πŸ› LazyForm too few arguments Needs review .

leymannx Berlin

There's a config in the module to override this path already. The current patch unfortunately doesn't work if an absolute URL has been entered. And it also won't work if the base path has been manually prefixed already.

I think we need to additionally check two things first:

  1. 1. Is the URL relative and has the base path already been entered
  2. 2. Is it an external URL

In both cases the base path should not be added.

leymannx Berlin

I guess that's why I got no credit now. Although I spent time reviewing and testing it. 😿

leymannx Berlin

Needs review and needs follow-up issue to place the currently omitted [OMITTED] Mailchimp keys.

leymannx Berlin

Both types of hero - frontpage and detailpage - are too different. And the frontpage is much different from the blog template as well. So I've put the frontpage hero with just raw markup.

We then only need the hero component for the detailpages.

leymannx Berlin

Today @klausi and me met in Vienna to get this issue resolved ultimately.

leymannx Berlin

Friends, please: There's just one ContributionWeekend tag. We want to count all issues afterwards easily and have them all show up on one page easily.

Thank you! Happy contribution weekend everybody πŸ€—

leymannx Berlin

Only into the suggest section I'd say. Same as the others

leymannx Berlin

Is this still an ongoing issue? What's the idea on how to proceed here?

Production build 0.71.5 2024