#Accessibility

for version 3.6 this is the correct patch : " https://www.drupal.org/files/issues/2024-08-07/field-group-tabs-8.x-3.x.... → "

Sorry for the late response. Not sure how related this is to the parent unless the scope of that issue is different then the title.

Technically we're already using the … entity so which is probably better then the UTF8 character. This is issue adds information to the pager to explain what an ellipsis means in the context of the pager list. Specifically explaining what the missing information is.

I moved the note to the beginning of the form and added a test.

I was there too, thanks.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

@james.williams what is probably missing is a little update to the README file.

Users will appreciate a bit of documentation about that important new a11y feature.

Reviewed & tested.

Hi,

The patch works for me (Drupal 11 - Gin 4.0), after clearing the cache.

Thanks for the issue and the patch!

As Adam says, we need to basically allow a "description" more generically. That's the first piece. FilterBase should support a description field (which accepts markup?).

Yup, let's add a new string|\Stringable|null $description = NULL parameter to FilterBase::__construct(). There's no reason it couldn't take markup.

This would be an API addition, not an API break, and therefore this issue is not a beta blocker.

I SWORE I already provided feedback on this, but it's not here - might have been one of those ones that I started in an open tab, then closed the tab.

There's a few more major things here, and it might be that this branches out into an issue that needs to be worked on before this one:

1. As Adam says, we need to basically allow a "description" more generically. That's the first piece. FilterBase should support a description field (which accepts markup?).

2. If a description is set, then we should set a ? icon, I think, next to the label (not next to the select). I would use core's questionmark-disc icon (although I'm not sure if we can always count on that being there, but might make sense to try anyway - even if it's not guaranteed).

3. As we're doing now, I think it's OK to then open that description markup in a popup/modal.

4. I'm not sure where we're actually getting the names/descriptions from, but I think we'd want to actually have this queried from Drupal.org; not sure if that means it needs its own endpoint? I'm happy to do that part later, and rely on something like the test fixture (for now), but I don't love that as a long-term solution. (Also, happy to cache that for like an hour or even longer if needed, to help reduce requests to d.o)

Merged. Thanks @james.williams!

@james.williams looks like a rebase is needed.

Thanks!

Let's merge it and see if things break 😉 🤞

OK, all done, and the checks are now fully passing :)

Thanks for your great merge-request @james.williams!

Looks like some phpcs and phpstan warnings are still remaining.

And to avoid any BC breaking changes, could we try to make all the new method parameters optional if possible?

Ah I see some of the Vimeo videos used in tests were updated, so I've had to update the expected values too. And it appears that Vimeo only provides limited functionality for channels in the EU now (see https://www.reddit.com/r/vimeo/comments/1gga3gf/comment/luqd62h/), so I've removed the test for that as it fails in the GitLab pipeline.

MR !67 is now ready for 3.0x, with tests passing :)

Option 3 looks like a good start for me.

Thanks! But I see from #3483205-5: Drupal 11 compatibility fixes for Video Embed Field → that the video_embed_wysiwyg module has gone from the 3.0.x branch. Which direction would you like to head in?

1. Merge this MR for 8.x-2.x, with a view to merging 8.x-2.x into 3.0.x later (alongside other improvements currently in 8.x-2.x but not 3.0.x)
2. Postpone further work on this issue until video_embed_wysiwyg for 3.0.x is ready (😢 my least favourite option, personally!)
3. Make a version of the current MR from this issue which doesn't include the work for video_embed_wysiwyg, for merging into 3.0.x (and perhaps split out a follow-up for this issue's work for a later 3.0.x version of video_embed_wysiwyg)

I guess it depends on where plans for video_embed_wysiwyg, and your support for 8.x-2.x, are at. If video_embed_wysiwyg isn't going to make a return, we may as well just do option 3. Though you still have plenty of us using 8.x-2.x who might like this functionality in that branch too please!

Automatically closed - issue fixed for 2 weeks with no activity.

Could we have an MR against the 3.0.x-dev branch?

As it is still in alpha version, we could merge it without taking too many risks.

Marking needs review for @chrisfromredfin. I know there's still more work needed, but I would like some feedback on what's been done.

Currently the help button is added to all multiselect drop-downs. Would be nice to make it only shown if the filter has help text. And maybe make it more general, so any filter can offer help text.

Also needs some tidying up to make the tests work again, but would be helpful to get some feedback on whether this takes it in the right direction.

confirmed fixed with manual testing.

Gave this a quick manual test and confirmed that the selector looks awful in 2.0.x with Gin's dark mode enabled, and much nicer with this MR applied!

The CSS doesn't look problematic to me either. As far as I can tell, we're good to go here.

I was there. It was fun :)

Fixed some copy pasta on CheckboxWidget.php.

I attended, but had to leave early :(

Believe feedback has been addressed

Treating #50 as sign off for accessibility

Seems to have gone through several rounds of review and looking at the MR I see the threads have been resolved and I'm not noticing anything off.

Verified by going to a simple view, example /admin/structure/views/view/content and just opening dialog there.
Can confirm the styling appears unchanged also.

LGTM

I'm going to try to move the MR forward a little.

Adding Needs issue summary update because the IS mentions IE11 and AFAIK Drupal dropped support for IE11.

thank you! i've manually tested on macOS Sequoia 15.3.1 with Safari 18.3, latest Firefox, and latest Edge - the voiceover output is the same in all three browsers. it looks like the prefixed abbreviations got already removed for default menu items with icons without the MR applied, but menu items without an icon still show the abbreviation:

With the MR applied those abbreviations get also hidden/removed for menu items that do not have an icon:

so overall that looks good to go. since the MR is following the approach suggested by @finnsky in #5 🐛 Remove prefixed abbreviations from top-level menu items Active i think it would be ok to set the issue to RTBC.

A patch for v3.0.x - I tried to create a merge request for it but gitlab wouldn't let me pick 3.0.x as a destination branch, even after I updated the issue fork from the main repo.

Am working on porting it to v3.

This needs to be reworked for v3.