🇩🇪Germany @rkoller

re #13: thanks, i've tried to follow your suggestion, not sure if the position i've placed both flags in was the correct one (i am not a developer), but it looks like your suspicion might be right, at least in regard of 📌 Update to jQuery 1.14.1 and use the newly added option for dialog modal headings Active , the test output seems to complain about the inability to connect to the webdriver instance and that the selenium host could not be resolved?

The test wasn't able to connect to your webdriver instance. For more information read core/tests/README.md.
The original message while starting Mink: Could not open connection: Could not resolve host: selenium

https://git.drupalcode.org/issue/drupal-3485202/-/jobs/4069988

About the previous discussion, @lauriii summarized it in #3370326-4: Refine field descriptions → :

The file upload group was created based on a card sort exercise we ran earlier this year. The results we got from there were pretty clear that people tend to map both image uploads and file uploads together. That said, Media was a challenging one; some people listed it under reference and some people listed it under file upload. That's why we decided to move it to the main level to try to address this challenge.

One of the use cases we tested as part of the usability testing was image and media fields because we had similar concerns that it might not be intuitive. However, both existing and new users were able to find the fields and we didn't find any challenges with the current sorting so we decided to proceed with that.

and about the point people were able to find fields... when i first tested the new add field functionality i had a real hard time finding the image field type, i havent expected or even considered to look under the file upload group for the image field type.

personally i am a heavy +1 on moving media, file, and image field types into a group together.

In regard of the micro copy for the group two suggestions:

group label: Media assets (or just Assetsto keep things even more generic)
group description: Create media type with the ability to upload or link assets

suggestions were taken from https://docs.google.com/spreadsheets/d/1Lx7L40eRHotr5KQGn6au5WxQO3lFv19a...

benjifisher → credited rkoller → .

unassigned @stasadev since the issue is ready for review (but asked him and made sure he isnt still working on anything)

thank you! looks good to me. tested another round with the latest changes. created three instances in three different directories which results in drupal-cms, drupal-cms-1, and drupal-cms-2 projects. and when i copied another install inside a subdirectory of one of those three instances i get:

[rkoller@mycomputer][~/zeug/drupal-cms/recipes/drupal-cms]
$> ./launch-drupal-cms.sh 
You're trying to set up a Drupal CMS inside an existing project in /Users/rkoller/zeug/drupal-cms, refusing to do it.

i leave the issue at needs reviews. but from a manual testing perspective this looks good to go and is an improvement.

I've discussed the topic with @stasadev via Discord and tested the proposed resolution. with previous proposed resolutions i ran into issues. the initial proposal i sent over had the problem with three folders of drupal-cms the first launched launcher gets drupal-cms as the project name , the second drupal-cms-1 and for the third it refused to set up a project. and the problem with trying to setup a new instances of drupal cms inside the subfolder of an already installed instance of drupal cms ran on DDEV HEAD wasit behaved correctly, by running ddev config (updating the config the project in the parent folder) and then run ddev start (ddev restart in this case strictly speaking), but as i said not apparent to the user.

with the proposed resolution @stasadev posted i was able to create several instances of drupal-cms across my file system, and it properly counts up, i'Ve stopped after drupal-cms-4... the first project was drupal-cms, the second drupal-cms-1.. and when you try to execute the launcher in a directory where its folder is a subfolder of another already registered drupal project the script now just exits and we agreed on providing some context with You're trying to set up a Drupal CMS inside an existing project in $(pwd), refusing to do it. ... that way the user knows what goes wrong and where to look for the parent project by the path that was provided.

rkoller → created an issue.

rkoller → created an issue.

it looks like the crash got fixed when the workspaces navigation integration got in ✨ Integrate with Workspaces Active . unable to reproduce the crash now, closing.

the commit you've referenced is about something else, probably the commit was fixing #3458844-33: Improve keyboard navigation/general a11y for categories dropdown → , but the problem this issue is about is still relevant, otherwise i wouldnt have opened the followup. the issue got raised in #3458844-36: Improve keyboard navigation/general a11y for categories dropdown → . i've rested, and the problem persists with voiceover active in safari, edge and firefox on macos. as outlined in the issue summary and the acompanying video. simply activate voiceover, expand the multiselect. first navigate by the arrow down key one or two entries down then use VO-arrow left (VO = ctrl and option) and step back to the beginning of the multiselect dialog. and as you can see in the video the voiceover cursor steps outside the dialog and focuses elements outside the dialog that are located before the dilalog in the DOM. if you then press arrow down the voiceovercursor literally jumps back inside the dialog. the dialog remains open all the time. if you now scroll further down the dialog with the arrow down key and one or two entries before the end use the VO-arrow right button, now. the VO cursor is also reaching outside the dialog, but here the dialog immediatly collapses.

I've tested the MR tonight. With the feature branch checked out the autosave button seems to be hidden and is not visible to sighted users nor screenreader users, but if i search for the class autosave-form-save in the web inspector i have no matches at all on the entire node edit form. so it seems like the autosave button isn't available at the moment? cuz there is also a deprecation error shown:

Deprecated function: Creation of dynamic property Drupal\autosave_form\Form\AutosaveFormBuilder::$_serviceId is deprecated in Drupal\Component\DependencyInjection\Container->createService() (line 285 of /var/www/html/repos/drupal/core/lib/Drupal/Component/DependencyInjection/Container.php).
Drupal\Component\DependencyInjection\Container->createService() (Line: 177)
Drupal\Component\DependencyInjection\Container->get() (Line: 430)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters() (Line: 237)
Drupal\Component\DependencyInjection\Container->createService() (Line: 445)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters() (Line: 237)
Drupal\Component\DependencyInjection\Container->createService() (Line: 177)
Drupal\Component\DependencyInjection\Container->get() (Line: 28)
Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition() (Line: 100)
Drupal\Core\Utility\CallableResolver->getCallableFromDefinition() (Line: 34)
Drupal\Core\Controller\ControllerResolver->getControllerFromDefinition() (Line: 49)
Drupal\Core\Controller\ControllerResolver->getController() (Line: 166)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 709)
Drupal\Core\DrupalKernel->handle() (Line: 19)

tested on ddev on drupal 11.x and php 8.3. I'll set the issue back to needs work

rkoller → created an issue.

Forgot to add direct links to the current drupal terminology sheet, added now.

Ok I think I've cleaned everything up and brought the outline back in that got lost with the copy and paste from the draft in Google docs. Wrapped some names in code elements to increase the highlighting and removed the wrapping double quotes. In regard of the outline i went with an ordered list for the main point, if a main point only had one level points i went with an unordered list for those (for example 1 and 3), while for point 2 i've used also an ordered list for the first level and an unordered list for the second level. i'll unassign myself again.

hm i've rechecked my example after the comment in #10, and the testonly run is still passing there :/ https://git.drupalcode.org/issue/drupal-3485202/-/jobs/3973629 (triggered a new job)

benjifisher → credited rkoller → .

direct links to the follow ups are:

🐛 [PP-1] Mimic the aural interface of the multi select div to the select element Active
🐛 voiceover cursor moves out of the multiselect Active
📌 minor regression between navigation a multiselect and a select element Active

rkoller → created an issue.

rkoller → created an issue.

rkoller → created an issue.

since 📌 Improve keyboard navigation for categories dropdown Active is in this is active now (ironically i forgot to postpone it when i've created it)

rkoller → created an issue.

i'll add two images to illustrate the points @phenaproxima summarized:

and the select and deselect buttons also have to follow the accent colors.

joachim namyslo → credited rkoller → .

i'Ve already opened a follow up for the mimic the select element issue. the small regression would also be fine with me to move it into a follow up . the behavior with VO-arrow left and right i dont know on the other hand. it "might" be ok in a follow up as well. not sure how many screenreader users are using project browser at this point. i just have a bad feeling to commit with that kind of known problems. but on the other hand the MR improves the keyboard experience significantly. i dont know, maybe that is why i dislike deadlines ;) let chris decide.

hm i'Ve just checked admin/content in gin light and dark mode and learned that the bulk action bar is styled differently there. not as a dark bar but in light mode in as a light bar (which isnt meeting SC1.4.11 probably) and dark mode as a dark bar with a light border.

so the styling of the bulk action bar would need to follow the styling on admin/content for light and dark mode. :/ and it has also be noted that the primary button color needs to adjust to the set accent color for gin. at the moment the color remains always the same. checking against two themes every time becomes a rabbit hole :(

joachim namyslo → credited rkoller → .

i agree the status changes about how many modules are selected can be moved to a follow up. neither drupal cms nor core using project browser would be directly affected, drupal cms isnt showing the bulk action anyway while for core there is selection limit per default , so most of the people wont run into that problem therefore a good call to move it into a follow up.

in regards of the changes... for claro:

the button label on the regular button has a larger font size than the primary button? and both buttons sort of have a smaller padding on the left and right if you compare it to the primary button on admin/content

for gin:

the regular button is missing a background color and the button border same as the button label have with that dark blue against the black background a way too low contrast.

joachim namyslo → credited rkoller → .

awesome thanks! i can confirm with voiceover active and if you are pressing the arrow left or right repeatedly the dialog isnt collapsing anymore.

but while testing i found another small detail :( in the video after the dialog is open i navigated with arrow down first. then with VO-arrow up, that way i was able to get out of the dialog.. but after pressing arrow down again the cursor was again inside the dialog. with VO arrow right at the end of the dialog you get outside dialog but in this case the dialog collapses.

the other small nitpick, not sure if it is a regression. on the select element with voiceover deactivated and you navigate with arrow up and down if you reach the end of the option list you dont jump to the start and same the other way around. that circling that you jump to the start from the bottom and to the bottom from the start is only with voiceover active. at the moment with the MR that looping is with voiceover activated and deactivated ( if that is tricky that might be also moved to a follow up)

joachim namyslo → credited rkoller → .

added 🐛 The focus outline is using a different shade of green than the rest of Claro Active

rkoller → created an issue.

created a follow up about the adjustment of the multi select to the aural interface of the select element 🐛 [PP-1] Mimic the aural interface of the multi select div to the select element Active

and there is another problem. i've set the max selection to 5 with ddev drush config:set project_browser.admin_settings max_selections 5. there is no feedback that informs the user about the given limit. without the MR applied i get at least the disabled button mouse cursor that signifies the select button is unavailable, but with the MR applied the mouse cursor is still shown as a pointer and i am able to click the select button as many times as i want but nothing happens. i think two things have to happen:

• update the string from "x projects selected" to something like "3 out of 5 projects selected" and "Maximum number of projects selected" (on a side note, projects is an unknown concept in the context of the durpal admin ui, it is a concept known from drupal.org, plus the path on the browse page says modules and not projects - i would highly recommend to call things what they are and make concepts distinguish- and recognizable)
• when the maximum number of selected modules is reached it might be a good idea to use the disabled mouse cursor for any of the available select buttons on the page. so it is illustrated that is is not possible to add any more modules - like without the MR applied

rkoller → created an issue. See original summary → .

changes look good to me, the two remaining issues from #47 were adressed imho. thank you!

Throwing everything into a single form raises again the problem outlined in #9 ✨ Allow clearing of cached data from Project Browser sources Active :/

haven't really taken a closer look at project browser the last two or three weeks, was busy and still am with reevaluating an audit for gin, so i havent really noticed that the secondary tabs were gone. i've only taken a brief look at the linked issues now. i would agree this issue would be obsolete with the changes from linked issue. but my first impression i would consider those change highly confusing and problematic. they are mixing different concepts and blur the lines further. plus on a side note even though i have the core tab enabled for project browser i dont see any local item. i have to study those linked issues and read up on them and do some more research later today. but have to jump in an a11y call starting now. ufff.

i would orient towards the top tasks exercise we did a few months back. that illustrated which details were important to people assessing a module on a details page. but looks like the sheet got removed? https://docs.google.com/spreadsheets/d/17JiSqr5VHOfupdGLo6pLatM2VfxOy3vw... #gid=127356256 at least the link i've provided on the invision board is not valid anymore? and i'Ve attached the screenshot from the invision board from the discussions that spun out after the exercise was done.

as mentioned on slack i gonna discuss a few aspects of the MR in today a11y call, but wanted to provide two videos about another detail i've noticed testing the MR today. if you have voiceover active and the multi select expanded in case you are navigating the voiceover cursor by the left and right , if you reach the end of an option label the multi select collapses silently (see the two videos).... if you are using VO arrow left or right the voice over cursor steps through the the checkboxes and the checkbox labels.

testing the latest changes on the MR... I completely agree about showing the reset button only if anything is selected, but should the reset be also a primary button? that way you have two primary buttons next to each other? and it looks like the bulk action bar isn't sticky anymore at the bottom of the visible viewport, instead you have to scroll to the bottom of the page? that looks like a regression or is that an intended step (at least con admin /content the bulk bar is sticky at the bottom of the visible viewport). and in the context of drupal cms, the buttons are broken in gin:

i've pulled the latest changes in 2.0.x but i wonder where is the threshold set introduced with this MR?

it looks like i've ran into the same problem on https://www.drupal.org/project/drupal/issues/3485202 📌 Update to jQuery 1.14.1 and use the newly added option for dialog modal headings Active (link to the test-only changes job https://git.drupalcode.org/issue/drupal-3485202/-/jobs/3890198). per @catch in #testing where i've asked about my problem (https://drupal.slack.com/archives/C223PR743/p1735958223766589) it wouldn't hurt to have an extra example in here.

aside the browser_output i've also noticed https://git.drupalcode.org/issue/drupal-3493671/-/jobs/3819911 also contains the grep warnings grep: warning: * at start of expression.

jurgenhaas → credited rkoller → .

i've missed uploading the referenced claro video, apologies

Apologies for the delay. I went into the rabbit hole of reevaluating the gin audit which is also relevant for klaro in consequence (and i am far from finished yet but remembered the klaro issue in todays track meeting when the discussion got to open and remaining tasks.. i'll step through the list of changes and only comment if necessary or if i notice something new to keep the comment shorter:

title and description

i liked you changes in #21

Title: Adjust the UI to Drupal style
Desc: Use Drupal-like appearance for Klaro! elements. There are also customized designs for <em>Olivero</em>, <em>Claro</em> and <em>Gin</em>.

( i would even consider to strike the word "also" as it is sort of a filler)

but in the interface i currently see:

Adapt the UI to Drupal
Activate this to overwrite the Klaro! styling with a Drupal-like appearance. There are also customized designs for Olivero, Claro and Gin.

Gin

notice:

Yes, because links have "currentColor" and buttons another color.

the only problem i see taking a closer look, something i've missed so far. the outline for the dialog and the two buttons has with rgb(82, 179, 234) against rgb(250,250,250) only a color contrast of 2.2:1 but 3:1 would be required (plus the buttons have also the problem of simultaneous contrast close to the focus outline - got eased by the introduced offset)... but the color contrast is rather something for a follow up or an issue in the olivero issue queue.

notice: (windows high contrast)

the notice dialog has no focus outline
Fixed.

i am unable to confirm that in edge on macos (see olivero-whcm.mp4)

consent(windows high contrast mode)

the toggles dont have a focus outline.
Fixed.

i am unable to confirm that either unfortunately (see olivero-whcm-toggle.mp4)

Claro

consent(windows high contrast mode)

the close button is not autofocused like without the WHCM
Its autofocused, cant reproduce.

the toggle buttons have no focus outline.
Its autofocused, cant reproduce.

see claro-whcm.mp4

Gin

Going through your comments and with the fresh memory of the current reevaluation of the gin audit it is definitely the right call to make the fixes upstream in gin first. so i spare you with more nitpicks for gin ;)

so overall the current changes are a huge improvement over the previous state, a tremendous effort on your end. thank you for that! probably best have a quick chat about the remaining points from my comment on slack after you've reviewed them maybe? but commit the issue right after and then focus on fixing issues upstream in olivero, claro, gin and klaro.js and then do the final cleanup in a followup in klaro. what do you think about that plan?

@aidil in regard of image sizes, there is https://www.drupal.org/project/drupalorg/issues/3265866 → ... with drupal 10/11 available that issue might make sense to set active again? and i agree using pngs for images isnt the best choice (photos should better be jpg,webp and/or avif and should also be optimized as well - just optimizing the "woman with laptop" png with imageoptim saved 75,3% in file size)

grienauer → credited rkoller → .

i agree it is related imho, but the problem is, at least at this point, not fixed by the MR on this issue. i've just tested, on a content type where the entity ID is hidden on a vocabulary entity reference field, and i've entered "another (23)" on the node edit form when creating a new node and i still ran into an error: (The referenced entity (taxonomy_term: 23) does not exist.)

one detail i'Ve just noticed testing MR10308.... when i edit a entity reference field on the "manage form display" list page and untick "show entity ID", press update, press save, and then take a look at the field widget summary, it still shows "Show entity IDs: Yes" instead of the expected "No"

Updated the issue summary

i've already posted in #testing last night https://drupal.slack.com/archives/C223PR743/p1735958223766589 and asked there, but no answer yet. will give it another day or two and then repost in contribute (or comment in one or another thread that i found in the context of test-only tests not failing). but aside that detail i leave the issue at needs review so people can take a look at the recent changes. think the only thing left to do aside another round of review might and figuring out the test-only problem is updating the issue summary so it reflects the recent changes. will do so the next days.

@oily i've already added the changes we've discussed yesterday outlined in #30 📌 Update to jQuery 1.14.1 and use the newly added option for dialog modal headings Active to the MR last night (basically added the h2 to non modal dialogs and included an assertion that one of the non modal dialog examples has their title wrapped in an h2).

I got the automated tests green again but the test-only changes confuse me and remain green/passing. i followed your recommendation and tested locally. i simply copied the content of DialogTest.php into the clipboard, then checked out 11.x, then opened DialogTest.php again and pasted/replaced the test code with the one from the MR, and finally reran that test on 11.x with the test code from the MR.... there things fail as expected:

FAILURES!
Tests: 2, Assertions: 14, Failures: 1, PHPUnit Deprecations: 1.
Failed to run phpunit core/tests/Drupal/FunctionalJavascriptTests/Ajax/DialogTest.php: exit status 1

but on https://git.drupalcode.org/issue/drupal-3485202/-/jobs/3890198 it looks like tests are being skipped(?!?) and that is the reason why the job is shown as passed?

OK, but some tests were skipped!
Tests: 2, Assertions: 0, Skipped: 2.

commented on the other as well: #3485202-30: Change the dialog modal heading from span to h1 with the uiDialogTitleHeadingLevel option added in jQuery UI 1.14.1 →

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2025-01-03 Active . That issue will have a link to a recording of the meeting.

For the record, the attendees at the usability meeting were @benjifisher, @rkoller, and @simohell.

We’ve discussed the open question whether a section heading element should be used to wrap the title of a non-modal dialog same as the title of modal dialogs, and if so, which heading level should be chosen? There was the clear consensus that same as for the modal dialogs, any section heading element would a clear improvement over a span element. Since the non-modal dialog isn’t wrapped into an article or section element, it wouldn’t be advisable to go with an h1 just for consistency reasons; any page the non modal dialog is invoked on might probably already have an h1 element. Going with an h2 instead would the most neutral semantic choice, and accept in consequence that there “might” be a potential skip of a heading level. So although hierarchical gapless heading structures reflect the best practice, skipping heading levels does not represent a WCAG failure.(https://www.tpgi.com/heading-off-confusion-when-do-headings-fail-wcag/).

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

p.s. @mgifford agreed and leaned towards the proposed resolution going with an h2 for non modal dialogs in a thread on the Drupal Slack https://drupal.slack.com/archives/C1AFW2ZPD/p1735760864177209 in the run-up to the meeting, where i’ve asked for more opinions in the #ux and #accessibility channel.

Left a brief comment: #3370326-13: Refine field descriptions →

Left a brief comment: #3370326-13: Refine field descriptions →

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2024-10-25 Active . The link to the recording of the meeting is https://youtu.be/yuaPVd6Lgv0. For the record, the attendees at the usability meeting were @avani.bhut, @benjifisher, @rkoller, @rosendofig, @shaal, @simohell, and @worldlinemine.

And we have discussed this issue another time at 📌 Drupal Usability Meeting 2025-01-03 Active . That issue will have a link to a recording of the meeting. For the record, the attendees at todays usability meeting were @benjifisher, @rkoller, and @simohell.

In both meetings we’ve continued the wordsmithing of the micro copy for the field type options in the second step of the field creation flow. The work in progress can be found in column C in the spreadsheet (https://docs.google.com/spreadsheets/d/1Lx7L40eRHotr5KQGn6au5WxQO3lFv19a...). We plan to continue reviewing the current draft found in column B/D in the upcoming meetings in case no other issues come up.

not sure why the videos havent been uploaded and added back then. apologies. in both videos i first tab through the breadcrumb and then shift tab back to the start as illustrated by the keycastr output. then i do the same by stepping through the breadcrumb one way pressing VO arrow right... in claro the separators are announced and in focus, in olivero they are ignored.

benjifisher → credited rkoller → .

Changed the parent issue to 🌱 [Plan] Improve field creation experience Active ,which is still active, in contrast to ✨ Make field selection less overwhelming by introducing groups Fixed which is closed and fixed. for a while now. otherwise this issue might slip through.

Thank you for all the work! A few comments:

Title: Adapt the UI to Drupal

I am part of the group of people working on Drupalisms in Drupal, we are still mainly in the audit phase collecting the set of nouns, verbs and adjectives that build the mental model and grammar for drupal, there happened no discussion nor work uni- and clarifying the wording yet. but there is at least one detail i try to recommend and mind myself: i would first look at the current list for any noun, verb and adjective in question. Adapt is a verb that not looked like the perfect pick and also looked unfamiliar to me. taking a look at the current list at https://docs.google.com/spreadsheets/d/1p6i00ya_H9UHVagbpAUrJuuyw7GlUU4e... it turned out that Adapt wasn't listed yet. Did a quick global search in a core installation in vcode and Adapt was only found once in a help topic and aside that three or four more times in code comments. Personally I think Adjust would be more precise? Plus adapt "to Drupal" is also sort of imprecise, it might be sort of misleading in case a site administrator has installed a custom default theme that is not olivero and or neither claro/gin then the actual purpose of that checkbox might be potentially confusing same as the actual outcomes? How about:

Adjust the Klaro UI to the default and administration theme

It would use the verb adjust which is already used across the admin ui and which is more suitable in this context, making it more explicit which UI is adjusted and finally clarify to what the klaro ui is adjusted to, which is the chosen default and administration theme. it is a bit more verbose but clearer and more explicit from my point of view.

Desc: Activate this to overwrite the Klaro! styling with a Drupal-like appearance. There are also customized designs for Olivero, Claro and Gin.

the phrase "Activating this...." or "Activate this" is sort of redundant and unnecessary. A user has to skim by that snippet until the details of interest start. In the micro copy for descriptions in core in general that stylistic device isnt used. How about something like:

Klaro provides styles for Olivero, Claro, and Gin. Learn more [how to adjust your own themes].

It is front-loaded that klaro provides styles for olivero, claro, and gin, which is the "default" when this option is active, and then you provide a direct link to the docs page you've mentioned for the case a user is using their own theme. definitely not perfect yet but just an initial idea how to rephrase and shorten that description?

Styling:

Olivero
notice:
the strong white margin around the focus outline when the dialog is in focus looks odd if the focus switches to the customize link the the dialog shrinks in size significantly (against the dark/black footer)
The focus outline for customize is darker than then outline for the dialog and the buttons

notice: (windows high contrast)

• the notice dialog has no focus outline

consent
the privacy policy and the powered by klaro links have also a darker focus outlines.
the toggles dont have an offset like the two buttons.

consent (windows high contrast mode)

• the close button has a white focus outline when the dialog modal is opened, if the focus is moved to the privacy policy and then moved back to the close button then the focus outline is in the correct blueish color.
• the toggles dont have a focus outline.

Claro:
notice:

• still think the dialog element is hard to distinguish against the background due to the fact that only a light drop shadow is being used. feels not in line with SC1.4.11

consent: (windows high contrast)

• the close button is not autofocused like without the WHCM
• the toggle buttons have no focus outline.

gin (light)
notice:

• the focus outline (with the standard settings) has 3 pixels of the current focus color #7EB2FD and one pixel of a darker #6690CE?!
• in claro we have a offset for the focus outline for buttons, in gin there is no offset for focus outline (that way not meeting the color contrast requirements - but that is a gin problem not a problem of klaro - but the offset would at least ease the problem in the short term)

notice: (windows high contrast)

• the customize link has no focus outline

consent

• the toggles and buttons dont have an offset for the focus outline either. and there is also the problem of that one pixel thick darker focus outline.

consent (windows high contrast mode)

• only the two buttons have a focus outline, both links and the toggles as well as the close button miss a focus outline.

gin (dark)
notice:

• if the dialog is in focus the one pixel thin dark outline between the blue focus outline and the white dialog border is a different dark than the
• two available dark background colors in gin. personally i wouldnt add an offset or additional focus outline color to the focus outline around the notice dialog . the focus outline should be directly next to the border.

notice: (high contrast mode)

• the customize link is missing a focus outline

consent

• and it looks like the toggles and buttons have an offset with the dark mode active?
• powered by klaro has a too dark color contrast #5c5c5c against #2A2A2D (2.1:1 the required minimum is 4.5:1)
• and one note i am not sure if it was already the case in the previous iteration it looks odd that the background of the dialog is invisible. it is plain black here.

consent (windows high contrast mode)

• only the two buttons have a focus outline, both links and the toggles as well as the close button miss a focus outline.

i hope i got everything now. jumping inbetween browsers, themes, different states, and settings is super confusing. i think if the points i've listed are solved, the current state of the MR would be a significant improvement. some more work might be still required about the styling of the notice dialog, in particular the styling of the toggle button component, as well as you've already mentioned, the styling in the context of the windows high contrast mode, but i would move those to follow-ups probably?

thank you! #28.1 and #28.2 looks good and fixed now, excellent!

- about #28.3 there is one detail i've just realized when retesting. when you get to the end of a list by pressing arrow down and then immediately continuing on top and the other way around when you press the up arrow and when reaching the first option and then you immediately continue from the bottom only applies when voice over is enabled. with voice over disabled on select lists you dont jump to the other end of the list and continue, you just stop.

- there is one detail that is potentially confusing and sending mixed signals for sighted screenreader users when voice over is enabled. when you tick a checkbox the voiceover cursor jumps ahead, sometime by one sometimes by more options, but the focus remains on the option a user just ticked and often moves back to the option in focus. so the focus and the voice over cursor are not in sync that way? (vo_cursor.mp4)

and could you provide me some advice how to best switch between commits? at the moment i am uncertain how to compare the two approaches you'Ve mentioned illustrated in the two different commits. but i've recorded another video to illustrate the aural interfaces of the multiselect and the select (popup_and_summary.mp4). for one you know when interacting with the select that you have a popup button so something will "show up" while for the multiselect you only have a "group" which doesnt provide much context about the inherent functionality. and for the select you get the summary with the number of available options while for the multiselect you have nothing.

i ran the test-only test now, but unfortunately it succeeded, and it sounded like it would have to fail instead?!

hm i keep getting errors. the first two times i've restarted the 4/8 functional test you've mentioned, it failed each time with another mismatch for a string. totally unrelated to the changes in this MR. i've reran all tests now, again functional test 4 out of 8 fails but this time with a 403... plus an error in a test for package manager: https://git.drupalcode.org/issue/drupal-3485202/-/jobs/3827987 :/ confusing and "sort of" arbitrary.

ahhhhh and now i see the test only changes test. understood. thank you for the pointer, i would have missed that!

i was completely wrong and got confused by the several context switches in a row. in the given example with 119 and 121 the point of reference was NOT link 7 which is a none modal but button 1 which is a modal (line 113) and which i've missed. so the tests are correct. managed to get the local tests running and therefore i was able to validate and cross check. so your changes are all correct and doesnt look like a shortcoming or bug of the existing tests.

and i'Ve pushed another commit that contains two asserts that add checks for the aria-modal attribute. locally those work for me.

but if you install the ajax-test module the onethe tests are using for the given tests and you take a look at link 1 (modal) then this modals title is an h1 with an aria-modal="true" attribute while if you take a look at link 7 it opens a dialog that has no aria-modal attribute and the title is wrapped in a span. therefore the assertion in 119 and 121 would have to fail imho currently since they check for h1 and that h1 is not in place but a span instead still?! is it possible that the reason for the tests not failing is because modals and none modals are open both?! cuz the different dialogs are opened in the dialogTest.php but only the first opened dialog is also closed in line 70 with $close_button->press();, the rest is kept open?

thanks! but on a closer look it is surprising that the tests pass with your recent changes. cuz you've applied the change from span to h1 for every occasion of span. but some of the test cases are also for dialogs that are no modals (for example line 119 and 121 in the context of link 7 - non-modal, no target), cuz h1 is only set for dialog that are actual modals at this point in dialog.js

    dialog.show = () => {
      openDialog({ modal: false });
    };
    dialog.showModal = () => {
      openDialog({ modal: true, uiDialogTitleHeadingLevel: 1 });
    };

but maybe a good reminder to change the wrapping element for dialogs that are no modals as well? i would lean towards h2, might be a relatively save call in this case, but i've asked @mgifford and will report back when i get feedback.

and about the assertion about aria-modal and your comment:

The return value of getAttribute() is here. It is not true or false it is the value of the attribute or an empty string. So asserting that $dialog->getAttribute('aria-modal') is true or not true does not work. Okay, it seems the value of the 'aria-modal' attribute should be true. So that should be fine.

does that mean that $this->assertEquals('true', $dialog->getAttribute('aria-modal'), 'Dialog modal has aria-modal attribute'); would be fine? at least the same pattern is used on for example in FromGroupingElementsTest.php in line 136, therefore i thought it would be save to use, otherwise it would have to be corrected in more places in core tests? and i am still uncertain where to place that assertEquals in the DialogTest.php

ha i think we can stay "relatively" dry. the tests failed... the pipeline that contains the test where the two new asserts got added passed https://git.drupalcode.org/issue/drupal-3485202/-/jobs/3825700 :D buuuut another pipeline failed https://git.drupalcode.org/issue/drupal-3485202/-/jobs/3825702 and it looks like there is another test that tests not the positioning but the dialog in general (DialogTest) ... and the two asserts that fail there check for span.ui-dialog-title:contains('AJAX Dialog & contents') ... so i guess i remove the tests from DialogPositionTest, and update the selector in DialogTest and add the assert for the aria-modal there instead as well including the comment . and about the failing nightwatch test https://git.drupalcode.org/issue/drupal-3485202/-/jobs/3825720 i will see later, seems unrelated.

uh that is odd that the tests got green without any changes to the MR.

And about the to be added tests. i've tried to chop together a starting point. I am not a developer and haven't done any work on tests so far (only managed to create the upstream changes for jquery ui in relation to dialog modals including the necessary tests for both PRs). The first PR added the aria-modal attribute the dialog. when that was added to Drupal 11.0.0/10.3.0 there was no test added. therefore i think it would be good aside testing that the title is wrapped in a h1 also to test that the dialog modal also got an aria-modal attribute added. looking at the existing tests, there is core/tests/Drupal/FunctionalJavascriptTests/Dialog/DialogPositionTest.php, and i thought it might be the easiest to simply add the two necessary assertions here. even though it is against the single responsibility principle and the test is mainly about the positioning of the dialog modal. but if a dedicated test making sure the aria-modal attribute is set and the title wrapped in a h1, that test would contain a good share of steps DialogPositionTest.php already contains - so basically all those steps would be redundant and unnecessary. so i simply added the assertions to DialogPositionTest.php for now and added the reasoning and what is happening to a comment. as i said it is only a starting point. if people think it would be better to move those assertions to a dedicated test and or if the assertions need work please correct me and update the MR.

I've taken another look at the current state of the MR. Overall it looks quite solid, there are a few details i've noticed in no particular order:

1. if you have voiceover active and the muliselect is expanded and the focus is on the unchecked access control option and you press VO space in safari, instead of checking the category the checkbox remains unchecked and voiceover announces that it is an unchecked checkbox another time. in firefox and edge i am able to properly check and uncheck checkboxes by pressing VO space.

2. if you have voiceover on and you scroll the categories down until the end with the arrow down key in safari, the list collapses as soon as the last option is passed. that behavior does not happen with voiceover switched off in safari. in firefox and edge it does not happen no matter voiceover is enabled or disabled.

3. one detail in general, i've compared again the general behavior of select lists, went to admin/config/regional/language/add, and there if you reach the end of a list you automatically jump back to the top of the list and the other way around, if you reach the top of the list (by arrow up) you jump to the last item if you keep scrolling by the arrow key. so i wonder if the multi select should mirror that behavior as well?

in regard of #26 📌 Improve keyboard navigation for categories dropdown Active . what would be the recommended approach to switch to the prior commit? the commit_hash for the prior commit i could get with git log but what would be the best way to switch to it and being able to compare the two states? https://stackoverflow.com/a/4940090 suggests creating a branch?! the other suggestion by simply checking out the commit_hash here https://stackoverflow.com/a/30988318 would lead to a detached head? before i do anything wrong i thought i better ask.

@utkarsh_33 Yes i agree within the scope of this issue, having a button to reset the selection would make sense, everything else noted is rather suitable for followup issues for project browser and or core.

i've taken a look at the latest state of the MR after i was finally able to test without any error again.

point three in the remaining steps section is still to open... it is more about the consistency with claro than an actual a11y issue, but the checkmark is still a different one than the one in the drupal design system. but in the context of that checkmark there is also an a11y related issue to note. currently the checkmark has the color green (rgb(115,179,85) against the white background which leads to a color contrast of 2.5:1 which is not in line with SC 1.4.11 which requires a contrast of at least 3:1)... in the drupal design system the checkmark uses black which would have a high enough color contrast.

and one more detail i've noticed when retesting . the aural interface for the installed button currently is: Installed Pathauto is installed , dimmed button which is sort of redundant and hard to process for a screenreader user. i wonder if it would make sense to make the image for the checkmark decorational (alt instead of alt="Installed") and also drop the "is" on the visually hidden span so you only have Pathauto installed as the aural interface in the current example?

went through the three themes in high contrast mode:

all themes:
for the notice a drop shadow is not available in high contrast mode, so in every of the following videos you will notice an outline is completely missing, only claro has at least a focus outline, but a border is missing entirely (same for the consent dialog modal). i would suggest to go with the approach used on the dialog modals in drupal core and simply use a border same as for the dialog modal on for example a view.

Olivero (olivero.mp4) & Gin (gin.mp4):
notice dialog

• the notice dialog is missing a border
• the the notice dialog modal, the decline button, and the accept button are missing a focus outline
• the border of the decline and accept button have a too low color contrast, it only has 2.9:1 (#575757/#000000), the required bare minimum is 3:1, but the more the better.

consent dialog

• the consent dialog is missing a border
• the toggle buttons are invisible.
• the not always required toggle has a white focus outline instead of blue
• the decline and accept selected has no visible focus outline

Claro (claro.mp4):
notice dialog

• the notice dialog is missing a border
• the border of the decline and accept button have a too low color contrast, it only has 2.9:1 (#575757/#000000), the required bare minimum is 3:1 but the more the better.

0

consent dialog

• the consent dialog is missing a border
• the toggle buttons are invisible.
• the not disabled toggle has a white focus outline instead of blue

@jan kellermann i've discussed the whole problem space of the design, styling, and the requirements in regards of accessibility last friday and having a programmatic adjustment nico outlined in ✨ Stylings per used theme Active was the outcome. personally i would have kept the scope tight and made ✨ Stylings per used theme Active only about the functionality being able to switch the css based on the current theme while the styling should be still tackled within this issue. but it is also fine with me if we combine both in ✨ Stylings per used theme Active we could close this issue cuz it becomes redundant.

Thanks for the changes. A few more observations and thoughts:

micro copy:
i completely agree with @anybody that the previous label Override Klaro! CSS was unclear and confusing, but Use Drupal-like styles i not necessarily consider explicit and clear as well. As a user it makes me think what drupal like refers to? But the wordsmithing might lead into bikeshedding but i guess as long as the styling and a11y issues arent ironed out we could still keep discussing the wording for now;) and in regard what to write instead, i would label what this checkbox is all about, using or adjusting the styling of klaro to the installed drupal themes. and in the description i would add that klaro provides out of the box the styling for the two core themes olivero and claro as well as gin. and maybe also provide a link to a docs page how to handle the case if another frontend theme instead of olivero is being set as the default theme? so a site builder is being able to provide bespoke css for their theme of choice?

All themes:

• Most target sizes are good to go now, only the close button has only 24x20 pixels and is not meeting the requirement of at least 24x24px
• in regards of the toggle/checkboxes, a pitty that it is impossible to adjust that styling. :( then the styling for those toggle buttons might need some adjustment, cuz it has color contrast issues. will write up the problems in a follow up comment.
• about "Edit: not sure of the white shadow should be set for focus and / or for focus-visible, see" i am not sure. i gotta do some research, cuz i stumbled across one issue for core where there were opposing opinions, one person recommended focus while the other recommended focus-visible. will also add it to the follow up comment. from the top of my head i dont know the best practice.

Gin (light mode):
the notice and the consent dialog are both minding now the selected accent and focus color - fab! one thing that is confusing, the toggle buttons stick to the strong green background color. but if you change to the dark mode that background color for a toggle is following the set accent color as well now? is that intentional for gin or is that sort of a bug (i am using 4.0.0) - it is definitely an inconsistent behavior in between light and dark mode for gin?

The customize link on the notice, the privacy policy, the services group toggle, the toggle buttons and the powered by link on the consent dialoge still have the blue focus outline instead of the current selected focus color.

Gin (dark mode):
the background color is still white there. and there is sort of a white margin even outside the focus outline.

and circling back to the gin light mode the notice modal is hard to recognize against a white background without the focus outline active.

The customize link on the notice, the privacy policy, the services group toggle, the toggle buttons and the powered by link on the consent dialoge still have the blue focus outline instead of the current selected focus color.

Olivero:
the focus outline around the notice modal is visible... but the color for the focus outline for the notice modal and the focus outlines within the modal differ that is for sure, plus the real problem the focus outline is invisible for the blue primary buttons. (and the pattern that is listed for gin and claro applies here as well -> The customize link on the notice, the privacy policy, the services group toggle, the toggle buttons and the powered by link on the consent dialog modal still have the blue focus outline instead of the olivero outline

Claro:
The customize link on the notice, the privacy policy, the services group toggle, the toggle buttons and the powered by link on the consent dialog modal still have the blue focus outline instead of the green.

I've manually tested on 11.x .... with the feature branch checked out the email address and the username of the person last registered with the browser is no longer displayed, the fields for the email address and the username remain empty. as a crosscheck i've then checked out the 11.x branch again and reloaded the create new account page and the fields for the email address and the username get directly repopulated. so from a manual testing and privacy perspective a big +1 for the approach taken. and if the deprecation of the behavior can be taken care of in a separate follow up it would speed up the resolution of this issue. i leave the the status at needs review cuz i am unable to provide any feedback about the code.

Thank you for the MR! Initially i've struggled quite a bit cuz i've only faced the klaro upstream styling until i've realized i have to check the "override klaro css" option. :/ in general i like the direction the MR is going. but there are a few details to note about the styling:

all themes:

• between the focus outline and the component of notice dialog is a tiny offset. when there is any typography or content in general underneath due to the fact the dialog has no real border but just a slight dropshadow the underlaying content shines through - having no real border and just a light drop shadow makes it difficult depending on the background color (in particular on white) to distinguish the notice modal and meeting SC1.4.11 which requires a color contrast of at least 3:1)
• some touch targets don't meet SC2.5.8. (there is a meta issue for core 🐛 [meta] Some interface components don’t meet the minimum target size Active ). on the notice modal it applies to the customize link, and on the dialog modal to the close button, the privacy policy and the powered by klaro link, the checkbox labels and the toggle buttons to expand and collapse the services groups.

Claro:

• the focus outline in safari is in a light blue (rgb(143, 178, 249), in firefox in a darker blue (rgb(0, 122, 255), and in edge close to black (rgb(16, 16, 16)) but the color for the focus outline in claro should be in green (rgb(39, 167, 105)) - applies to the notice as well as the dialog modals
• the toggle button component gin and klaro are using is not available in claro, there only the checkbox component is being used instead? plus visually there is no way at all to distinguish between the current disabled and active checked/toggled state of the different toggle button. the difference is a color contrast of 1.5:1 (#4162D6 disabled and #003ECC active and checked) but due to simultaneous contrast the color contrast between the two appears to be even less than 1.5:1. And the disabled state is a problem on its own and out of the scope of this issue. but from my perspective i would go with the checkbox component for the claro theme (what do others think), it would be more consistent and the disabled state for checkboxes is more easy to distinguish from the checked and unchecked state - but on the downside the disabled checkbox state has color contrast issues, but those are there for core in general as well.

Olivero:

• Olivero uses the same set of focus outlines as Claro (safari is in a light blue (rgb(143, 178, 249), in firefox in a darker blue (rgb(0, 122, 255), and in edge close to black (rgb(16, 16, 16))), but in the case of olivero i am "not" sure what their default focus color is. there is no general "green" like in claro, but depending on the background and context, the focus outline styling changes. so i am not sure which of the few available to pick here?
• Same as Claro, i am not sure if something like that toggle button component Gin and Klaro uses is available for Olivero? Would it make sense to go with checkboxes in the case of Olivero as well?

Gin

• Gin uses the same set of focus outlines as Claro (safari is in a light blue (rgb(143, 178, 249), in firefox in a darker blue (rgb(0, 122, 255), and in edge close to black (rgb(16, 16, 16))). but the styling should follow the accent and focus color set in admin/appearance/settings/gin or on the user profile if overrides on a per user basis are active.
• The styling of the toggle button has a few color contrast related issues but those are out of the scope for this issue. that is something for the gin theme upstream. i am currently going through an audit we've done a few months ago and validate which of the points are still relevant (the spreadsheet is linked in the issue summary of this meta issue: #3475279: [Meta] Audit each module to be included into Drupal CMS for accessibility issues → ) .