@leymannx

Would have needed a rebase first.

MRs is nice yeah, got created already. But as long as GitLab doesn't offer a way to get static, non-changing patches, we still need that patch here.

Came here very later after asking myself why the clientside_validation module, that already has that logic to add a role="alert" attribute built-in (if the inline_form_errors module is enabled) is behaving differently on Webforms.

Clientside Validation module:

• Here the library gets added dynamically: https://git.drupalcode.org/project/clientside_validation/-/blob/4.1.2/cl...
• And here a wrapper with the role="alert" attribute gets added: https://git.drupalcode.org/project/clientside_validation/-/blob/4.1.2/cl...

But then Webform module takes over:

• Library gets added dynamically: https://git.drupalcode.org/project/webform/-/blob/6.2.9/modules/webform_...
• With its own logic and no wrapper and no role="alert" attribute: https://git.drupalcode.org/project/webform/-/blob/6.2.9/modules/webform_...

And because of there there's now two different logics:

• Webform have no wrapper and no role attribute
• All other forms have the errors within another wrapper which has the proper role attribute

Two thoughts on this:

1. I think this should actually be fixed upstream: https://github.com/jquery-validation/jquery-validation/issues/2519
2. And up until then it would be nice to at least have kinda feature parity for webform_clientside_validation and clientside_validation_jquery (which even is a dependency of the webform_clientside_validation module), instead of overriding it. Means: webform_clientside_validation should also add that wrapper with that role attribute.

Follow-up: 🐛 Leave preview points to undefined instead of node's edit page Needs review

Hmmm, okay, just noticed now that #10 is just reverting what was done in 💬 The link on the Image tag is redirecting to an undefined page from the node preview screen. Active , so I guess this can't be the fix. Looks like we have to check both. Like this maybe:

let href = event.currentTarget.href;
if (href === undefined) {
  href = event.target.href;
}
window.top.location.href = href;

Re-uploading the patch from #3308432-42: The link on the Image tag is redirecting to an undefined page from the node preview screen → that fixes this issue. Credit needs to be given to @keshavv.

This here is a follow up of that other issue.

Added the functionality to have it configurable and off by default. Let's get this tested in the wild.

Looks like they switched to using the no translation no CSS lib by default. They provide their own CSS and have Drupal take care of translations. Since our Simple Klaro module here allows you to switch the library yourself, anybody could take care of it themselves, I guess.

But let's maybe retitle this issue to a solvable task: Observe how making no translations no css library the default impacts performance and find a way to ship default CSS and translations.

Otherwise we need proof at least minimal for this statement: Klaro library is way too large. What does that mean? What is large? What is small? What is acceptable?

Uploading your current work as static patch, as this already does the job perfectly, just the new option is missing to have it disabled by default for people who don't want the heading tags changed.

Thank you! This patch is working already really, really nice as is. Very clean solution.

Now, what's left is to provide options in the Simple Klaro settings to enable this behavior or not. By default it must be switched off.

1. Update Simple Klaro settings form to provide a checkbox "Fix heading structure" with the description: "By default Klaro has the modal title as <h2> and the settings form title as <h1>. This is bad for accessibility (non-sequential heading structure) and for SEO (<h2>without parent; multiple <h1>). With this option enabled both headings are replaced with a <div> and its heading level as CSS class added (<div class="h2"> and <div class="h1">). Remember you now need to take care yourself of styling it accordingly.
2. Get this checkbox value passed from PHP to JS drupalSettings and adjust the heading JS to react only when this checkbox is enabled
3. Provide an update hook that sets these values to FALSE initially.

Let's make this a feature request.

Would probably be good to review the description again after 5 and a half years.

Tasks:

• Use aria-live="assertive" or role="alert" on error message containers. This ensures screen readers announce new errors as soon as they appear.
• Link error messages to fields using aria-describedby.

How you add that policy header? Contrib module?

Good idea!

Any chance you could give this a try for an MR?

norman.lol → created an issue.

RTBC++ 🥳

(Friends, please pay attention on the patch file naming. 🙏🏻 Following minimal convention of ISSUEID-COMMENTID.patch makes navigating drupal.org issues and identifying the connected comment much easier until everything is hopefully moved to GitLab soon. 🤞🏻)

This is a very good idea. And there's already similar issues filed to the official GitHub repo:

[Provide configurable consentModal.titleTag and consentNotice.titleTag #425]

But I guess, until this gets fixed upstream, we need to do it ourselves.

To maintain backwards compatibility we ideally also offer a new option in the simple_klaro.settings to configure the consentModal.titleTag and the consentNotice.titleTag.

But first of all we need to find a way to make it work, to get the heading markup-changing JavaScript attached at the right moment and the markup actually changed.

Later we need to get that option in.

Is MR44 another approach to solve the same problem?

With MR34 and running drush generate phpstorm-meta to get a nice mapping file for PhpStorm autocompletion, I get the following error, as these plugins are still referenced in code:

Welcome to phpstorm-meta generator!
–––––––––––––––––––––––––––––––––––––

In ExtensionManager.php line 78:

Plugin ID 'commonmark-heading-permalink' was not found.

phpstorm-meta [-d|--working-dir [WORKING-DIR]] [-a|--answer [ANSWER]] [--dry-run] [--full-path] [--destination [DESTINATION]] [--replace]

Failed to run drush generate phpstorm-meta: exit status 1

I think we should definitely go for a jigsaw puzzle-shaped shaped logo. It is instantly unique and instantly memorable—no one forgets it.

And most important: It opens the door to endlessly creative marketing.

Think puzzle piece stickers, collectible swag, maybe even a puzzle plush toy and real mini puzzles as giveaways. Each piece can represent a product, value, or community member—versatile and symbolic. It’s playful, interactive, and inherently shareable.

This logo invites engagement, not just recognition. So if we want Experience Builder to hit big, we should go for puzzle shape.

Nice! Yes, this looks cleaner.

I can confirm, that the lastest commit, tested manually, is working perfectly.

Ah you can just add blocks into the new navbar, okay.

Yeah, we would need to find a ways this gets placed automatically.

@scott – What do you mean with this:

Actually could just leverage the Navigation module

Nice, this is a very good idea. Gonna test it now

norman.lol → created an issue.

First patch contains a fixed applied too broadly, on all entity edit links. Instead, let's get this parent view mode fix only for paragraphs.

norman.lol → created an issue.

Hm, maybe yeah. On the other hand: Shouldn't a rating element provided by Webform core be accessible OOTB?

norman.lol → created an issue.

norman.lol → created an issue.

norman.lol → changed the visibility of the branch aortega1441-3452243-accessible-klaro-patch-52096 to hidden.

norman.lol → changed the visibility of the branch 3452243-a11y to hidden.

Turned out that the only thing that was needed for the initial focus to be removed was to remove the tabindex="0" from <div id="klaro-cookie-notice" tabindex="0"> added by the klaro lib. Now I could remove firstFormFocusableElement.focus();, and even without it the first tab press immediately focused on the first focusable element and stayed fully loop-able after that.

But still took me almost a day to get there. 😅

On the way I refactored the code quite a bit:

• added once
• replaced document with context where possible
• refactored the actual trap functionality into a separate function trapFocus with an option to pass different key press logic to it (depending if used on the main modal or the preference form)

norman.lol → created an issue.

norman.lol → changed the visibility of the branch 1.x to hidden.

norman.lol → changed the visibility of the branch 3452243-accessible-klaro to hidden.

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

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.

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.

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?

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.

yesct → credited norman.lol → .

yesct → credited norman.lol → .

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 🍻

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.

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

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

norman.lol → created an issue.

norman.lol → created an issue.

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. 🙏🏻

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

Let's get this merged and released 🙏🏻

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

Bump 🙂

Saw the project listed in https://drupal.stackexchange.com/q/321793/15055

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?

The patch is completely unrelated to this issue.

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

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

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.

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.

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 .

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

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.

norman.lol → created an issue.

💙

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

norman.lol → created an issue.