link to the combined comment from last weeks meeting and yesterdays posted by @benjifisher at #3537624-28: Deleting filter formats may result in data loss → . thanks benji!
and @benjifisher left a combined comment for this meeting and the meeting on the 31st of october in #3537624-28: Deleting filter formats may result in data loss → . thanks benji!
after checking out MR5993, i run ddev drush cr i get the following error:
PHP Fatal error: Uncaught Error: Class "Doctrine\Common\Annotations\TokenParser" not found in /var/www/html/repos/drupal/core/lib/Drupal/Component/Annotation/Doctrine/StaticReflectionParser.php:192
Stack trace:
#0 /var/www/html/repos/drupal/core/lib/Drupal/Component/Annotation/Doctrine/StaticReflectionParser.php(329): Drupal\Component\Annotation\Doctrine\StaticReflectionParser->parse()
#1 /var/www/html/repos/drupal/core/lib/Drupal/Core/Hook/HookCollectorPass.php(507): Drupal\Component\Annotation\Doctrine\StaticReflectionParser->getMethodAttributes()
#2 /var/www/html/repos/drupal/core/lib/Drupal/Core/Hook/HookCollectorPass.php(402): Drupal\Core\Hook\HookCollectorPass->collectModuleHookImplementations()
#3 /var/www/html/repos/drupal/core/lib/Drupal/Core/Hook/HookCollectorPass.php(138): Drupal\Core\Hook\HookCollectorPass::collectAllHookImplementations()
#4 /var/www/html/vendor/symfony/dependency-injection/Compiler/Compiler.php(73): Drupal\Core\Hook\HookCollectorPass->process()
#5 /var/www/html/vendor/symfony/dependency-injection/ContainerBuilder.php(826): Symfony\Component\DependencyInjection\Compiler\Compiler->compile()
#6 /var/www/html/repos/drupal/core/lib/Drupal/Core/DrupalKernel.php(1397): Symfony\Component\DependencyInjection\ContainerBuilder->compile()
#7 /var/www/html/repos/drupal/core/lib/Drupal/Core/DrupalKernel.php(916): Drupal\Core\DrupalKernel->compileContainer()
#8 /var/www/html/repos/drupal/core/lib/Drupal/Core/DrupalKernel.php(507): Drupal\Core\DrupalKernel->initializeContainer()
#9 /var/www/html/drush/Commands/core_development/DevelopmentProjectCommands.php(90): Drupal\Core\DrupalKernel->boot()
#10 /var/www/html/drush/Commands/core_development/DevelopmentProjectCommands.php(68): Drush\Commands\core_development\DevelopmentProjectCommands->drupal_rebuild()
#11 [internal function]: Drush\Commands\core_development\DevelopmentProjectCommands->rebuild()
#12 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(276): call_user_func_array()
#13 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback()
#14 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(175): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter()
#15 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(387): Consolidation\AnnotatedCommand\CommandProcessor->process()
#16 /var/www/html/vendor/symfony/console/Command/Command.php(318): Consolidation\AnnotatedCommand\AnnotatedCommand->execute()
#17 /var/www/html/vendor/symfony/console/Application.php(1073): Symfony\Component\Console\Command\Command->run()
#18 /var/www/html/vendor/symfony/console/Application.php(356): Symfony\Component\Console\Application->doRunCommand()
#19 /var/www/html/vendor/symfony/console/Application.php(195): Symfony\Component\Console\Application->doRun()
#20 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(110): Symfony\Component\Console\Application->run()
#21 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun()
#22 /var/www/html/vendor/drush/drush/drush.php(140): Drush\Runtime\Runtime->run()
#23 /var/www/html/vendor/bin/drush.php(119): include('...')
#24 {main}
thrown in /var/www/html/repos/drupal/core/lib/Drupal/Component/Annotation/Doctrine/StaticReflectionParser.php on line 192
Fatal error: Uncaught Error: Class "Doctrine\Common\Annotations\TokenParser" not found in /var/www/html/repos/drupal/core/lib/Drupal/Component/Annotation/Doctrine/StaticReflectionParser.php:192
Stack trace:
#0 /var/www/html/repos/drupal/core/lib/Drupal/Component/Annotation/Doctrine/StaticReflectionParser.php(329): Drupal\Component\Annotation\Doctrine\StaticReflectionParser->parse()
#1 /var/www/html/repos/drupal/core/lib/Drupal/Core/Hook/HookCollectorPass.php(507): Drupal\Component\Annotation\Doctrine\StaticReflectionParser->getMethodAttributes()
#2 /var/www/html/repos/drupal/core/lib/Drupal/Core/Hook/HookCollectorPass.php(402): Drupal\Core\Hook\HookCollectorPass->collectModuleHookImplementations()
#3 /var/www/html/repos/drupal/core/lib/Drupal/Core/Hook/HookCollectorPass.php(138): Drupal\Core\Hook\HookCollectorPass::collectAllHookImplementations()
#4 /var/www/html/vendor/symfony/dependency-injection/Compiler/Compiler.php(73): Drupal\Core\Hook\HookCollectorPass->process()
#5 /var/www/html/vendor/symfony/dependency-injection/ContainerBuilder.php(826): Symfony\Component\DependencyInjection\Compiler\Compiler->compile()
#6 /var/www/html/repos/drupal/core/lib/Drupal/Core/DrupalKernel.php(1397): Symfony\Component\DependencyInjection\ContainerBuilder->compile()
#7 /var/www/html/repos/drupal/core/lib/Drupal/Core/DrupalKernel.php(916): Drupal\Core\DrupalKernel->compileContainer()
#8 /var/www/html/repos/drupal/core/lib/Drupal/Core/DrupalKernel.php(507): Drupal\Core\DrupalKernel->initializeContainer()
#9 /var/www/html/drush/Commands/core_development/DevelopmentProjectCommands.php(90): Drupal\Core\DrupalKernel->boot()
#10 /var/www/html/drush/Commands/core_development/DevelopmentProjectCommands.php(68): Drush\Commands\core_development\DevelopmentProjectCommands->drupal_rebuild()
#11 [internal function]: Drush\Commands\core_development\DevelopmentProjectCommands->rebuild()
#12 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(276): call_user_func_array()
#13 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback()
#14 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(175): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter()
#15 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(387): Consolidation\AnnotatedCommand\CommandProcessor->process()
#16 /var/www/html/vendor/symfony/console/Command/Command.php(318): Consolidation\AnnotatedCommand\AnnotatedCommand->execute()
#17 /var/www/html/vendor/symfony/console/Application.php(1073): Symfony\Component\Console\Command\Command->run()
#18 /var/www/html/vendor/symfony/console/Application.php(356): Symfony\Component\Console\Application->doRunCommand()
#19 /var/www/html/vendor/symfony/console/Application.php(195): Symfony\Component\Console\Application->doRun()
#20 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(110): Symfony\Component\Console\Application->run()
#21 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun()
#22 /var/www/html/vendor/drush/drush/drush.php(140): Drush\Runtime\Runtime->run()
#23 /var/www/html/vendor/bin/drush.php(119): include('...')
#24 {main}
thrown in /var/www/html/repos/drupal/core/lib/Drupal/Component/Annotation/Doctrine/StaticReflectionParser.php on line 192
[warning] Drush command terminated abnormally.
Failed to run drush cr: exit status 1
... on 11.x i dont get that error. i am on php 8.3 with ddev.
i havent had the chance to test another time. but looking at the commits all the changes since i last looked were about code related issues and tests. so i most likely havent anything else to add, and in regard to your question in #82 🐛 Drupal Current theme condition plugin should provide an option to select all themes Needs review i have not enough time and focus at the moment. currently completing a course which is heavy on assignment. i have currently dialed back everything in regard to contributions, the friday's ux meeting is the sole thing i do in recent weeks. i wouldnt be angry if anyone else beats me to opening those follow ups or otherwise i will do that after that course is over at the end of november.
yep i agree updating the existing CR might be good idea. and in regard to how to make developers in contrib aware of that change @jurgenhaas had the idea in todays lean coffee table to add tests to the drupal CI process. how many of those rules will be testable will be the question but things like to avoid periods at the end of the description of a field type or avoiding the term field in the description should be do able. and to sum up a brief follow up discussion with benji over in a thread in the ux channel on slack, he said phpcs might be the test to go with (the other suggestion from juergen was phpstan - he was uncertain about the right pick) and it would imply to add a new rule to the drupal coding standards. so it would definitely mean adding those tests in a follow up issue. and then to start with a test for "common" entity types (listed along with nodes, media, taxonomy terms) that do not have descriptions. other rules might be harder to test.
I left a suggestion on the MR in regard the wording. keeping the string in line with the one for the language plugin is a good call. i wasnt aware that you are able to unhide that plugin as well.
in regard of the summary, yep we noticed the missing summary on the vocabulary tab as well. we would open a followup issue for that in case no one else beats us to it. but for the current theme we just thought if it would be easy to add then do the adding in the scope of this issue for the sake of consistency, otherwise also as a follow up.
in regard of the order and the radio buttons i agree. that is what i tried to express. in contrast to the summary those two bits introduce sort of "new" approaches so it would hold back the issue.
As mentioned before, I guess it would be a reasonable step to open up another follow up issue about creating some sort of style guide for the micro copy used on field types and field groups - basically the rules we've applied to the strings in this issue. otherwise the micro copy for any contrib field type will stand out in the interface. one individual example i ran into i've already filed an issue for is 📌 Make the mermaid diagram field type presentation consistent by adding a description and an icon Active . to prevent that it might be good to a have central document about the general rules how those strings should look like that is easy to look up for maintainers. would it make sense to add a change record for this issue pointing to the to be created style guide?
removed one slash too much from the path for the second picture... fixed now.
updated the three after screenshots in the issue summary to reflect the latest changes
We've revisited 🐛 Drupal Current theme condition plugin should provide an option to select all themes Needs review in todays meeting. looks like we havent managed to comment on the issue back then. but during our revisit we managed navigate around our stumbling blocks from back then that prevent us from a review and we wanted to ask for clarification on the issue. comment for todays review is #2783897-76: Current theme condition plugin should provide an option to match all themes and introduce checkboxes → . the comment was the last missing task for this issue. marking as fixed since credits were granted and transcript and recording are in place.
Usability review
We discussed this issue at 📌 Drupal Usability Meeting 2025-10-24 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 @the_g_bomb
At first apologies, I remember we've tried to review this issue back in 📌 Drupal Usability Meeting 2025-05-09 Active but had issues reproducing and getting to the gist of the problem. We agreed on asking in here for clarification. But somehow no one of us has actually commented on the issue back then. :( Sorry again. :(
About two weeks ago I've noticed in my inbox that I got referenced in comment #70 by @grevil. I am "a bit" swamped in recent weeks so just got to revisiting the issue in todays meeting. We were able to navigate around the previous stumbling block and were able to review it. A few observations:
- The description is referring to a boolean value which is kind of taxing for less technical users and should probably be avoided . And in combination with the label of the negate checkbox it makes things hard to comprehend. the cognitive load is rather high.
- In general the negate checkbox is a taxing pattern. We have raised our concerns in related issues in the context of visibility plugins using that negate checkbox before. From a clarity perspective the radio button pattern used for the pages plugin is the preferable choice. (I have to search for the issue - i will add to this comment later on) .
- In general the preferable reading/processing order on those conditional plugins would be first what you want to do (aka the negate checkbox or the radio button pattern) and after that "what you want to apply it to"
- the tab is missing a menu link summary other tabs like pages, response status, roles, and content type have.
So in summary we would have the following recommendations:
- add the menu link summary for consistency reasons.
- IF it would be technical possible try to use a radio button component instead of the negate checkbox. The two labels for the radio buttons could simply be
Show for the selected themesandHide for the selected themesand move the radio button before the list of checkboxes for the available themes. The description would become obsolete cuz the radio button labels would be self explanatory and clear from our perspective. In case a radio button would be impossible for this plugin or that step should be moved to a follow up then replace the descriptionSelect the themes this condition should evaluate TRUE on.withShow for the selected themesfor now
attendees at todays meeting were: @rkoller, @simohell, @the_g_bomb
push another small update to the MR. during the meeting on friday ( 📌 Drupal Usability Meeting 2025-10-17 Active ) we've changed one bullet point on the description of the datetime field. we've noticed some more inconsistencies with the descriptions of the other date and time field types. after a brief discussion on slack with @benjifisher we agreed on the folllowing changes:
adjust the second bullet point of datetime range to the new pattern used on datetime and change
Two Datetime values: start and end
to Choose either date and time, date only, or all day which are the settings on the field settings page.
while Input requires: date, time on the timestamp field type is simply wrong. the description of the input field for the timestamp states "Leave blank to use the time of form submission.". So the input is not required and we simply striked the bullet point
commented on the issue: #3370326-92: Refine labels and descriptions for field types → and also had the time to apply the changes we've discussed today. MR is updated and issue is NR.
We discussed this issue at 📌 Drupal Usability Meeting 2025-10-17 Active . That issue will have a link to the recording of the meeting. For reference the attendees at the meeting were @benjifisher and @rkoller.
We've discussed the second batch of comments raised by @dww. The first batch was discussed during
📌
Drupal Usability Meeting 2025-10-10
Active
last week. The only noteworthy detail from todays discussion was that we've kept A reference to a(n) @item as is due to the fact that it is an extremely rare edge case most users will never run into and any effort trying t rephrase that string made things even worse. :/ I'll set the issue back to needs review
added the link to the recording to the issue summary.
added the trimmed recording to the issue summary
added a link to the trimmed video of the recording to the issue summary
added a trimmed version of the recording.
the video was already trimmed but not published. published now and added the link to the issue summary
added the trimmed video. only the transcript is missing then the issue can be set to fixed.
added the trimmed video. attendees were @benjifisher, @rkoller, and @simhohell. only credits and transcript are missing.
added the trimmed video. and the attendees of the meeting were @benjifisher, @rkoller, and @simhell. the things left to do on the issue are uploading the transcript and adding credits. setting it to NW
Added the trimmed video and simo commented on the first issue of the day ( 🐛 Advanced settings in Views edit - preserve state or leave expanded? Needs work ). on the other issue we havent left a video yet cuz we've touched the second issue only in the last few minutes of the meeting, but we've picked up the issue in 📌 Drupal Usability Meeting 2025-09-26 Active and finalized the review. So the issue is only missing the transcript and crediting. Attendees were @benjifisher, @rkoller, and @simohell
thanks for the transcript and providing credits benji and i've added the link to the recording.
thanks for the transcript and for adding the credits. i've trimmed the video on youtube and set it public. updated the issue summary and will set the issue to fixed
hm that is odd. benji was just adding the credits for the attendees of an ux meeting three weeks ago. I am following that issue cuz i've created it and received an empty notification which usually is an indication that either the IS got updated or credits got added. checking the credit record on https://new.drupal.org/contribution-record/11424226 confirmed the latter. but i havent received an email that i got credit what this issue here was about. or havent i received a credit notification because i was already following the issue and created it?
I was the only attendee in today's meeting, left the zoom call after 20 minutes waiting. Closing the issue as won't fix.
and in regard to credit the attendees at the meeting were @simohell and myself, @rkoller
Left a comment on #2704931-42: Provide Drupal root path on file system admin form →
Usability review
We discussed this issue at 📌 Drupal Usability Meeting 2025-09-26 Active . That issue will have a link to a recording of the meeting. For the record, the attendees at the usability meeting were @rkoller, and @simohell
If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.
It has to be noted upfront that we usually don't do a review with two attendees only. Initially we wanted to call off the meeting but then ended up talking about that issue anyway, and due to the fact that we've started a review in 📌 Drupal Usability Meeting 2025-08-29 Active together with benji before, we've continued our conversation and exploration.
In general we were in line with the changes this MR introduces. We've considered it useful showing the absolute paths on admin/config/media/file-system. It provides context and situational awareness to the user. In regard to clarity there are two minor nitpicks:
- The description for the application root directory states
The top level directory of the Drupal installation. Relative paths shown below are relative to this location.. The second sentence is sort of confusing. With the MR applied the page is showing only absolute paths while this second sentence is referring to relative paths; that is potentially confusing and might make the user search for relative paths on the config page (at least I've did). Then the second sentence also uses directional language with "...shown below..." which is not adiseable. So we've wondered why not simply strike the second sentence? And in regard to the firstThe top level directory of the Drupal installation.we've wondered if it would make sense to add a detail thatwebis the docroot? Not a recommendation more of a question, intention was to help less technical users to distinguish what is the root path and what the docroot? We would have come up with a suggestion but none of us is a native speaker, and it is always preferable to have at least one native speaker attending for providing a suggestion or recommendation for anything micro copy related. We could revisit that in next weeks meeting with hopefully native speakers attending, as long as no one beats us to it with a suggestion on the issue here or argues against such an addition. - And we agree with
#41
✨
Provide Drupal root path on file system admin form
Needs work
and consider that sort of a problem, but from our perspective probably out of the scope for this issue and more suitable for a follow up? With the MR applied we've quickly tested the following scenario: We've created a folder
playgroundin the root of the project. Then added$settings['file_private_path'] = '/var/www/html/playground';to the settings.php resulting in/var/www/html/playgroundon the file system page. We've then changed the line in the settings.php to$settings['file_private_path'] = '../playground';; the output remained the same, without any indication that a relative value is being used in the settings.php file. And that it is possible to actually use a relative value in the settings.php file is also counter the comment in the settings.php file which states "A local file system path where private files will be stored. This directory must be absolute, outside of the Drupal installation directory and not accessible over the web.". So having at least an indication of sorts that the output value is based on a relative value on the settings.php might help the user in the context of debugging and/or context awareness. But as we've said from our point of view this point is out of the scope for this issue.
In todays meeting there was only @simohell and myself. initially we wanted to skip the meeting because a critical mass of at least three wasn't reached but we then ended up "quickly" revisiting ✨ Provide Drupal root path on file system admin form Needs work , an issue we've discussed before together with benji in 📌 Drupal Usability Meeting 2025-08-29 Active , and ended up in a full proper meeting anyway.
aside the scope, another potential problem that i see with the four alternatives listed in the issue summary; i just did a quick test navigating with the keyboard trying to open and go through a demo tour with voiceover active for each of the examples and none of them had an acceptable quality of the interaction and aural interface out of the box compared to shepherd unfortunately :(
on the #ux channel two issues were raised:
by @smustgrave
✨
Provide Drupal root path on file system admin form
Needs work
by @acbramley
📌
Consider showing all revisions on the revision overview
Active
left a comment on the issue: #2804387-96: Inform users that the maintenance mode message is not displayed on cached pages → . In regard to the remaining tasks:
- we have to figure out a new process for the time being, since benji handled downloading the video and transcript from zoom and then upload the video to his youtube account.
- and in regard to crediting, the attendees at the friday meeting were @jannafiester, @joannajackson, @rkoller, and @simohell . none of us have the permissions to grant the credit on this issue.
We discussed this issue at 📌 Drupal Usability Meeting 2025-09-19 Active . That issue will have a link to a recording of the meeting. For the record, the attendees at the usability meeting were @jannafiester, @joannajackson, @rkoller, and @simohell
If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.
The removal of the status message based on the rational provided in #92 🐛 Inform users that the maintenance mode message is not displayed on cached pages Needs review sounded like a reasonable step for us if that is fixing the outlined problems including the one in #91 🐛 Inform users that the maintenance mode message is not displayed on cached pages Needs review . The change of the micro copy to the new description is ok, except it is a bit unspecific by referring only to "message"
But the way bigger problem with this change from our point of view is that the description is very very had to notice. We have skimmed through comment #92 🐛 Inform users that the maintenance mode message is not displayed on cached pages Needs review at the beginning of the meeting and then took a look at the maintenance page on a test site with MR9015. For context two of us attended the review about one year ago only vaguely remembering the details at best, the other two looked at the issue for the very first time, so all of us looked at this issue with some fresh pairs of eyes. We've searched for an extended period of time for the introduced changes, at the end we even had two windows open, one without the MR and one with the MR, and we were still unable to notice that the description of the text field contained the change (reminded me a bit about the video of that selective attention test https://www.youtube.com/watch?v=vJG698U2Mvo). as a last resort before asking over here on the issue we've taken a look at the merge request; copied the string for the description to the clipboard and then searched on the maintenance page for that string - that way we were able to finally find the change. The reason why we haven't noticed the change was probably due to a combination of reasons; the description is at the very end of the page, it is a description for a text field, and the font size is very tiny for an important detail like that - the information has a very low information scent. If you take into consideration that sighted users usually don't read a page but skim most of the time, the odds are rather low that they will notice the information. but it might not be solely challenging for sighted users, screenreader users might also jump off early on right after the text field label "message to display when in maintenance mode" was announced. they might think "ahhh this is the field for the message displayed in maintenance mode i can jump off now". so either way, the descriptions might get missed by sighted users and/or screenreader users.
Probably in part out of scope for this issue, but our discussion shifted towards the general order of components on the maintenance page when we've tried to come up with a suggestion how to communicate the information introduced in this MR.
Currently we have, first the help text, then the checkbox to put the site into the maintenance mode, and finally the textfield. From our point of view the more logical order would be a) help text b) textfield c) checkbox , that way you consciously pass and validate the maintenance mode message before you set the site into the actual maintenance mode - currently you have to jump back and forth in regard to the reading order.
With a change of the component order the microcopy might also be rearranged. the description currently used on the checkbox is strictly speaking about the general behavior of the maintenance mode, something a help text is about, while the description introduced with this MR is about a CTA for the admin to clear the cache after the site is set into the maintenance mode.
An initial suggestion for the help text might be as follows - moving the string currently used on the checkbox into the help text:
Use maintenance mode when making major updates, particularly if the updates could disrupt visitors or the update process. Examples include upgrading, importing or exporting content, modifying a theme, modifying content types, and making backups.
Only users with the "Use the site in maintenance mode" permission will be able to access the site. Authorized users can log in directly via the user login page. Visitors will only see the maintenance mode message. It will not show on already cached pages without clearing the page cache.the description for the checkbox would be a more specific variant of the string currently used on text field description of this MR:
The maintenance mode message will not show on already cached pages without clearing the page cache.
The rearrangement and rephrasing of the help text is just an initial draft to convey the general idea, we haven't had enough time left during the meeting to hone the micro copy. But the basic idea was to move the description of the checkbox into the help text. The added part outlines the general behavior of the maintenance mode in regard to permissions, first about who can login and then who can view what. The last sentence of the "who can view" is sort of redundant to the description of the checkbox but that was a deliberate choice. It is an important detail and by having it be addressed twice, the odds are getting higher that the detail wont go unnoticed, after there is no proper way to highlight that detail visually more prominently aside using a status message.
How to proceed? if others agree to the general direction we've suggested, we could revisit the issue in the next meeting and do some more word smithing, in case no one else beats us to it. And talking of general direction we were also uncertain how to chop the proposed changes up, cuz the reordering of components is most likely out of the scope for this issue?
i am closing this issue, after i was the only attendee. I've already created the issue for next week ( 📌 Drupal Usability Meeting 2025-09-19 Active )
and copying over the issue that got raised by @poker10 in the #ux channel on the drupal slack: 🐛 Inform users that the maintenance mode message is not displayed on cached pages Needs review
added links to the issue summary for the two meetings where we've summarized the problem space of the whole media module topic
looks like a duplicate of 📌 Drupal Usability Meeting 2025-09-12 Active . closing this one
@poker10 raised 🐛 Inform users that the maintenance mode message is not displayed on cached pages Needs review in the #ux channel on the drupal slack
hm the reason i've opened the two issues as follow ups was to not hold back ✨ Open the "discard changes" dialog in the off-canvas tray Active . and strictly speaking this is not an issue ✨ Open the "discard changes" dialog in the off-canvas tray Active is introducing (the MR only introduces changes to php), instead it is simply using the dialog modal styling of demo:_umami in this case.
there is one detail that makes the naming potentially complicated. we should try that the name used by sighted users to address the sidebar and top bar is in line with the one used in the aural interface. otherwise you get a tower of babel like problem, that screenreader users would search for "administrative sidebar" or "administrative top bar" in the docs with no luck or address those names in conversations for example on the drupal slack. plus navigation sidebar wouldnt necessarily ring a bell for screenreader users directly . and at the same time it is necessary to be mindful about the point @mherchel raised in #3452724-63: Navigation side bar and top bar should have appropriate aria labels →
i agree that "Contextual administrative operations" would be a bit too verbose. i wonder, maybe positional language like sidebar and top bar is even unnecessary? how about something like admin navigation for the sidebar and context bar for the top bar? that way the label for the sidebar would be just admin (becoming admin navigation) and it wouldn't collide with the front end theme and context bar would be announced with "context bar, complementary"
rkoller → created an issue.
i've noticed one problem, on the second step within the reference field group the description for the user looks broken (in EntityReferenceItem.php):
wont be necessary if the drupalSettings approach in 📌 [PP-1] change from the global configuration object to the data-skipto approach Postponed is used
i've managed to get a more drupal like approach working locally. i've added two small snippets to the skipto.js file that enable pagescript 5 to basically use drupalSettings. i am not sure if it would make much sense to add that snippet upstream cuz strictly speaking the easiest and preferable approach for folks wanting to use skipto would be using the skipto module. but maybe i can open an issue and simply ask the maintainer. would be the most convenient if it would get added but still also not biggie to add it every new skipto asset release? but that approach is definitely the cleanest and saves more module code.
and maybe we can come up with shorter variable name for for example $currentUser or $userData? that way we might also be able to use one liners instead of multi line statements in SkiptoSettingsUser.php (line 72 and 76).
and aside @jurgenhaas and @drubb please also credit @boromino who participated in this weeks discussion and initially came up with the whole abstraction approach.
and i'Ve shortened two of the three fieldset keys. that way less multiline statements were necessary anymore.
ok, i finally managed to get the refactoring working. instead of two separate form classes, one for global and one for the user with redundant form code we now have a single abstract class with the form and an abstract method to get the default value. in the course of the refactoring i also aligned the config and user data, now the config is also flat schema wise. manually tested on d10 and d11 and also checked with phpstan on d10 and d11 as well as phpcs on d11, everything passes now.
added 🐛 on the mobile viewport with the navigation sidebar collapsed the menu items are still available in the aural interface Active to the screen readers section
forgot to add the mp4
rkoller → created an issue. See original summary → .
rkoller → created an issue. See original summary → .
discussed the issue in todays weekly lean coffee call of the drupal user group munich a second time. another detail to improve is to move away from the two files (skiptoglobalsettings.php & skiptousersettings.php) the MR currently employs. that way you have redundant form definitions. the consensus the group agreed to is go go with an abstract class skiptosettings and extend it one time for global settings and another time for user settings, that way you have three files. will give it a try to improve the MR, assigned the issue to myself.
i'Ve ended with the preliminary snippet which is also using the right variables for the background compared to #3
'gin': {
hostnameSelector: 'drupal.org',
fontSize: 'var(--gin-font-size)',
fontFamily: 'var(--gin-font)',
menuTextColor: 'var(--gin-color-title)',
menuTextDarkColor: 'var(--gin-color-title)',
menuBackgroundColor: 'var(--gin-bg-layer)',
menuBackgroundDarkColor: 'var(--gin-bg-layer)',
menuitemFocusTextColor: 'var(--gin-color-button-text)',
menuitemFocusTextDarkColor: 'var(--gin-color-button-text)',
menuitemFocusBackgroundColor: 'var(--gin-color-primary)',
menuitemFocusBackgroundDarkColor: 'var(--gin-color-primary)',
focusBorderColor: 'var(--gin-color-focus)',
focusBorderDarkColor: 'var(--gin-color-focus)',
buttonTextColor: 'var(--gin-color-button-text)',
buttonTextDarkColor: 'var(--gin-color-button-text)',
buttonBackgroundColor: 'var(--gin-color-primary)',
buttonBackgroundDarkColor: 'var(--gin-color-primary)',
}
with those everything is colored in the right way. there are more settings, but with those i failed to get them even properly to override
zIndex: '2000000',
z2Index: '20000002',
zHighlight: '19999',
hiddenHeadingColor: 'var(--gin-color-title)',
hiddenHeadingDarkColor: 'var(--gin-color-title)',
hiddenHeadingBackgroundColor: 'var(--gin-color-primary)',
hiddenHeadingBackgroundDarkColor: 'var(--gin-color-primary)',
dialogTextColor: 'red',
dialogTextDarkColor: 'red',
dialogBackgroundColor: 'blue',
dialogBackgroundDarkColor: 'blue',
dialogBackgroundTitleColor: 'green',
dialogBackgroundTitleDarkColor: 'green',
highlightBorderSize: 'x-large',as you can see the values for the dialog dont take even effect when you apply them to the skipto theme.
looks like the errors are gone... closing this
should tests be added? the recent article by @philipnorton42 https://www.hashbangcode.com/article/drupal-11-object-oriented-hooks-and... has a few example and ideas for how to test oop hooks?
ok tests are green now and everything is working as expected in d11 and d10 with manual testing. just to clarify as long as a user hasnt saved their skipto settings the user is using the global settings as soon as a user is saving their settings on their user profile page the user settings are used for that user from that point on. you can still alter the global setting if you have the permissions but they have no effect to the skipto settings of your own user
added 🐛 The “reduce motion” setting in the operating system should be respected Active to the animation section in the issue summary
hmmm i am kind of lost and out of ideas. i've fixed the phpcs errors.. the only thing failing is the pipeline for drupal 10 ... on my local test it worked in drupal 10 before without a problem, now when i try again with the recent MR it fails with a WSOD. but if i copy in my local copy that fails as well. not sure what is causing that. i'll set the issue to needs review also for all the other changes introduced in the MR. ill unassign myself and set the issue to needs review. maybe someone else has an idea (but i keep looking alongside)
and could please credit @jurgenhaas and @drubb on this issue? the two helped clearing up my confusion in the weekly lean coffee call, moving from procedual to OOP hooks, and making sure that the legacy hooks in drupal >10.1 are working as well. on a side note i've also tested skipt in upgrade status and except the ^12 in the info.yml there were no additional changes necessary. i'Ve already added the 12 to the info,yml as well on the MR. it does no harm imho.
initial draft is up, i'll fix the phpcs pipeline errors now.
setting back to active and assign to myself
@kentr the blue bulk action button is unrelated to this issue, but the problem is not related to the font size, the white of the button label against the blue of the "apply to selected items button simply has a too low contrast ( as axe and webaim correctly flagged a contrast of only 4.23)... on the other hand on hover the button background has only a contrast of 2.62:1 against the bulk bar.
hm one follow up i've already created quite a while ago, the one referenced in the sidebar 🐛 [PP-1] Make the relative default value validation more forgiving towards deviating time zone notations Active . but the more important other one i am struggling to write up for a while now. problem is that the relative default date time using timezones implicitly ripples though the entire problem space. meaning it isnt only requiring a followup for the relative default date time field in the field settings but changes to other parts as well. i did some research and currently struggling to chop things up. probably the "easiest" might be to open a meta issue outlining the most pressing problems. then it can be decided how to proceed. will try to finish that hopefully on the weekend. but have to finish a few other things first.
one test that might be considered adding is simply checking the skipto.js file for all the key values that are currently added to the skipto module and also those that will be added soon. that way it would be a fail safe that not one default value or default key changes at one point. that way it would be safer to update to a new version of skipto.js