- 🇺🇸United States dianacastillo Miami
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.... → "
- 🇺🇸United States neclimdul Houston, TX
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.
- 🇫🇷France prudloff Lille
I moved the note to the beginning of the form and added a test.
- @prudloff opened merge request.
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.
- 🇫🇷France mably
@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.
- 🇫🇷France vanessa.fayard
Hi,
The patch works for me (Drupal 11 - Gin 4.0), after clearing the cache.
Thanks for the issue and the patch!
- 🇺🇸United States phenaproxima Massachusetts
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 toFilterBase::__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.
- 🇺🇸United States chrisfromredfin Portland, Maine
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)
-
mably →
committed 6a987795 on 3.0.x authored by
james.williams →
Issue #3200253 by sokru, john franklin, herved, arturs.v, cgknutt,...
-
mably →
committed 6a987795 on 3.0.x authored by
james.williams →
- 🇬🇧United Kingdom james.williams
OK, all done, and the checks are now fully passing :)
- 🇫🇷France mably
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?
- 🇬🇧United Kingdom james.williams
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 :)
- @jameswilliams-0 opened merge request.
- 🇬🇧United Kingdom james.williams
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.
- 🇫🇷France mably
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.
- 🇮🇪Ireland lostcarpark
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.
- 🇺🇸United States chrisfromredfin Portland, Maine
confirmed fixed with manual testing.
-
chrisfromredfin →
committed 967fba8e on 2.0.x authored by
utkarsh_33 →
Issue #3498570 by utkarsh_33, simobm, chrisfromredfin, sandip poddar,...
-
chrisfromredfin →
committed 967fba8e on 2.0.x authored by
utkarsh_33 →
- First commit to issue fork.
- 🇺🇸United States phenaproxima Massachusetts
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.
- 🇺🇸United States DamienMcKenna NH, USA
Fixed some copy pasta on CheckboxWidget.php.
- 🇺🇸United States smustgrave
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
- 🇺🇸United States kentr Durango, CO
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.
- 🇩🇪Germany rkoller Nürnberg, Germany
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.
- 🇺🇸United States DamienMcKenna NH, USA
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.