Problem/Motivation

Many forms in Views UI have long lists of options which can be filtered by keyword or category (e.g. Add sort criteria dialog). When using these filter controls, the contents of the list change dynamically. The changes are easily noted visually, but are not conveyed to users with screen readers.

Proposed resolution

Use Drupal.announce() to convey a short message about the number of options now present in the filtered list.

We already do this with the module filter on the admin/modules page - see system.modules.js.

Remaining tasks

  • Devise the announcement string(s): "6 options are available in the modified list."
  • Write a patch.
  • Tests - can we confirm the updated content of div#drupal-live-announce with JavascriptTestBase?
  • Review changes to javascript to reflect comments by @cottser in #45 See #80.

User interface changes

  • No visible changes intended.
  • Add a screen reader announcement to convey updates which are currently only apparent visually.
  • Introduces a new translatable string for Drupal.announce().

API changes

None expected.

Data model changes

None expected.

CommentFileSizeAuthor
#80 2805197-80.patch3.39 KBLendude
#80 interdiff-2805197-79-80.txt512 bytesLendude
#79 2805197-79.patch3.39 KBLendude
#79 interdiff-2805197-78-79.txt1.65 KBLendude
#78 2805197-78.patch3.19 KBLendude
#63 interdiff-2805197-56-63.txt1.06 KBstarshaped
#63 2805197-63.patch3.82 KBstarshaped
#56 2805197-56.patch3.99 KBandrewmacpherson
#56 interdiff-2805197-47-56.patch808 bytesandrewmacpherson
#47 Interdiff-2805197-37-47.txt901 bytesrachel_norfolk
#47 2805197-47.patch3.99 KBrachel_norfolk
#44 2805197-37-screenshot-23-options-available.png452.57 KBrachel_norfolk
#37 interdiff-2805197-36-37.txt1.8 KBloopduplicate
#37 2805197-37.patch3.91 KBloopduplicate
#36 2805197-36.patch3.1 KBLendude
#36 interdiff-2805197-34-36.txt795 bytesLendude
#34 2805197-34.patch2.22 KBLendude
#34 interdiff-2805197-33-34.txt1.7 KBLendude
#33 interdiff-2805197-27-33.txt1.85 KBleft
#33 views_edit_page_options_list_filter_accessibility_feedback-2805197-33.patch1.36 KBleft
#27 2805197-27-views_edit_page_options_list_filter_accessibility_feedback.patch893 bytesMunavijayalakshmi
#25 Interdiff-2805197-21-25.txt619 bytesrachel_norfolk
#25 2805197-25-views_edit_page_options_list_filter_accessibility_feedback.patch932 bytesrachel_norfolk
#21 2805197-21-views_edit_page_options_list_filter_accessibility_feedback.patch928 bytesleft
#19 2805197-19-views_edit_page_options_list_filter_accessibility_feedback.patch576 bytesleft
#16 2805197-16-views_edit_page_options_list_filter_accessibility_feedback.patch975 bytesleft
#15 2805197-14-views_edit_page_options_list_filter_accessibility_feedback.patch691 bytesstarshaped
#14 2805197-14-views_edit_page_options_list_filter_accessibility_feedback.patch691 bytesstarshaped
#9 interdiff.2805197.8.9.txt943 bytesDuaelFr
#9 2805197-9-views_edit_page_options_list_filter_accessibility_feedback.patch909 bytesDuaelFr
#8 2805197-8-views_edit_page_options_list_filter_accessibility_feedback.patch562 byteskiwimind
#6 2805197-6-views_edit_page_options_list_filter_accessibility_feedback.patch574 bytesEhud
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andrewmacpherson created an issue. See original summary.

andrewmacpherson’s picture

Title: Provide screen-reader feedback when Views filterable options are updated » Provide screen-reader feedback when Views UI filterable options are updated
andrewmacpherson’s picture

mgifford’s picture

This is the exact use case why Drupal.announce() was built. We just need to use it more consistently.

andrewmacpherson’s picture

Issue summary: View changes
Issue tags: +Dublin2016, +Novice

This would be a good novice task for a mentored sprint at Dublin DrupalCon 2016.

It basically involves a copy/paste, adding a JS behaviour to views module.

We have an accessibility table in the general sprints room but I can come to the mentored room if you need me.

Details of the Drupal.announce() are here:
New accessibility feature: Drupal.announce() (change record)
Accessibility tools for JavaScript in Drupal 8 (handbook page)

Ehud’s picture

Patch by Matthew Lambert (xiwar) and Ehud Ovadia (Ehud).

kiwimind’s picture

Re-rolling to fix indentation issue.

DuaelFr’s picture

I don't like the way the results are counted. The entire handleFilter function is DOM-agnostic so I think we should keep it this way so, if an admin theme wants to change the way that UI is done, this function will continue working. See the attached patch.

I think that we could test this feature but, as an accessibility improvement, I'd like it to be commited quickly and maybe add tests in a follow-up. @mgifford what would you think about that?

mgifford’s picture

I'd like to do that, but not sure that we'll get buy-in. The D8 cycle has had tests first for pretty much everything.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

BarisW’s picture

Status: Needs review » Needs work
+++ b/core/modules/views_ui/js/views-admin.js
@@ -536,10 +537,18 @@
+      Drupal.announce(
+        Drupal.t(
+          '!views views are available in the modified list.',
+          {'!views': areActive}
+        )
+      );

I suggest using Drupal.plural to make sure that we don't end up with '1 views are available'.

starshaped’s picture

Working on this as part of #sprintweekend2017 #sprintweekendBOS.

starshaped’s picture

Added call to Drupal.formatPlural to display '1 view is' or 'n views are' depending on the search results. Patch attached.

starshaped’s picture

Re-uploading the patch and flipping the ticket to 'needs review' to kick off testing.

left’s picture

This patch is based on #9.

I think #14 is incomplete due to not having incorporated #9 and the use of both Drupal.t and formatPlural was not necessary.

I've used only formatPlural (without Drupal.t) and I've renamed the areActive variable to activeViews as I feel it is more descriptive.

BarisW’s picture

Status: Needs review » Needs work

Great work left and starshaped. The patch in #16 works, but can be improved just a little.

+++ b/core/modules/views_ui/js/views-admin.js
@@ -536,10 +537,18 @@
+      Drupal.announce(
+        Drupal.formatPlural(activeViews,
+          '1 view is available in the modified list.', '!views views are available in the modified list.',
+          {'!views': activeViews})

One thing to improve here: formatPlural already replaces the @count variable out-of-the-box. If you only want to replace the number of items, you don't need to add the extra parameters.

This would suffice:

<?php
Drupal.announce(
  Drupal.formatPlural(activeViews, '1 view is available in the modified list.', '@count views are available in the modified list.')
);
?>
left’s picture

Hi @BarisW

Thanks for the review and input. I've made a new patch to reflect and changed the status to 'needs review'.

BarisW’s picture

Status: Needs review » Needs work

Ah, but now the patch is missing the logic to count the results. Plus, the Drupal.announce is new. This seems more like a diff against your precious patch than against drupal core?

left’s picture

Oops, you're right. I think I might be missing some details about the patching procedure. It's starting to make more sense now.
Another patch attempt attached :) Thanks for taking the time @BarisW

BarisW’s picture

Perfect. When the test turns green this is rtbc according to me.

andrewmacpherson’s picture

Status: Needs review » Needs work

The message in patch #21 says "3 views are available in the modified list." (emph mine), but it isn't a list of views - it's a list of fields (or filters, or sort options, etc.).

The tasks in the issue summary says "6 options are available..." so that's minor change we still need.

Bonus idea: could we get the message to reflect which list we are looking at? Something like "6 fields are available..." vs "6 filter types are available...". If that turns out to be hard, we can leave it for a follow-up.

rachel_norfolk’s picture

Think I can see how to do this - taking a look now. Give me till the end of the day...

rachel_norfolk’s picture

Whilst it annoys me greatly that I can't seem to obtain the thing being added, as per #23, we do need to change the text slightly. The text

@count options are available in the modified list.

seems to make sense in all circumstances, at least.

andrewmacpherson’s picture

@rachel_norfolk - Thanks, patch #25 fixes the issue from #23.

The next step is adding functional javascript tests for the announcement message.

Some of the related Drupal.announce() issues already have some tests which are partly written. #2715663: Use Drupal.formatPlural for when announcing module-filter results for screenreader users is a good starting point. If anyone wants to have a crack at those, make sure you understand how to run FunctionalJavascript tests in D8. See Javascript Testing Comes to Drupal 8.

Munavijayalakshmi’s picture

Status: Needs review » Needs work
benjifisher’s picture

Looking at the testbot results, I see this:

TypeError: undefined is not a function (evaluating 'Drupal.announce')
TypeError: undefined is not a function (evaluating 'Drupal.announce')

If you look at what happens with your JS debugger, then it should be pretty easy to see what the problem is.

dmsmidt’s picture

Issue tags: +sprint
BarisW’s picture

For those wanting to write the functional javascript tests, here is an example: #2805205: Provide screen-reader feedback when filtering by block name.

left’s picture

Assigned: Unassigned » left
left’s picture

The new patch includes announce.js added as a dependency to views_ui.libraries.yml to correct the undefined function error.

And I added a test for the singular case, where one option is available in the list.

I'm adding a patch and inter-diff to reflect.

The plural case, where multiple options are available in the list, still needs a test.

Lendude’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
1.7 KB
2.22 KB

Added test coverage for the multiple rows feedback.

Status: Needs review » Needs work

The last submitted patch, 34: 2805197-34.patch, failed testing. View results

Lendude’s picture

Status: Needs work » Needs review
FileSize
795 bytes
3.1 KB

Actually including the fix might help :)

loopduplicate’s picture

We need to edit views-admin.es6.js and transpile it to views-admin.js. See Drupal core now using ES6 for javascript development , which is also linked to in a comment at the top of views-admin.js. Here's an updated patch.

loopduplicate’s picture

Status: Needs review » Reviewed & tested by the community

This looks pretty good to me. The tests are still passing too. Marking as RTBC.

larowlan’s picture

Status: Reviewed & tested by the community » Needs review

Hey, can we get someone else to RTBC your patch?

Thanks

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

rachel_norfolk’s picture

Version: 8.5.x-dev » 8.4.x-dev
Status: Needs review » Reviewed & tested by the community
FileSize
452.57 KB

Latest patch applies cleanly to 8.4.x and I believe we are still allowed to submit this change there, based on the rules. Changing version above to reflect that.

Using Apple VoiceOver and Safari, I am now correctly notified of the number of elements in the list, as desired by the change. No unwanted side-effects seem to be apparent.

Screenshot showing the text being read out correctly by VoiceOver (kinda happy it does that now!)

screenshot showing VoiceOver reading out the correct number of items in the modified list

I think RTBC

star-szr’s picture

+++ b/core/modules/views_ui/js/views-admin.es6.js
@@ -534,10 +535,15 @@
+        activeViews += found;

Maybe this would be clearer as:

if (found) {
  activeViews++;
}

Couldn't find a standard on this but it made me do a double take because of the type mixing :)

rachel_norfolk’s picture

Status: Reviewed & tested by the community » Needs work

I’m inclined to agree. I didn’t understand it and just assumed it was because of my lesser js experience.

I’ve had a search through and can’t find anywhere else in core where we do this, mixing the types. Given it’s a simple fix for better clarity, I think we should do it. I’m going to have a look now how to do this es6 thing...

rachel_norfolk’s picture

Status: Needs work » Needs review
FileSize
3.99 KB
901 bytes

Okay, this is my first time using this fancy-pants yarn thing for es6. Please check I’ve not done anything too bad!

findleys’s picture

Will this ticket cover reporting success/failure messages for the action updates on views as well or just filter changes? I'm trying to determine is a ticket already exists for the actions or if I need to create a new one to report to our 508 compliance organization.

andrewmacpherson’s picture

@findleys This issue is just about the behaviour of the filters. Please file a separate issue (it's best to keep an issue focused on one problem).

rachel_norfolk’s picture

Issue summary: View changes

Just updating Issue Summary remaining tasks

Lendude’s picture

Status: Needs review » Reviewed & tested by the community

Looks good now, feedback has been addressed, back to RTBC

andrewmacpherson’s picture

Category: Feature request » Bug report
Issue tags: +wcag, +wcag21
Parent issue: » #2864791: Implement new Success Criteria from WCAG 2.1

Thanks to everyone who has been working on this. It's working well!

As far as WCAG 2.0 goes, I was unsure if this sort of filtering qualified as a "change of context" under SC 3.2.2 On Input. It's kind-of borderline IMO.

However WCAG 2.1 introduces SC 3.2.7 Change of Content, which clearly matches the situation here. There's a longer explanation at #2864791-33: Implement new Success Criteria from WCAG 2.1.

Since we're now actively pursuing WCAG 2.1, I'm reclassifying this as a bug report. Under WCAG 2.0 the announcement was a nice-to-have, but under WCAG 2.1 it becomes a must-have.

andrewmacpherson’s picture

The FunctionalJavascript test doesn't try a scenario where zero items are are expected. Is this important? Note we did include a zero-items test in #2805205: Provide screen-reader feedback when filtering by block name..

xjm’s picture

Status: Reviewed & tested by the community » Needs work

Thanks for the patch and the accessibility feedback! One small thing:

+++ b/core/modules/views_ui/js/views-admin.es6.js
@@ -518,6 +518,7 @@
+      var activeViews = 0;

This introduces a regression for our JS coding standards for the no-var rule -- see #2914946: JS codestyle: no-var.

GrandmaGlassesRopeMan’s picture

@xjm 👏

andrewmacpherson’s picture

Changed var to let in the es6.js file. The .js file remains the same after transpiling.

andrewmacpherson’s picture

Doh. I gave the interdiff a .patch extension instead of .txt
Still OK to review, but expect a spurious test result.

The last submitted patch, 56: interdiff-2805197-47-56.patch, failed testing. View results

Lendude’s picture

Status: Needs review » Reviewed & tested by the community

All feedback has been addressed, back to RTBC

xjm’s picture

Status: Reviewed & tested by the community » Needs work

Just a note that while this is a string addition that normally would be done only in a minor release, I agree with backporting it because the accessibility bug (lack of any feedback) is worse than having an additional untranslated message in this UI on a smaller portion of sites.

  1. I missed #53 before. Having a test for no options is a good idea, to make sure the user gets feedback in the scenario that's probably the most confusing otherwise. Good suggestion @andrewmacpherson! So setting NW for adding that test case. We'd just need to add the last test scenario in the existing test, I think.
  2. +++ b/core/modules/views_ui/tests/src/FunctionalJavascript/FilterOptionsTest.php
    @@ -56,6 +56,10 @@ public function testFilterOptionsAddFields() {
    +    $session->wait(10000, 'jQuery("#drupal-live-announce").html().indexOf("1 option is available in the modified list.") > -1');
    

    That seems like a mighty long timeout. Turns out Session::wait() takes the parameter in ms, whereas Element::waitFor() takes it in seconds. I checked and we do Session::wait() a few other places in HEAD with 5000, but this test already uses 10s elsewhere, and JsWebAssert::waitForField() has an explicit default of 10,000 ms. (Also note how inconsistent the methods are, switching whether it's the first or second parameter...)

    Anyway. So this is okay since it's a timeout rather than a fixed time and HEAD and the API have precedent for 10s. So nothing to change here, but golly, I hope we don't use too much of that time. :)

  3. +++ b/core/modules/views_ui/tests/src/FunctionalJavascript/FilterOptionsTest.php
    @@ -71,6 +75,13 @@ public function testFilterOptionsAddFields() {
    +    // Clear the search and see if the new number of rows is set by announce.
    +    $options_search->setValue('');
    +    $block_rows = $page->findAll('css', 'tr.filterable-option');
    +    $block_row_count = count($block_rows);
    +    $session->wait(10000, 'jQuery("#drupal-live-announce").html().indexOf("' . $block_row_count . ' options are available") > -1');
    +    $web_assert->elementTextContains('css', '#drupal-live-announce', $block_row_count . ' options are available in the modified list.');
    

    Rather than doing this dynamically, it would be better to just hardcode the expected result, since the number of expected results is related to the test fixture and counting tr.filterable-option isn't really the same as just saying "We expect two rows here!" That also lets us remove a couple of lines from this hunk.

Thanks for the quick turnaround on this!

Lendude’s picture

Just some quick feedback on the feedback:

#60.2 Yeah the idea behind the ridiculously long timeout is indeed that it's never used under normal circumstances and so long we will never have random fails because of it if it occasionally takes a little long to get a response

#60.3 I'd be against this using a hardcoded number, since the expected number ('all available node and global filters') can change when we add/remove any sort of node or global filter to core. And although that doesn't happen that often it would be annoying to have to change a completely unrelated test when you do that.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

starshaped’s picture

Version: 8.5.x-dev » 8.6.x-dev
FileSize
3.82 KB
1.06 KB

Fixed #60.3 by hardcoding the expected result.

rachel_norfolk’s picture

Hmm - weirdly, whilst this appears good in Voiceover on Mac etc, I’ve just been watching it using NVDA on Windows 10 and it is reading out the number of items in the list but not the supporting text. LIterally, just “10” not “there are 10 items in the list”

Odd.

andrewmacpherson’s picture

That's an interesting find, @rachel_norfolk - thanks for showing me this at the Leeds sprint meeting.

When we listened to NVDA with this patch, we heard differences between Firefox+NVDA (whole message was correctly announced) versus Chrome+NVDA (only the number was announced).

I wonder if this about the way aria-relevant or aria-atomic properties work. Drupal.announce() doesn't try to set these - perhaps the different browsers are making up their own default values? If so, that sounds like something that could be improved in the Drupal.announce() itself. When Drupal.announce() was first written, I think it was mainly tested with VoiceOver. In any case, all the browsers and screen readers have had new releases since then. It's worth opening an issue to explore how robust we can make Drupal.announce().

I think we can stick with the current approach in this issue though.

Update: To clarify, I mean the problem/motivation is still valid. I'm open to alternative solutions though. WCAG 2.0 doesn't really address this, but it does seem to fall under the new WCAG 2.1 success criterion called "Status Updates". In the latest WCAG 2.1 working draft (final, supposedly) this has been narrowed down a lot. This issue may not be a hard requirement any more, but reading around the public W3C comments, it seems to be a borderline case for "Status Updates". I'm still digesting this, and will update our main WCAG 2.1 main plan when I know more.

andrewmacpherson’s picture

Issue tags: -Novice +SprintWeekend2018
GrandmaGlassesRopeMan’s picture

  1. +++ b/core/modules/views_ui/js/views-admin.es6.js
    @@ -541,10 +542,17 @@
    +          activeViews++;
    

    While only a warning, the eslint rules try to discourage unary operators.

  2. +++ b/core/modules/views_ui/js/views-admin.es6.js
    @@ -541,10 +542,17 @@
    +      Drupal.announce(
    +        Drupal.formatPlural(activeViews, '1 option is available in the modified list.', '@count options are available in the modified list.')
    +      );
    

    This getting a bit long, something like this might make it a bit more readable,

    Drupal.announce(
      Drupal.formatPlural(
        activeViews,
        '1 option is available in the modified list.',
        '@count options are available in the modified list.',
      ),
    );
    

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andrewmacpherson’s picture

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Lendude’s picture

Status: Needs work » Needs review
Issue tags: -JavaScript +JavaScript
FileSize
3.19 KB

Reroll, plus #67 feedback, plus rename of the variable since this is used for more than just views filtering

No interdiff because of reroll.

Lendude’s picture

Attempt at fixing the linting.

Also back to getting the number of expected values dynamically in the test, basically because of what I said in #61, the number can change (and unsurprisingly did in the intervening 5 years) and figuring out the right number is annoying.

Lendude’s picture

I should really get linting to run at some point....

dww’s picture

This came up for daily random #bugsmash triage, that's why @Lendude helpfully resurrected this and re-rolled to address the feedback. Thanks!

I wish we fixed #3122231: Singular variant for plural string must contain @count so we stopped "doing it wrong" and all new code used @count for both the single and plural strings. Probably not worth holding this up to change it, so not setting NW. Close review of the code shows no other problems to complain about.

The bot is happy. There's new test coverage. The issue title and summary are clear. I don't believe this needs / wants a CR or release note, but at least tagging that it's a "String change".

RTBC!

Thanks again,
-Derek

dww’s picture

Status: Needs review » Reviewed & tested by the community

Removing credit for #27, a 1-off re-roll with failing tests that the author never came back to help fix.

Adding credit to @BarisW, @mgifford and myself for helpful reviews.

I think that should be pretty close to accurate, but of course the committer will have the final say.

Thanks again, everyone!
-Derek

mgifford’s picture

Issue tags: +wcag327

Adding tag for WCAG SC 3.2.7

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 80: 2805197-80.patch, failed testing. View results

dww’s picture

Status: Needs work » Reviewed & tested by the community

Seems like a random fail to me:

Drupal\FunctionalTests\Installer\InstallerExistingConfigMultilingualTest
exception: [Other] Line 0 of sites/default/files/simpletest/phpunit-408.xml:
PHPUnit Test failed to complete; Error: PHPUnit 9.6.7 by Sebastian Bergmann and contributors.

Testing Drupal\FunctionalTests\Installer\InstallerExistingConfigMultilingualTest
E                                                                   1 / 1 (100%)

Time: 00:08.944, Memory: 4.00 MB

There was 1 error:

1) Drupal\FunctionalTests\Installer\InstallerExistingConfigMultilingualTest::testConfigSync
Exception: Drupal\language\Exception\DeleteDefaultLanguageException: Can not delete the default language
Drupal\language\Entity\ConfigurableLanguage::preDelete()() (Line: 177)
bnjmnm’s picture

Status: Reviewed & tested by the community » Needs work

A few optimizations to do but this overall looks good.

  1. +++ b/core/modules/views_ui/js/views-admin.js
    @@ -573,12 +574,23 @@
    +          }
    

    The unary operator converts boolean to integers 0/1, so this can be a single line
    activeItems += found

  2. +++ b/core/modules/views_ui/tests/src/FunctionalJavascript/FilterOptionsTest.php
    @@ -66,6 +66,10 @@ public function testFilterOptionsAddFields() {
    +    $session->wait(10000, 'jQuery("#drupal-live-announce").html().indexOf("1 option is available in the modified list.") > -1');
    +    $web_assert->elementTextContains('css', '#drupal-live-announce', '1 option is available in the modified list.');
    

    These two lines can be consolidated by using waitForElementTextContains() see WidgetUploadTest in Media Library

  3. +++ b/core/modules/views_ui/tests/src/FunctionalJavascript/FilterOptionsTest.php
    @@ -81,6 +85,14 @@ public function testFilterOptionsAddFields() {
    +    $session->wait(10000, 'jQuery("#drupal-live-announce").html().indexOf("' . $block_row_count . ' options are available") > -1');
    +    $web_assert->elementTextContains('css', '#drupal-live-announce', $block_row_count . ' options are available in the modified list.');
    

    These two lines can be consolidated by using waitForElementTextContains() see WidgetUploadTest in Media Library

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.