Nürnberg, Germany
Account created on 7 May 2015, about 10 years ago
#

Merge Requests

More

Recent comments

🇩🇪Germany rkoller Nürnberg, Germany

Left a brief comment on the issue #3370326-54: Refine labels and descriptions for field types and i will push the changes we've discussed to the issue later today.

🇩🇪Germany rkoller Nürnberg, Germany

At 📌 Drupal Usability Meeting 2025-06-13 Active , we continued work on this issue, I will push some of the discussed changes later today. For the record, the attendees at the usability meeting were @benjifisher, @rkoller, and @simohell.

🇩🇪Germany rkoller Nürnberg, Germany

I’ve demo’ed the current state of the MR at the monthly NWDUG meetup last Tuesday. For the record the attendees I am aware of their nickname on d.o were: @asherry, @bernardm28, @dydave, @h2cm, @mr_scumbag, and @philipnorton42. All in all, the first impression was also a good one, an improvement over the current state. They had a few comments on certain field types and groups we’ve taken a look at, in no particular order:

  • On the first step the hyphenation used on the descriptions was considered distracting and making it harder to skim
  • It was suggested to move the bullet point Recommended for to the top of the bulleted lists. It was argued that what a field type is recommended for is the most important information people are interested in, and they could then still read on in case they want to know more in-depth information.
  • In regard to the bullet point Efficient storage on for example short plain text it was asked does that imply that the other ones listed are not efficient then?
🇩🇪Germany rkoller Nürnberg, Germany

left a comment on the issue: #3511972-42: Allow Composer and rsync location to be configured via the UI

and for the record the attendees of the meeting last friday were @simohell, @worldlinemine, and myself

🇩🇪Germany rkoller Nürnberg, Germany

Posted a comment on the composer and rsync issue: #3511972-42: Allow Composer and rsync location to be configured via the UI ... and it looks like the file name issue still needs a comment.

🇩🇪Germany rkoller Nürnberg, Germany

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2025-05-30 Active . The recording of the meeting: https://youtu.be/18q8p7xz7VE. For the record, the attendees at the usability meeting were @benjifisher, @rkoller, and @simohell.

We revisited this issue at 📌 Drupal Usability Meeting 2025-06-06 Needs work . That issue will have a link to a recording of the meeting. For the record, the attendees at the second meeting were @rkoller, @simohell, and @worldlinemine.

We have taken a look at the new settings page on admin/config/system/package-manager, the change to the check for composer on the status report page admin/reports/status, and simulated a few scenarios by renaming composer and rsync binaries in the DDEV web container, and experimented with overriding those executable path settings via the settings.php file. A few observations:

  1. Having a status message that is reporting that the paths for composer and rsync were automatically detected each and every time a user is visiting admin/config/system/package-manager raises many potential questions and might be confusing to the user. One might ask themselves is it necessary to access this settings page first, before package manager is able to autodetect composer and rsync, until then the two binaries remain unavailable to the site and package manager since the auto-detection was not trigged yet? Furthermore one would expect as soon as the two binaries were autodetected the status message wouldn’t show up ever again as long as the path and/or file name don’t change - why is something auto-detected and announced again and again after it was already detected and the executable path set for that location? Having a status message informing the user about a state change, a newly arisen situation, and/or a successfully completed task is the right purpose, but having a redundant message about a task that was already successfully accomplished feels counter productive and leaves a certain uncertainty about the persistence of that autodetected setting.
  2. It seems the accuracy between the auto-detection on admin/config/system/package-manager and the composer test on admin/reports/status is divergent. Out of the box with /usr/local/bin/composeravailable in the DDEV web container admin/config/system/package-manager reports that composer is automatically detected, and admin/reports/status reports it checked successfully for composer returning 2.8.9 (/usr/local/bin/composer). If you rename /usr/local/bin/composer to /usr/local/bin/composerer, reloading admin/config/system/package-manager returns again that the path to composer was automatically detected, while admin/reports/status is throwing an error on reload that composer is not found. If you now change the executable path on admin/config/system/package-manager to /usr/local/bin/composerer and save, the status message again returns that composer was automatically detected, same for reloading admin/reports/status, it reports that it successfully checked for composer returning 2.8.9 (/usr/local/bin/composer). The only time you are able to trigger an error on admin/config/system/package-manager is by changing the executable path to /usr/local/bin/composer while path and binary are still renamed to /usr/local/bin/composerer. In consequence, the auto detection on admin/config/system/package-manager feels not trustworthy compared to the status check on admin/reports/status.
  3. If you change the composer executable path from /usr/local/bin/composer to another binary that is available in the same directory within the DDEV web container, like /usr/local/bin/node, the form is still getting validated successfully.
  4. If you set $config['package_manager.settings']['executables']['rsync'] = '/usr/bin/rsync’; in the settings.php and change the rsync executable path to /usr/bin/rsyncer on admin/config/system/package-manager the following two warnings show up twice:
    Warning: Undefined array key "#parents" in Drupal\Core\Form\FormState->getError() (line 1176 of /var/www/html/repos/drupal/core/lib/Drupal/Core/Form/FormState.php).
    Drupal\Core\Form\FormState->getError() (Line: 166)
    Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 126)
    Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 126)
    Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 26)
    Drupal\Core\Form\FormErrorHandler->handleFormErrors() (Line: 200)
    Drupal\Core\Form\FormValidator->finalizeValidation() (Line: 119)
    Drupal\Core\Form\FormValidator->validateForm() (Line: 585)
    Drupal\Core\Form\FormBuilder->processForm() (Line: 321)
    Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
    Drupal\Core\Controller\FormController->getContentResult()
    call_user_func_array() (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 622)
    Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183)
    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: 53)
    Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
    Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 715)
    Drupal\Core\DrupalKernel->handle() (Line: 19)
    
    Warning: foreach() argument must be of type array|object, null given in Drupal\Core\Form\FormState->getError() (line 1176 of /var/www/html/repos/drupal/core/lib/Drupal/Core/Form/FormState.php).
    Drupal\Core\Form\FormState->getError() (Line: 166)
    Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 126)
    Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 126)
    Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 26)
    Drupal\Core\Form\FormErrorHandler->handleFormErrors() (Line: 200)
    Drupal\Core\Form\FormValidator->finalizeValidation() (Line: 119)
    Drupal\Core\Form\FormValidator->validateForm() (Line: 585)
    Drupal\Core\Form\FormBuilder->processForm() (Line: 321)
    Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
    Drupal\Core\Controller\FormController->getContentResult()
    call_user_func_array() (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 622)
    Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183)
    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: 53)
    Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
    Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 715)
    Drupal\Core\DrupalKernel->handle() (Line: 19)

    . The warning is also shown if you set the executable path and the settings override to /usr/bin/rsync but set a none existing executable path for composer like /usr/local/bin/composerer. So the warning seems only to show if either of the two executable paths is failing the form validation and one of the two fields is overridden in the settings.php.

The points we came up with:

  • We had a clear consensus in both meetings to recommend replacing the status message with a text box (solves point 1). Instead of auto-detecting and creating “noise” every time you visit admin/config/system/package-manager, simply display the path the two binaries are available at and auto-detected in a text box. Only return an error message, with the same accuracy as experienced on the reports page (solves point 2), if package manager is unable to auto-detect the binary/binaries. That way the user is always in control and gets the reassurance everything is in place and package manager is fully operational. At the same time it is clearly communicated that it is possible to change/override the executable path(s) - for that the two fields should not use any placeholder text and be empty instead.
  • In addition to adding the text box, we would also suggest to change the title of the field set underneath from Executable path to Custom executable path to further clarify that you are actually overriding the auto-detected defaults here and/or provide working executable paths in case one or both binaries are not available on the host.
  • We also had a clear consensus adding the check available for composer on admin/reports/status for rsync as well. We’ve already created a follow up issue for that: Add a runtime requirements test for rsync analogues to composer Active
  • The warning about the undefined error key in case settings are overridden via the settings.php should be fixed. (solves point 4)
  • In the end we wonder if one of the following scenarios we’ve observer and/or discussed have any security implications:
    • Adding a different binary, like for example node, for composer and/or rsync (see point 3)
    • Adding an executable path to a binary in the sites public or private folder, a location where users are able to upload files.

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

🇩🇪Germany rkoller Nürnberg, Germany

About the styling, i am not that savvy and familiar with Figma for being able to extract the exact specifics, but in case I interpret the current changes in the MR correctly then:

  • the padding between a breadcrumb item and a breadcrumb separator is 0.5rem in the MR while in the styleguide it is set to 0.75rem same as for Claro without the MR
  • the separator has the extend of 4px by 4px (width by height) while in the figma file it looks like 3px by 6px..

the increase in separator size got fixed. thanks! oh and the separator is still pointing upwards for RTL languages.

🇩🇪Germany rkoller Nürnberg, Germany

The reason why the pipeline is failing, the linter requires vertical-align come before border-inline-end, but vertical-align is at the end. and looking at https://git.drupalcode.org/project/drupal/-/merge_requests/12046/diffs that also notifies that the file was modified in the source and the target branch - needs someone with write access to resolve that. and i guess the MR could need a rebase in any way, due to the rename of the SandboxManagerBase class in package manager, that is requiring me to uninstall project browser when i try to checkout this MR for testing.

and i am in the process of reviewing the latest changes by @ll66382, will post another comment later.

🇩🇪Germany rkoller Nürnberg, Germany

your latest commit did the trick. tested with the combination of claro and navigation module, gin and the navigation module, and gin, gin_toolbar and the navigation module, all in safari, firefox, and edge on macos sequoia, and in every scenario the toolbar is shown now! thanks!

🇩🇪Germany rkoller Nürnberg, Germany

rkoller created an issue.

🇩🇪Germany rkoller Nürnberg, Germany

nice thank you! I've manually tested the MR12310, and i can confirm that the WSOD is gone when I am installing workspaces and workspaces_ui. also tested step wise uninstalling. everything works fine (only noticed a color contrast issue on the confirmation dialog for switching the workspace - will check existing issues later and in case file an follow up, after it is clearly out of the scope for this issue). i'll leave the issue at needs review since i am not able to review the actual code.

🇩🇪Germany rkoller Nürnberg, Germany

probably a duplicate of 🐛 InvalidComponentException when workspaces and workspaces ui is installed Active ... someone already opened a merge request there but that isn't fixing the issue (but the issue wasnt set to needs review yet so the MR is probably still a work in progress)'

🇩🇪Germany rkoller Nürnberg, Germany

and on a side note while going through the info files i've noticed too more detail probably out of the scope for this issue.

in pii_ui.info.yaml the second dependency is pii:pii_ui shouldnt that be just pii? and i guess it might make sense to also remove the requirement for drupal 9 in pii_ui.info.yaml and pii_api.info.yaml

🇩🇪Germany rkoller Nürnberg, Germany

After a brief discussion at todays monthly track meeting i'Ve changed the suggestion from Privacy to Privacy & Data protection per the recommendation from @jurgenhaas. According to him, the scope of the term privacy alone would have been too narrow.

the only detail, looking at the screenshot, the module names might need some adjustment as well, the three look sort of inconsistent.

🇩🇪Germany rkoller Nürnberg, Germany

and while writing up the summary in the previous comment i've also noticed one other detail, for consistency reasons it might be worth a thought moving the help text on the field upload group on the second step right before the field types instead of after them, after we've enabled help text on for example the selection list group right before the field types.

🇩🇪Germany rkoller Nürnberg, Germany

I’ve demo’ed the current state of the MR at the weekly lean coffee table call from the Drupal User Group Munich on Tuesday. For the record the attendees were @drubb, @franz-m, @jurgenhaas, and @martin mayer - most experienced longtime backend developers). All in all, the first impression was a good one, an improvement over the current state. They had a few comments on certain field types and groups we’ve taken a look at, in no particular order:

  • On the first step in the dialog modal, with the list of field types and groups, they commented that some field types might be placed more prominently, for example finding the link field type close to the end felt surprising - it was considered an important and often used field type, and with more additional field types listed it could get easily missed.
  • In regard to the description of the group for Formatted text “…with option to assign text editor”, the with “option to assign” sounded odd and off for none native english speakers.
  • Looking at the descriptions for the three field types within the Formatted text group their first remark was about the order of the descriptions.
    • They considered the detail about, if a field type is single row or multi row the most important detail, instead efficient storage was the first bullet point on the field type for short text.
    • Even though the group title is formatted text, short is explicitly mentioning that you have certain formatting options, the other two don’t have that mention. By explicitly naming it for one field type , the indirect assumption/conclusion is that it is not available on for the others.
    • Aside reconsidering the weighing of bullet points for the different field types by importance, they also suggested to reconsider the grouping of text related field types. Instead of going with Plain text and Formatted text, the suggestion was as already mentioned to make the single and multi row aspect more prominent by grouping by single row and multiple row.
  • In regard to the description “Maximum values depend on the system” on the integer number field type, attendees have asked, where would it be possible to find that kind of information if it is mentioned here? A direct pointer is sort of missing.
  • in the File upload group it was confusing to see the example of jpeg/png/webp/gif for the file as well as the image field type.
  • Within the Reference group, why the “Other” field type label? It was considered confusing and sort of ambiguous.
  • On a related note in general it would be helpful to provide clear and brief instructions for media/file/image when to use which
🇩🇪Germany rkoller Nürnberg, Germany

re #61 META: Provide locale (regional) formats framework for automated translation of non textual data Active i agree, that adding an option for following the defaults for the display language for the thousand and decimal marker is a good idea. the only thing i might object is moving that setting into the field settings (if i understand the proposed resolution correctly). the fields settings is mainly about storage in the database, but those settings are about how thousand markers and decimal markers are presented in the frontend to the user. that way i guess the better choice would be to keep those two select fields where they currently are on the manage display page and simply add those options to the select lists (and set them as default)?

re #62 META: Provide locale (regional) formats framework for automated translation of non textual data Active i completely agree and i wonder if aside adding those localization setting to /admin/config/regional/settings it would make sense or even be necessary to consider adding those settings to the user profile page as well. so a user is able to override the global localization settings.

🇩🇪Germany rkoller Nürnberg, Germany

added before screenshots for selection list and reference and also added before and after screenshots for the first step to illustrate the change there (removing the mention of the concept of field as well as the periods at the end as well as the general streamlining of the descriptions)

🇩🇪Germany rkoller Nürnberg, Germany

Added two screenshots so that they can be used in the to be updated issue summary. and i also adjusted the the issue title to the updated MR title, after the issue is not only about the descriptions but also about the labels. and the draft status got removed from the MR as well since all the changes are in and tests are passing thanks to @benjifisher

🇩🇪Germany rkoller Nürnberg, Germany

We haven't had the time for a full review in today's meeting ( 📌 Drupal Usability Meeting 2025-05-23 Active with @benjifisher, @rkoller, and @simohell), we just got to it during the last ten minutes. We will revisit the issue hopefully next week. But I am writing this comment cuz we stumbled across one noteworthy detail:

while demonstrating the MR, i wanted to demonstrate the detail about when the binary is not available (for that i've simply renamed rsync to rsyncer and composer to composerer within the webcontainer in ddev). when i got to the settings page from the reports page i think ive adjusted the names to the new naming in the executable path fields and saved - then the contradictory message showed together (see the screenshot). problem is when i was trying to reproduce the behavior the settings page behaved odd. the reports page was properly noting when composer was available and when not (when i'Ve renamed it). but on the settings page sometimes the binary was shown as autodetected even though it was renamed, at one time it seeemed even the path and file name got auto adjusted to the renamed file name. have to test some more tomorrow.

🇩🇪Germany rkoller Nürnberg, Germany

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2025-05-23 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.

In todays meeting we've discussed the remaining questions in #3370326-27: Refine field descriptions , the changes we've agreed on were already committed by @benjifisher and myself. To contain the scope we've created three followup issues:

the fourth point we touched today we've already opened another followup 📌 Add media reference to the same field type group as file and image Active , it is about the question where to place the media field type and the potential confusing of the naming of the group that is containing the file and image field type. and it has to be pointed out that 📌 [PP-1] Move label and machine name fields to the field config edit form Active is also important in the context with this issue.

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

🇩🇪Germany rkoller Nürnberg, Germany

should the default branch also be set to 2.1.x, at the moment it is still at 2.0.x?

🇩🇪Germany rkoller Nürnberg, Germany

it might be good to discuss 📌 Allow composer location to be configured via the UI Active ... Ran into that issue and after some initial testing #3511972-32: Allow Composer and rsync location to be configured via the UI and due to the fact that according to the issue it looks like there wasnt a pattern in core so far to edit the paths for certain binaries it might be a good idea to discuss that topic in a meeting on friday.

🇩🇪Germany rkoller Nürnberg, Germany

one question, i've checked out the MR and tested in DDEV. Both binaries, composer and rsync, are available within the ddev webcontainer at the default locations, and ssh'ed into the container to make sure that it is the case (*and it is, both are available).

The question i have, when the composer and rsync binaries are available and in default places (/usr/local/ & /usr/bin/), is package manager then fully functional with no further actions necessary?

At the moment the ui is sending mixed and a bit confusing messages in my testing. For one the path for composer has "auto-detected" appended. strictly speaking a placeholder text is about to inform the user what kind of data could be entered into that field not a status check. it imho can't be taken for granted that a placeholder text informs you about everything is all right, that the binary is available and composer will be available to package manager. and if i would take for granted that auto detect placeholder would inform me about the availability then i would assume that i have to enter a path for rsync because it is not available in the default location in /usr/bin/.

that placeholder approach leaves a quite a bit of uncertainty to me the user imho.

🇩🇪Germany rkoller Nürnberg, Germany

@acbramley i couldnt remember at all what i was referring to that i've outlined in point 2. i did some digging and first installed drupal 10.1 (thought the discussion were around that time) but the block types page havent had any help text - same as in 11.x. i've then installed drupal 10.0.0 et voilá, mystery solved:

So I guess most of the points in #34 are listed as fixed. the only point not labeled with fixed would be #34.5. so i guess i agree with you and @quiteone for closing this issue, and reassess the status quo soon and open up a new in case there is a need?

🇩🇪Germany rkoller Nürnberg, Germany

forgot to upload the mp4 i've mentioned.

🇩🇪Germany rkoller Nürnberg, Germany

thanks for the thorough and thoughtful comparison in #4 🐛 Breadcrumb separators are focusable Active between the approaches in Gin and Claro @batigolix. i've also discussed the topic with @the_g_bomb in the weekly drupal cms a11y call today. In addition to your assessment we've done a smanual test and simply checked out MR12046 which fixes the issue for Claro ( 🐛 Breadcrumb separators are focusable with the voiceover cursor and get also announced Active ) and then switched the theme to Gin with the MR still active for Claro - the breadcrumb separators remained focusable in Gin - q.e.d. to your assessment

After the meeting I've manually tested MR617 in Gin. The only minor nitpick to note is checking out the MR adds some slight padding (see padding.mp4) which is "probably" negligable (tested in safari in sequoia)? or would that slight padding shift be easily preventable?

Aside that everything looks good. The seperators are not focusable anymore and i've also cross checked if the separators are displayed correctly for RTL - all fine. excellent thank you!

🇩🇪Germany rkoller Nürnberg, Germany

I agree it is a disruptive change, but also a necessary one. There is actually already an issue where discussions and work happened over the course of the last few months: 📌 [PP1] Refine field descriptions Active . The issue has a broader scope since we are also refining the descriptions as well. I've quickly skimmed through the suggestions in the issue summary, and they are mostly in line with the suggestions in the current draft. Sole difference, with Use modal in add new field flow Active in and the field creation flow moving into a dialog modal, we wanted to make the dialog modal title more explicit for each step of the wizard. In consequence instead of something like Long formatted text, the title would be called "Formatted text" (and maybe something like "Step 1 of 3:"prefixed) and the available fields would be called "Short text", "Long text", "Long text with summary" - we also disliked parenthesis within the labels which puts a toll on readability.

A summary of the current state and the open to-dos can be found in #3370326-27: Refine field descriptions . The issue itself contains a lot of information and discussion, and most of the initial thought process is contained in https://docs.google.com/spreadsheets/d/1Lx7L40eRHotr5KQGn6au5WxQO3lFv19a... (there are also some more ideas for follow ups in the comments there).

If it would be ok with you @tstoeckler i would suggest, to invite you to join the discussion over in 📌 [PP1] Refine field descriptions Active (would be really valuable to get some additional feedback there - the recent discussions were solely around the regular attendees of the ux meetings) - simply to focus the effort in a single issue if that would be ok with you?

🇩🇪Germany rkoller Nürnberg, Germany

would it make sense to open up a follow up issue for comments and parts of documentation that are still using the term stage as a reminder? for core that is also handled in a follow up issue ( 📌 Update all comments and documentation in Package Manager to refer to "sandboxes" and related terminology Postponed ).

🇩🇪Germany rkoller Nürnberg, Germany

thanks for the MR @ll66382, i finally found the time today to test MR12046, a few observations:

  1. The MR fixes the basic problem the issue is about. i've tested in voiceover on macos sequoia and neither by tabbing nor by the voiceover cursor the seperators are announced anymore, excellent!
  2. If you compare the styling of the seperator between 11.x and the MR you will notice that the MR increases the separator size (compare in styling.mp4) - it seems like the separator is bigger now compared to the drupal design system
  3. The orientation of the separator in LTR is correct but for RTL it points up instead of to the left

And the separator looks fine in forced color dark mode - it is possible to test those in for example edge on macos (what i did) . In the screenshot you see the necessary settings to set the forced color dark mode in the rendering tab:

the other option is the page color setting in the accessibility section of the edge settings.

and there things look also fine in for example the desert theme

🇩🇪Germany rkoller Nürnberg, Germany

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2025-05-16 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, shaal, simohell, and worldlinemine.

We have based todays discussion on MR12089, in the following a list of details we’ve noticed:

  • Previous and Next are styled as buttons, but they are actually links.
  • In the current state each button only contains a button label, the user is required to read/skim the label, there are no visual cues like additional iconography.
  • Since the two buttons link to other pages, strictly speaking SC2.4.4 (Link purpose - in context) applies here - after the labels Previous and Next are missing some sort of context.
  • When you are on a details page and press the Previous or Next button repeatedly, you will notice that the column with the row labels is sort of uneasy and shifting constantly in width in between log messages - on some page the column gets even that narrow that the label wraps to a second line. From a readability point of view, you don’t have a fixed starting point for your eyes, you have to adjust on every page, which leads to an increased cognitive load if you are clicking through several error messages in a row (see column_width.mp4).
  • If you consider the use case of a site with thousands of recent log messages, there is the problem that the Previous and Next button are not respecting the applied filters. If you filter your log messages for example for a particular error message, and then click on one of the filter results, the filter scope is being reset - pressing Previous or Next on the details page will not bring you to the next/previous log message of your filter results, but to the next log message overall.

Aside the aforementioned points we’ve also asked ourselves if there is already a pattern in Core using such a Previous and Next pattern to step through instances - but we were unable to come up with one - there is only the remotely related Pager pattern used on Views (for example on /admin/content). In conclusion, these are the points the group reached a consensus on to suggest in scope for this issue:

  1. Instead of displaying links as buttons, style them as actions links (see Drupal Design System ) - that would be semantically more appropriate since each of the two point to a new page. Also re-add those single forward and backward step icons shown on the Figma link.
  2. Add some context to the button label by using the slightly more verbose Previous log message & Next log message as the label for the action links.
  3. Let the Previous and Next action links respect a set filter value. If you are for example filtering for the type “php” with a severity of “error” and you get four matches, log message number 25, 126, 134, 175, and you click on the detail view for number 126, the behavior on the detail page should be as follows:
    • There should be a button with the label Clear filter
    • Clicking Next, forwards you to log message number 134
    • Clicking Previous twice, forwards you to log message number 25
    • Clicking the Clear filter button, the button would get hidden, and in this case the previous button would get shown again (since in the scope of the filter results 25 is the first available message and Previous button hidden) - clicking the Previous button would forward you to log message number 24, clicking the Next button to log message 26, instead of going through the four available numbers of filter results.
    • Through the display of the Clear filter button it should be self explanatory that the current scope of displayed log messages are filter matches, while when the Clear filter button is not shown the scope is all the existing log messages. But ideally that could be validated in user tests, perhaps additional (visual) cues might be necessary.

In regard to the problem with the shifting column width, we’ve thought that is not “directly” related to this issue and should probably be moved to a followup issue instead.

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

🇩🇪Germany rkoller Nürnberg, Germany

Thank you @dev.drupal.ln & @batigolix for picking up and working on the issue, and apologies for the late reply, i am quite behind on going through all the a11y issues across issue queues that received attention recently. :( Todays a11y meeting was a timely and necessary reminder. So I went ahead and have taken a look at MR93 - a few observations:

1) The styling of the filter field needs some work - in general for Claro the design should orient to the Drupal Design System, for Gin I am not aware of any Figma file. Problem with the current state of the MR, the border has a color contrast of 1.2:1 rgb(235, 235, 235)/rgb(255, 255, 255) in Claro and Gin which is not in line with WCAG 2.2 SC1.4.11.
claro

gin

For Claro the border color should be var(--input-border-color) and on hover var(--input--hover-border-color), the border should be solid and the size var(--input-border-size). The filter field in Claro has a height of 48px (the min-height property for that is calc(((var(--input-padding-vertical) + var(--input-border-size)) * 2) + var(--input-line-height))).
For Gin the border color should be var(--gin-border-color-form-element) and on hover var(--gin-color-text) (checked the filter field on admin/content for the variable on hover), the border should be solid, the size is 1px, and the corners should be rounded.
The filter field in Gin has a height of 40px (the min-height property has calc(var(--input-padding-vertical) * 2 + var(--input-line-height)). There might be more styling details for Claro and Gin to mind, the variables provided for Gin are only for the light mode, they might vary for the dark mode.

2) The input field is missing a label (WCAG 2.2 SC 3.3.2)

3) The filter field is missing an outline when being in focus (WCAG 2.2. SC 2.4.7 & SC 1.4.11)

4) If you are entering a term to filter for into the filter field the list underneath is immediately updated accordingly. This would happen to non-sighted users unexpectedly, which is not in line with WCAG 2.2. 3.2.2. In project browser ( 🐛 Improve the accessibility of the search field Active ) we’ve applied the following approach, we’ve added a search button (for the token module the button could be labeled Filter analogous), and the user can either press enter within the filter field or tab to the filter button and then filter. That way the user is in control.

5) If a user wants to clear the input of the filter field they have to use the back space button. If someone assumes pressing the ESC key would clear the field that closes the token dialog instead. It might be worth to consider to again apply the same approach that is used for project browser and currently proposed for the field ui Add a clear button to the fields ui Active , that way the behavior would become consistent across Core.

6)

If you click on a token in the list, it will populate the last field that had focus. With this change, the last field will become the new token filter field.

this behavior is problematic. That way a keyboard user has no way to accomplish their key task, inserting a token into a field in the background of the token dialog.

7) the hierarchical list becomes hard to read after you have filtered for a string. the treetable is already hard to comprehend in the unfiltered form, but if you enter something into the filter field (for example user), you dont have a top level element as the first element but one that is from the second or third level (the exact level is impossible to assess)

8) Looking at the treeTable problem, that reminds me to the more fundamental question, about how to proceed - there is 📌 Token UI 2.0 Active about Token UI 2.0. It is the question what would be the best approach for the Token UI going forward.

  • Should there be a fundamentally different UI and all other issues postponed until a consensus is reached on Token UI 2.0?
  • Should there be a fundamentally different UI but until a consensus is reached there should be smaller issues like this one improving the status quo.
  • Or should we proceed in the current direction and iterate in small steps with issues like this?

Uncertain what the best approach might be. In regard of the ideation for Token UI 2.0, I’ve suggested in today’s a11y meeting to raise the topic in one of the future weekly UX meeting on friday.

🇩🇪Germany rkoller Nürnberg, Germany

Followup for #68 🐛 Claro Shortcuts star fails WCAG Use of Color and Non-text contrast Needs work . The reason why the two star states were barely visible was my usage of the "enable automatic dark mode" setting in the edge rendering tab. Even though it uses also prefers-color-scheme: dark it is still behaving off. @kentr and I had a discussion and some further digging over in: https://drupal.slack.com/archives/C2ANFUGGG/p1746978093222529

We still dont know the exact reason why the "enable automatic dark mode" setting is behaving differently output wise, but the following pen @kentr created illustrates the behaviro pretty well between the two approaches: https://codepen.io/kentr/full/XJJyerz (...and narrows down the cause to the color-scheme property as the key factor) . enable automatic dark mode on the left and prefers-color-scheme: dark on the right.

the color-scheme property is not used in core so far (only gin uses it in a few places). therefore the suggestion for the way forward would be two fold, to open up a followup issue adding something like

:root {
  color-scheme: light dark;
}

to core in general as suggested in the almanac on css tricks ( https://css-tricks.com/almanac/properties/c/color-scheme/). that way all elements of the page/site get opted into both color schemes making those elements theme-able in the forced color mode. and for testing avoid "enable automatic dark mode" nevertheless, instead use the following two settings (here in microsoft edge)


And the star looks good with prefers-color-scheme: dark as well as all the available page color themes available. The only small nitpick i might have and i just noticed last night, in forced color mode the unchecked star border is yellow and the checked star is filled yellow. while in none forced color mode the checked star is blue which is in line with the forced color mode, since the color blue stands for links. but the unchecked star border is black / gray.

so i wonder shouldnt the star in the unchecked state also have a blue border for consistency and semantics?

🇩🇪Germany rkoller Nürnberg, Germany

carrying over from last week: 📌 Disallow dangerous filenames e.g. command injection characters Active
and this one came up in the contribute channel: Add next and previous buttons to database log entry pages Active

🇩🇪Germany rkoller Nürnberg, Germany

hm that is odd, i've tested MR11498 in Edge with high contrast mode on macos sequoia, the star in the screenshot in #58 🐛 Claro Shortcuts star fails WCAG Use of Color and Non-text contrast Needs work looks okay to me, but in my own testing the star is barely visible in each of the two states.

unchecked:

checked:

🇩🇪Germany rkoller Nürnberg, Germany

left a comment on #3223022-40: Open the "discard changes" dialog in the off-canvas tray , feel free to add any detail i might have missed.

🇩🇪Germany rkoller Nürnberg, Germany

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2025-05-09 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, @simohell, and @worldlinemine.

There was a clear consensus within the group that using a dialog modal instead of forwarding to a dedicated confirmation page is a desirable improvement. Forwarding to a confirmation page is only preferable in case additional tasks/steps are necessary to take, but for an informational confirmation step, staying within the context of the page the confirmation is required on, a dialog modal is the better choice. But we found a few details that need further work:

1) The title of the dialog modal is getting truncated. In Umami you only see: Discard cha…

In Olivero you see even less Discar…:

If you replace the h1 wrapping the title of the dialog modal with a span with devtools, the title becomes fully legible:

Some context, until a few weeks ago, a span element was used for wrapping the title of dialog (modals) in jQuery UI in Drupal, which was semantically wrong and a problem for screenreader users. 📌 Update to jQuery 1.14.1 and use the newly added option for dialog modal headings Active solved that problem, for dialogs the span got replaced by h2s, and for dialog modals by h1s. In Claro that change caused no visual changes, but I was unaware that dialogs and dialog modals would be available for frontend themes like olivero and umami as well, and therefore have not tested for that detail, which lead to results seen in the screenshots. :/

It would be necessary to open up issues in the Olivero and the Umami issue queue for fixing the truncation of the title. Aside the truncation there are a few more problems with those dialog (modals). The close button has a too small target size (16x16) for Olivero and Umami, the required minimum target size to meet WCAG2.2 SC2.5.8 would be at least 24x24 pixels. Then in Umami the confirm button has a too low color contrast and is not meeting the required color contrast of at least 3:1 to comply with WCAG2.2 SC1.4.11. Also in the context of WCAG2.2 SC1.4.11 the dialog component is not easily recognizable nor distinguishable from the background - in Claro you have at least a visually more clearly darkened background and the dialog modal sort of “stands out” from the background. The last point to note is in regard to the general layout, the dialog modals in Umami as well as Olivero are missing some padding, the content and the buttons feel cramped into the dialog component.

2) At the moment the confirmation dialog is only available for layout overrides on nodes, the default layout for a content type still employs confirmation pages (as noted in #39). There was the clear consensus to make things consistent and use confirmation dialogs on default content types as well. If that change could be done easily it would be good to fix that within the scope for this issue, if it would be a more complicated endeavor it could also be moved to a follow up. And on a side note, by switch to the confirmation dialog on the default layout for a content type, those overly conversational verbose titles could be avoided, that are currently used on the confirmation page (it uses Are you sure you want to discard your layout changes? as the title). Titles should always provide immediate clarity of context and action to be taken in a concise manner - for single task titles like that, with imperative verb phrases.

3) The issue title needs to be updated. Currently it refers to opening the dialog in the off-canvas tray, but with the MR the confirmation dialog opens in a dialog modal instead. The user interface changes section in the issue summary also needs before and after screenshots. Therefore I am adding the Needs issue summary update tag.

I set the status to Needs work for now, until it is decided what the preferable order for fixing those three points is. One thing to note, in regard to point 1, the suggestion/idea during the meeting was, to postpone this issue until the to be created issue(s) in the theme queues are being fixed - question also is should there be several tightly scoped issues for truncation, target size, color contrast, and padding per theme or should some of them be combined? And it should also be checked if Olivero for Drupal CMS would require additional fixes after Olivero is being fixed.

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

🇩🇪Germany rkoller Nürnberg, Germany

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)

🇩🇪Germany rkoller Nürnberg, Germany

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.

🇩🇪Germany rkoller Nürnberg, Germany

the update to the issue summary helped, i am able to reproduce and test now. thanks a lot for the updated instructions @grevil!

🇩🇪Germany rkoller Nürnberg, Germany

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

🇩🇪Germany rkoller Nürnberg, Germany

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

🇩🇪Germany rkoller Nürnberg, Germany

Thanks to all the candidates for their submissions. My votes go to:

  • Marine Gandy (mupsi)
  • Nico Grienauer (grienauer)
🇩🇪Germany rkoller Nürnberg, Germany

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

🇩🇪Germany rkoller Nürnberg, Germany

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.

🇩🇪Germany rkoller Nürnberg, Germany

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. :/

🇩🇪Germany rkoller Nürnberg, Germany

rkoller created an issue.

🇩🇪Germany rkoller Nürnberg, Germany

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

🇩🇪Germany rkoller Nürnberg, Germany

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

🇩🇪Germany rkoller Nürnberg, Germany

i've assigned the issue to myself and already created a MR.. Will work on an initial draft the next few days...

Production build 0.71.5 2024