the_g_bomb → credited rkoller → .
a third issue got put on the list 📌 Disallow dangerous filenames e.g. command injection characters Active (i guess @benjifisher might be familiar with it from discussions in the security team)
i am unaware about the situation back when the issue was created by @marvil07, but currently if a user is able to clear a search field by pressing the ESC key, like @hitvikav_ was able to, is solely dependent on which browser is used. I've created a small comparison table for ✨ Add a clear button to the fields ui Active :
In the linked issue i've suggested a bit broader solution. At the moment some browsers provide only browser based solutions to clear a search field by showing a clickable clear button as well as adding the ability to clear the field by pressing the ESC button. But the behavior as well as the styling differs across browser, if they have one. My suggestion in the linked issue is adopting the clear button that was added to project browser ( ✨ Ability to clear keyword/search filter with one click Needs work , 📌 Remove the clear button from the tab order Fixed ) for core in general.
the update to the issue summary helped, i am able to reproduce and test now. thanks a lot for the updated instructions @grevil!
there are currently two issues in need for usability review:
✨
Open the "discard changes" dialog in the off-canvas tray
Active
🐛
Drupal Current theme condition plugin should provide an option to select all themes
Needs review
i can put the issue on the shortlist for the ux meeting on friday. just one question upfront and one remark. is it an intentional behavior that the discard changes dialog is only shown on the layout for a node/content item, but when i discard a change on the layout of a content type i still get to a confirmation page? and i guess the MR needs a rebase because of the recent renaming changes in package_manager, on drush cr i get PHP Fatal error: Uncaught Error: Class "Drupal\package_manager\SandboxManagerBase" not found in /var/www/html/repos/project_browser/src/ComposerInstaller/Installer.php:14
i saw the needs usability review tag, and we can put the issue on the shortlist for the meetings on friday. but only one question, i've tried to quickly test the MR on 11.x now, but i have a hard time to see any actual change in the ui. the proposed resolution section in the issue summary is not very clear, i then tried to follow the instructions in
#32
🐛
Drupal Current theme condition plugin should provide an option to select all themes
Needs review
, but again i see nothing in the ui of a block configuration page on a block layout page enabling me to choose a theme (nothing theme related in the vertical tabs nor in the select for the region - and i ran an drush updatedb as well as a drush cr).
the_g_bomb → credited rkoller → .
Thanks to all the candidates for their submissions. My votes go to:
- Marine Gandy (mupsi)
- Nico Grienauer (grienauer)
one detail i forgot to mention, we've also wondered if it might be necessary in a followup to update some of the documentation on d.o? the core codebase seems already properly covered by the MR. we haven't had the time during the meeting to do any in depth research in that regard
left a comment on #2447955-21: Change "Extend" menu item to "Extensions" →
Usability review
We discussed this issue at 📌 Drupal Usability Meeting 2025-05-02 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.
Thanks for picking up this issue and already providing an initial MR. The group had a clear consensus about the point in
#2447955-19: Change "Extend" menu item to "Extensions" →
with Extend being a verb while the rest of the menu items are nouns. This is a detail that was also raised during the discussions in
📌
Locate drupalisms that might create confusion among users
Active
and in the following we’ve encountered it again during the noun foraging exercise for Drupalisms.
We’ve then taken a look at the bigger picture, at this point the extend page only contains modules. Project browser is adding the concept of recipes to the extend page. In the future there is also the plan to add the concept of themes there as well. In parallel there is also the plan to restructure the information architecture of the admin ui: 🌱 Restructure the admin interface Information Architecture Active . In that context there was also one idea voiced in the course of an ux meeting and summarized in the second part of: #1973322-26: Add drop-buttons to the appearance page → about moving the theme list page into the extend page as well.
Taking these circumstances into account, we’ve then asked ourselves, is Extension the right choice or is there maybe a more clear alternative as the top-level menu item?
First we’ve looked how that kind of concept is called in other CMSes - WordPress for example uses Plugin. Problem with that noun, in the context of WordPress it is only the counterpart to the concept of modules in Drupal, while in the context of Drupal the term is already used as a “sub” concept of the concept of modules even though the user doesn’t have many touch points in the micro copy with that concept - so we’ve discarded it.
The other term that we’ve discussed was Project which is already used in the context of project browser as well on d.o. Problem with that, the term is way too generic plus it is missing some sort of cue what the purpose of that concept actually is - “project” is sort of too neutral.
Extension on the other hand clearly expresses the purpose of concepts like modules, recipes, and themes to extend Drupals function wise, at the same time all top level menu times become nouns. And we suspect that some people might won’t even notice the change if they only skim the top level menu items. So over all we had a clear consensus to agree to change the verb Extend to the noun Extensions.
If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.
benjifisher → credited rkoller → .
the_g_bomb → credited rkoller → .
I’ve taken another look at the current state of the MR and created two small videos to illustrate the difference between the current state of the navigation (check 11.x.mp4) and the state in the MR (check MR.mp4)
The point that still needs some more discussion is the level of granularity of landmarks, as outlined in the first question in the google doc by @katannshaw. In 11.x you currently have the complementary wrapping landmark and the two navigation landmarks for the content and the footer section (11.x.mp4). With the MR you now have the complementary wrapping landmark and another landmark for each of the five navigation blocks (MR.mp4) - three of those blocks even have just a single menu item.
We have discussed the matter in the accessibility office hour end of march where we havent come to a consensus which of the variants outlined in the first question of the google doc should be picked. Back then we agreed to ask for feedback over on the accessibility slack about the preferences and expectations of screenreader users in regard to landmark regions - reason is that quite a few screenreader users are part of the community there. But unfortunately we havent received any actionable feedback yet :( In addition to that i’ve posted over in the CMS Garden Rocketchat instance and asked in their a11y channel as well.
The worry and the reason why we tried to get feedback from actual screenreader users simply is, that the number of navigation landmarks might be way too granular - to reach the main landmark with the MR applied you have to pass by eight landmarks first (five navigation landmarks and two more complementary landmarks, both wrappers for the navigation sidebar and the topbar). The docs of the skipto script (https://skipto-landmarks-headings.github.io/page-script-5/config.html) state something in a similar vein:
The list of landmarks and headings should be relatively short, the more items the menu contains the more time the user will need to scan and navigate to the section they want to “skip to”.
“Strictly” speaking, landmarks should segment a page into semantically clearly distinguishable sections, so a user is able on one hand to understand the sites general structure and on the other hand is able to skip to the section in question more easily. but the five navigation blocks are semantically too close/similar, since together they build the main navigation aka navigation sidebar.
The other minor problem i see, the complementary landmark that you get with the aside element feels semantically wrong, complementary is about supporting the main content (https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/), which might be the right choice in cases like for example https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/nav, but for something like the main navigation it feels not like the best pick?
Personally I would lean towards a solution with two landmarks, one for the navigation sidebar and one for the navigation topbar. For the sidebar i would use a navigation landmark and label it with primary, that way it would be crystal clear that this is the main navigation for the admin ui. For the topbar the complementary landmark is fine. But that is only my personal opinion. Curious what Kat and others think about it, plus i still hope to get some feedback from actual screenreader users, those who actually use landmarks on a daily basis would be the most helpful answering this question. :/
benjifisher → credited rkoller → .
Thanks a lot for all the work! I went through everything and tried to review as much as i was able to, a few comments and questions.
- most of the changes look like those to satisfy the linter by adjusting indentation and or adding commas at the end of arrays.
- explicitly setting array() isnt necessary? thought it would be, that was the reason i'Ve added thoses type. but is it only necessary for functions like
public function buildForm(array $form, FormStateInterface $form_state): array {
but unncessary and or wrong within functions in the cases you've corrected?
'#options' => array(
- a good call renaming generate_skipto_config_json to skipto_generate_config_json. looks more consistent
- and also good call to wrap the landmark labels into t() functions. i am using my OS with english due to the fact that using voiceover in german is unbearable and i was under the impression that those landmark labels would use the english labels as well in consequence ^^. but i'Ve rechecked right now and they are localized. we have to check if there are official translations for those landmarks by the W3C cuz i wouldnt necessarily use landmarks used by a certain screen reader solution in a certain language. meaning, translations might differ inbetween screen readers in the worst case?
- the only thing i am uncertain and unable to check are the use statements you have removed in a view files. is there some automated test to check which are obsolete? or is that a manual task?
so aside the use statements the MR would be RTBC from my end. the use statements i am unable to assess so it is up to you ^^ but guess it should be fine (manually tested aside running phpstan).
benjifisher → credited rkoller → .
the_g_bomb → credited rkoller → .
benjifisher → credited rkoller → .
the_g_bomb → credited rkoller → .
@grienauer noted that aside removing the css asset from the libraries.yml, it "might be" a good idea to also remove the css assets in the first place as well. ^^ completely forgot about that detail.
i've assigned the issue to myself and already created a MR.. Will work on an initial draft the next few days...
Turns out the skipto.css file was unnecessary to add and that additional css was causing all those issue. the three issues listed in the issue summary as well as the problems with the plus suite are solved with MR3. i was only unable to reproduce the problem with the status messages so far but odds are it might be solved as well.
I have to add I also noticed two more issues, but those were only one time and i was unable to reproduce so far, but i'll add them in the context of this issue cuz the "might" be at least related as well:
- Installing the plus suite with layout builder plus, navigation plus, and edit plus alongside skipto made the topbar not showing up and the edit button in the navigation sidebar was shown neither. after several installs and uninstalls both coexisted but it felt flaky.
- the skipto button and the expanded menu collided somehow with status messages and the display of the two in combination was off.
The MR needs a rebase after ✨ Use modal in add new field flow Active went in
benjifisher → credited rkoller → .
I've pushed the remaining changes we've discussed over the course of the last few weeks. The are still strings on the google sheet that still need to go in:
- I was not sure how to add a help text to the second step for selection lists. the proposed microcopy for the help text is:
Each list item has a Name, with limited formatting, and a Value. Values that are in use cannot be changed, since they are stored in the database. The Names can be edited later. - it has to be kept in mind that this issue needs additional work after ✨ Use modal in add new field flow Active goes in, cuz at the moment the h1 on the page isnt changed and adjusted but it has to (and we also prepared suggestion for what could/should be used in the title of the dialog modal.
A few more details about the current state i've noticed applying the changes and going through the interface again:
- for some field groups there is the problem on the second step that the order of options is alphabetical but it would be better to order them in a different manner. for example for
plain textthe labels weretext(plain)andtext(plain, long), so short was first and the more complex second. with the new labelsshort textandlong text,long textcomes first. - for the long text option for formatted text it might make sense to make a characteristic for text areas more explicit.
short textstates that text fields are oneliners whilelong textjust says it uses text areas for input instead. it might be reasonable to clearly communicated that text areas are multi-line? - I've removed the periods at the end of each bullet point on the reference field options. we havent used periods on the bullet points for any other fields.
- would it be possible to add a bullet point to the
otherreference field? it looks odd without any. - on date and time field types it is not entirely clear what the examples refer to. are those the format date and time has to be entered or is it the example in what way dates and time are stored in the database - it might be misinterpreted.
thank you! that is definitely beyond my skill level to figure out on my own :D will add the rest of the strings tomorrow. need to get some sleep first.
not sure if the issue can be closed. according to #3467181-15: Navigation module button icons are obscured when in Windows High Contrast → the introduction of the icon api in 📌 Navigation leverage icon core API Needs work solved the problem, so it will be fixed with 11.2. but problem is, would there still need to be a fix for 10.x, since gin is still supporting it, and it doesnt look like that the icon icon api will be backported?
benjifisher → credited rkoller → .
replaced the link to 🐛 Navigation block titles have a too low color contrast Active , since it got closed by 🐛 Increase the color contrast of the navigation block title Active
?
if gin is using the exact same shade of grey as claro then grey 800 would be --admin-toolbar-color-gray-800: var(--drupal-admin-color-gray-800, #4f5661); and that is color contrast of 1.9:1 against rgb(42,42,45) ... while the issue you have linked went to --admin-toolbar-color-gray-700: var(--drupal-admin-color-gray-700, #6d7583); which is 3.1:1. #d2d3d3 is just --gin-color-text which is identical to the menu items. but as @ckrina pointed out on the other issue the block title should be an unobstrusive label to a nav block and should not be using the identcial coloring.
i've tried to understand the problem, but i am unable to reproduce the problem in the first place unfortunately. i've ticked "Add close button to the Klaro! dialog" but for me the close button is still in the header section of the modal and not in the footer like outlined in the issue summary. could you provide some more instructions who to reproduce the problem? i am using 3.x.-dev of klaro and i am unable to reproduce the scenario you've outlined? thank you?
I am not sure if aria-expanded would be my attribute of choice here. aria-expanded is usually associated with expanding and collapsing elements in close proximity to the toggle button. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Referenc... also lists the option of elements displayed or hidden, but still aria-expanded is more associated with cases like a details element and the expanded or collapsed summary element- where both are in close proximity.
in this case you have a dialog modal that is displayed in the center of the visual viewport, not in close proximity to the link/button in the bottom right corner. at the moment klarojs hasnt set up the dialog modal properly, it has neither an aria-modal attribute nor is using a dialog element (something we wanted to propose upstream in the klarojs queue on github). so at the moment a screenreaderuser would have the aria-expanded="true" attribute accessible (but it is not anywhere near in close proximity in the dom), but as soon as the aria-modal attribute would be in place or the dialog modal is being used, everything outside the dialog modal element is being excluded from the accessibility object model (AOM) and in consequence the aria-expanded="true" would be unavailable and inaccessible.
my preferred attribute of choice in this scenario would be the aria-haspopup="dialog". that way, if a user is getting the customize link/button into focus the aural interface would be Customize, dialog pop up, button. that way it would be crystal clear to the user that by pressing that link/button a dialog modal would open up. the expectations would be set in a very clear manner. a page for testing the attribute can be found here: https://a11ysupport.io/tests/html/aria/aria-haspopup_examples.html ... the support isn't 100% yet, there is an older overview https://a11ysupport.io/tech/aria/aria-haspopup_attribute as well as a more recent writeup here https://www.matuzo.at/blog/2023/aria-haspopup/ - in general it is good but the main offenders with only partial and or lacking support are narrator and talkback
i've discussed the topic with @mgifford and he agreed to my points, his only worry was also that the support for certain screenreaders is not perfect yet. at the end he added: "We are always working to the future with work on Core. aria-haspopup will get better support"
p.s. but would probably the best choice to make those changes in regard of aria-haspopup as well as aria-modal/dialog element upstream on the klarojs queue
jan kellermann → credited rkoller → .
pushed a commit extending the support to drupal 10 as well (tested on a drupal 10 install and all good) . as written on slack, i would created a release now and then discuss which issues need to be opened in the skipto drupal queue, the drupal core queue, and which in the skipto queue upstream. but i wouldnt make it a full release but rather an alpha1 release at first.
one thing i wonder after changing the target in the title to drupal 11, would it make sense to support certain minor versions of drupal 10 as well? would simply have to expand the info.yml. i only dont know how to test which drupal 10 version would be supported as well?
rkoller → created an issue. See original summary → .
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.
benjifisher → credited rkoller → .
benjifisher → credited rkoller → .
The issue was raised in the #bugsmash channel on the drupal slack. i've retested on drupal 11.x with macos 15.3.1, safari 18.3, in voiceover. - tested on a node in the article content type. i am sort of unable to reproduce the problem with the subject field of a comment, instead to me it looks like more of a problem with the ckeditor field used for the comment body field.
In the video (comment.mp4) i've first added something to the subject field then tabbed to the body field and added some text there as well, then i've shift tabbed back to the subject field and started deleting characters. those deleted characters were not announced. but as soon as i started entering characters again in the subject field either after deleting everything or after one or two characters only the newly entered characters are being announced. if i then delete again characters with backspace in the subject field, those characters are getting announced now as well in contrast to when i shift tabbed back into the subject field. i've tabbed into the body field then and tried to add or delete any characters there, none of that was announced. so from my point of view it seems more like a problem with the body field?! firefox and edge are announcing every character entered or deleted in the subject as well as the body field correctly.
not sure if those changes are reasonable and enough in scope and or valid but at least the errors are fixed and the config inspector lists the schema as 100% validatable now.
will try to provide an initial MR
thejimbirch → credited rkoller → .
the_g_bomb → credited rkoller → .
grienauer → credited rkoller → .
a heavy +1 for bringing SkipTo to Drupal 11
and your proposed solution on the MR you've created with #222330 for the navigation block title color would have a color contrast of 1.1:1 against the background (rgb(42,42,45)) of the navigation sidebar in gin.
for core and the navigation component there is already 🐛 Navigation block titles have a too low color contrast Active so 🐛 Increase the color contrast of the navigation block title Active is a duplicate of that. but i've opened the issue in here because you have a dark mode for gin that does not exist for core. and the shade of grey that is used for the navigation block title in gin is the same in light and dark mode. it is right to fix the color contrast for the navigation block title in core as well. but still the darker grey that will be picked as the solution for 🐛 Navigation block titles have a too low color contrast Active might then not work with the dark background in gin (and being mindful about gin would be out of scope for 🐛 Navigation block titles have a too low color contrast Active ). it might work but still out of scope and why i think it is reasonable having a dedicated issue.
benjifisher → credited rkoller → .
i am taking a look now, was swamped the last few weeks with finishing the draft for gin.
took an initial stab trying to improve the issue summary making it more actionable and instructional.
the_g_bomb → credited rkoller → .
i agree probably it might be not only about the color contrast but also about the font size, a detail we've touched before in yesterdays discussion. and i've updated the issue summary and added a pointer what the navigation block title is in the screenshot.
just as an fyi, the images are not focusable, only the three elements on the image, the checkbox, the edit button and the remove button.
@rahuly072 your theme settings are ok, but you also have to active the forced colors mode. your screenshots show that you are just in dark mode. you have to activate forced colors mode either in your operating system in case you are on windows or in chromium based browsers https://devtoolstips.org/tips/en/emulate-forced-colors/