benjifisher → credited rkoller → .
The issue was raised in the #bugsmash channel on the drupal slack. i've retested on drupal 11.x with macos 15.3.1, safari 18.3, in voiceover. - tested on a node in the article content type. i am sort of unable to reproduce the problem with the subject field of a comment, instead to me it looks like more of a problem with the ckeditor field used for the comment body field.
In the video (comment.mp4) i've first added something to the subject field then tabbed to the body field and added some text there as well, then i've shift tabbed back to the subject field and started deleting characters. those deleted characters were not announced. but as soon as i started entering characters again in the subject field either after deleting everything or after one or two characters only the newly entered characters are being announced. if i then delete again characters with backspace in the subject field, those characters are getting announced now as well in contrast to when i shift tabbed back into the subject field. i've tabbed into the body field then and tried to add or delete any characters there, none of that was announced. so from my point of view it seems more like a problem with the body field?! firefox and edge are announcing every character entered or deleted in the subject as well as the body field correctly.
not sure if those changes are reasonable and enough in scope and or valid but at least the errors are fixed and the config inspector lists the schema as 100% validatable now.
will try to provide an initial MR
thejimbirch → credited rkoller → .
the_g_bomb → credited rkoller → .
grienauer → credited rkoller → .
a heavy +1 for bringing SkipTo to Drupal 11
and your proposed solution on the MR you've created with #222330 for the navigation block title color would have a color contrast of 1.1:1 against the background (rgb(42,42,45)) of the navigation sidebar in gin.
for core and the navigation component there is already 🐛 Navigation block titles have a too low color contrast Active so 🐛 Increase the color contrast of the navigation block title Active is a duplicate of that. but i've opened the issue in here because you have a dark mode for gin that does not exist for core. and the shade of grey that is used for the navigation block title in gin is the same in light and dark mode. it is right to fix the color contrast for the navigation block title in core as well. but still the darker grey that will be picked as the solution for 🐛 Navigation block titles have a too low color contrast Active might then not work with the dark background in gin (and being mindful about gin would be out of scope for 🐛 Navigation block titles have a too low color contrast Active ). it might work but still out of scope and why i think it is reasonable having a dedicated issue.
benjifisher → credited rkoller → .
i am taking a look now, was swamped the last few weeks with finishing the draft for gin.
took an initial stab trying to improve the issue summary making it more actionable and instructional.
the_g_bomb → credited rkoller → .
i agree probably it might be not only about the color contrast but also about the font size, a detail we've touched before in yesterdays discussion. and i've updated the issue summary and added a pointer what the navigation block title is in the screenshot.
just as an fyi, the images are not focusable, only the three elements on the image, the checkbox, the edit button and the remove button.
@rahuly072 your theme settings are ok, but you also have to active the forced colors mode. your screenshots show that you are just in dark mode. you have to activate forced colors mode either in your operating system in case you are on windows or in chromium based browsers https://devtoolstips.org/tips/en/emulate-forced-colors/
rkoller → created an issue. See original summary → .
rkoller → created an issue.
rkoller → created an issue.
rkoller → created an issue.
rkoller → created an issue. See original summary → .
rkoller → created an issue.
rkoller → created an issue.
rkoller → created an issue.
removed 🐛 Invalid ARIA attributes Active from the table in the issue summary, added it as a child of the newly created 🌱 [Meta] Improve the accessibility of the Gin theme Active
rkoller → created an issue. See original summary → .
jurgenhaas → credited rkoller → .
The draft of the MR is a work in progress, reflecting proposed changes we've discussed over the course of several ux meetings. The MR is not complete and ready for review yet - we plan to finalize the MR in the coming weeks. We've considered it helpful at this point being able to see all the changes in context. A few remarks:
- So far i've only added the discussed changes for the available field group options, but all those changes also rely on changes on the h1. At the moment the first two steps are just called
Add fields
across all field types and field groups. ✨ Use modal in add new field flow Active has to get in first for that. After that the proposed changes for the h1 on second step could be added as well. - While making the changes for the MR I've noticed that the changes on the field type/field group labels also entail necessary changes in help texts and tests. But those changes should happen after a final consensus was reached about the micro copy. Leave it here as a reminder.
- It has to be noted that at this point the description for every option available for the reference field group has an identical description defined in
EntityReferenceItem.php
. That way it is impossible to provide some sort of guidance to the user what the actual difference between the different reference options actually is and which option to pick in which scenario. So we probably need a separate issue to add the ability to provide individual descriptions for the different reference options. - The
file upload
field group currently has no description yet and is still using the pattern starting with "field to..." we've removed on the description for every other field type and field group. But the entire question how to deal with the media field type is out of the scope for this issue and complicates everything. @benjifisher already opened 📌 Add media reference to the same field type group as file and image Active a few weeks back - the general discussion and work should happen there.
benjifisher → credited rkoller → .
added 🐛 Navigation block titles have a too low color contrast Active
Updated the title and issue summary and added screenshots
the_g_bomb → credited rkoller → .
I've simply pushed another commit now reverting the changes of the commit on the MR from the 27th of january invalidating the changes in the pipeline.yml (4th february) that got in with the rebase... tests are green now and the test only fails properly now as well: https://git.drupalcode.org/issue/drupal-3485202/-/jobs/4243209 ... so the only thing left to do is still another pair of eyes reviewing the MR.
@oily since you are kindly following along. i would have one question. if you see the comments here in this thread, between #44 and #45 i've committed and tested the changes that got into
📌
Change the pipline service from with-chrome to with-selenium-chrome
Active
and then committed and went back to the previous state with *with-chrome
... now
📌
Change the pipline service from with-chrome to with-selenium-chrome
Active
got fixed, so i went ahead and rebased this MR to get the changes in. problem is. if you take a look at https://git.drupalcode.org/project/drupal/-/merge_requests/10631/diffs#n... , there are changes for the pipeline.yml. it somehow the changes introduced in the other issue and that got added by rebase are getting invalidated and the service changed to *with-chrome
. looking at https://git.drupalcode.org/project/drupal/-/merge_requests/10631/commits... is it possible that on a rebase all the commits on a MR are committed again after the rebased core? and that is the reason the change gets reverted again back to *with-chrome
with ba444f84??? and my question is how to fix that?
i've retested with the latest changes, and the problem in #97 ✨ Use modal in add new field flow Active with the focus not moving back to the component that revealed the dialog in safari still persists.
there is another potential rule i just ran into today, but i am not sure if the alt_text_validation module would be able to validate against it, editoria11y might be able to. but the thing i ran it was that the site used the same string the title / header of a card component is using for the alt text. in that particular case that header wasnt even marked up with an h-tag but with a p-tag instead, which might make the validation even trickier. here is an example of the html source. the title/header of the card:
<p class="h1 featured-card__title">Books - Fanzines - Magazines</p>
the image on the card:
<img class="featured-card__image lazyloaded" src="//bisaufsmesser.com/cdn/shop/collections/books_420x.jpg?v=1612535878" alt="Books - Fanzines - Magazines" srcset="" sizes="(min-width: 960px) 450px, (min-width: 720px) 50vw, 100vw" style="max-height: 247.5px;">
if you want to take a look at the different card examples in context, the img src contains the link to the domain.
another rule could be that alt texts should not contain copy right information. i saw it repeatedly that editors used the alt text simply for the copy right information crediting who took the photo not what the photo was actually about.
benjifisher → credited rkoller → .
One detail to note, i've tested the latest state of the MR and the point i've raised in #20 ✨ Use modal in add new field flow Active still applies. when the dialog modal is closed by pressing esc or the close button the focus isnt returning to the "create a new field" button, but instead to the top of the DOM, so the keyboard user has to navigate back to the previous position one left off. the expected and desired behavior is that the focus moves back to the component which revealed the content/dialog.
Adding 📌 Update to jQuery 1.14.1 and use the newly added option for dialog modal headings Active as related issue. The issue is about changing the element wrapping the dialog modals title from a span to an h1, which is relevant and important for this issue as well.
the_g_bomb → credited rkoller → .
the only pressing problem i see is the color contrast of the checkmark (rgb(85, 86, 91)) in dark mode against the background (rgb(59, 59, 63)) (only quickly used the color picker), the resulting contrast is 1.5:1, but it should have at least 3:1. other points would be styling and aesthetics related and the consistency in between light and dark mode and so on. but that could be moved to follow up issues.
jurgenhaas → credited rkoller → .
jurgenhaas → credited rkoller → .
isn't line 89 defining the with-selenium-chrome test? but what the problem was was that test-only changes used with-chrome instead. and this MR changes using with-selenium-chrome for the test-only tests instead of with-chrome.
@oily the fail was a false fail. after some more investigation and some advice from @catch it turns out the tests-only pipeline service definition has to be changed from with-chrome to with-selenium-chrome... the last two commits tested that on this issue. made the change on this MR, the test-only test then correctly failed with
There was 1 failure:
1) Drupal\FunctionalJavascriptTests\Ajax\DialogTest::testDialog
Failed asserting that null is not null.
/builds/issue/drupal-3485202/core/tests/Drupal/FunctionalJavascriptTests/Ajax/DialogTest.php:64
FAILURES!
Tests: 2, Assertions: 14, Failures: 1.
(see https://git.drupalcode.org/issue/drupal-3485202/-/jobs/4155477) . but per the recommendation of @catch i've created a new issue that makes that one line change. 📌 Change the pipline service from with-chrome to with-selenium-chrome Active . when this issue is in then we can rebase this issue and can come to a proper finish.
i've tested in the latest state of 2.0.x and the problem illustrated in the issue summary still applies.
The test-only test is failing as it should.
yeah but if you take a look at the output as noted in #41 you'll notice the two failures were:
Caused by
WebDriver\Exception\CurlExec: Could not resolve host: selenium
that is an infrastructure specific error not an error based on what is actually tested against with this MR?