Implement bulk operation designs

Postponing this until the design is done.

Alright will paste this exploration for the Gin Admin Theme here for the Bulk operations: https://dribbble.com/shots/10769244-Gin-FutureUI-Actions-Multi-select-co... as an example (like discussed in today's Admin UI call).

(I unassigned @kostyashupenko as the assignment occurred while I was in the process of uploading this patch... I hope there isn't too much confusion or duplicated work)

This is very much a first patch. Not all the styling is present yet. The primary intent here is to implement just enough functionality of the redesign to ensure it can be done accessibly, and that it supports views bulk operations.

For accessibility, the screenreader announces when the bulk operations becomes available to operate on selections (this coincides with when it becomes sticky)
Also, users of keyboard navigation will find an option on each row that allows them to skip down to bulk operations. (maybe this should only be available on selected items?)

The minimal styling added was to get standard and VBO looking OK and seems to work... However there may be VBO options that add more form elements?

Standard:

Views Bulk Operations:
'

Setting to "needs review" because it could use feedback on the accessibility implementations and the sticky behavior. The code and styling are NOT complete and it has only been tested in Chrome and Safari. It would be nice to get some +1 on A11Y+sticky+overall approach before devoting too much time to the details.

I hope there isn't too much confusion or duplicated work)

@bnjmnm - you don't need to hope, just assign a task. How could i know if you were on it ?

Anyway, good work from quick overview.

I would suggest the following:
1. Make label inline with select, and make select with form-element--extrasmall modifier. Same for submit - button--extrasmall

2. Increase z-index of sticky part

3. position: sticky; is not working for IE. If we don't care about sticky bulk part for IE - then no need to animate it
+ animation: fadeInBottom 320ms 1 forwards;
I bet you can use @supports (position: sticky) { for it

This refines the approach in #10 and brings back in most of #2 for attribute and region addition.

I also opted for the graceful degradation approach to position: sticky;. In IE11, which does not support this, it uses position: fixed; instead. It's completely fine, just not quite as pleasant as the sticky version.

Tagging "Needs accessibility review". The addition of skip-to links, Drupal.announce() behavior when checking/unchecking items, and aria additions warrants some expertise.

1. +++ b/core/themes/claro/claro.theme
   @@ -369,6 +369,89 @@ function claro_form_alter(array &$form, FormStateInterface $form_state, $form_id
   +        $form['header']['#weight'] = 100;
   

   Instead of moving header to the bottom, should we rename the container? This way if there's something else in the header container, it won't be moved below the form.

2. +++ b/core/themes/claro/claro.theme
   @@ -369,6 +369,89 @@ function claro_form_alter(array &$form, FormStateInterface $form_state, $form_id
   +        $form['header'][$key]['actions']['submit']['#attributes']['class'][] = 'button--primary';
   

   Let's set #button_type as 'primary' and then manually set the button--small class.

3. +++ b/core/themes/claro/js/tableselect.es6.js
   @@ -0,0 +1,91 @@
   +          const $checkboxes = $form.find('input[type="checkbox"]');
   

   We should probably add a more consistent checking for the checkboxes because there could be checkboxes not related to bulk operations. At the moment this counts the deselect all checkbox too.

4. Once we have confirmed that it's feasible to make the bulk operations accessible with this approach, we should create some tests.

At the weekly Claro check in, we identified a potential need for mobile designs. There was a suggestion to make the form items stack on narrower widths, so that is a possible option.

This addresses all the points in #15, including the addition of tests. Some additional changes

• Fixed a few whitespace nits
• Addressed an issue where if the form was accessed via the back button, the bulk operations status would not be aware of already-checked checkboxes
• Implemented basic mobile styling. Design is absolutely welcome to step in to improve on this.
  Medium (only the label is repositioned)
  
  Narrow (everything stacks)
  

I discussed this with @rainbreaw earlier this week and she raised a concern that it's difficult to notice the drawer being opened. She recommended that we would try a darker background color which would make it more distinguishable.

I discussed the feedback from @rainbreaw on the Claro meeting today. We agree that we would try if one of the alternative designs created as part of the design phase would address this. We decided first to try to implement the dark background from this variation:

The designs can be found on Figma: https://www.figma.com/file/NpZUAp8BEkMJvm5gKBCDBB/Claro?node-id=16505%3A...

This implements the designs mentioned in #19. Other changes:

• The skip-to link is only appended to checked checboxes
• A handler specific to the select all checkbox so the checkbox changed event handler doesn't fire for every change related to selecting/deselecting all. Updated tests to cover this as well
• Refactored the JS to be a class and changed js-prefixed attributes to data attributes so concerns are more apparent.

IGNORE THE PATCHES ON THIS COMMENT. They were posted to the wrong issue (I have too many tabs open)

Re-posting patches from #20 so the most recent patches are not ones I mistakenly posted to the wrong issue.

I did a quick test to see how this works with Media Library.

1. The dark tray background looks great! It's a significant improvement, IMO.

2. The patch works as expected in the Media Library table view, but not in the grid view. The tray does not appear when items are selected, and the element is not full-width.

Table:

Grid:

Btw. how's the contrast between the button and the background now with the dark bg?

This patch addresses #26

Re #28: The contrast of the button background is not enough to meet any text on background standard, but apparently the standard for button backgrounds are different, something that was figured out in this issue/comment: #3023311: Modal dialog style update. So while a change isn't necessary, if there is a higher contrast color choice that works well with the overall design system, it could provide a nicer experience for low vision users.

For some reason, the tray does not appear when items are selected on the media library grid with #29

I also noticed that the hover effect doesn't look nice when there's a dark background behind form elements. I don't think this is necessarily something that has to block this issue since hover effects are not a hard requirement, but it would be nice to at least try to address this in a follow-up issue.

Added previous issues
#2721807: Design for filters and bulk operations already has a lot of mentions and related issues linked

I have a bunch of concerns about the proposal.

1. The design proposal isn't explained well. It's not clear what problem you're trying to solve, or who benefits from these changes. Without this it's "picture driven development" starting from #6. I think that some discussions haven't been captured here, is that right?
2. What's the reason for moving the bulk operations controls below the table? Who benefits?
3. What's the reason for the sticky positioning? Again, who benefits?

lauriii  2 hours ago
@rain we’ve implemented darker background for the Claro Views bulk operations: https://www.drupal.org/project/drupal/issues/3070558#comment-13756204, when would be a good time to check if this addresses your concerns?

Jul 26th, 2019
andrewmacpherson  1 hour ago
There are far more problems in that design than colour. I made some notes but I can't find them.

andrewmacpherson  1 hour ago
But I think I already said I thought the skip links and positioning was bad.

lauriii  1 hour ago
@andrewmacpherson I remember you mentioned that you have given it a spin but maybe I missed your feedback? Would you by any chance be able to post your feedback on the issue?

lauriii  1 hour ago
I think the skip link behavior has changed

lauriii  1 hour ago
the skip link is now only available after checkboxes that are checked

andrewmacpherson  40 minutes ago
I recall there were some questions, which I gave a quick "no" to, but didn't follow up with detailed reasons. I can't find the notes (I swapped machines and moved a bunch of folders).

andrewmacpherson  39 minutes ago
I think the skip-links are bizarre. I'd get rid of them entirely. I've never seen that pattern on any selectable rows ever before. Invisible controls are a great nuisance generally. Makes an unpredictable experience for keyboard and speech control. Not sure who it's actually intended to help.

andrewmacpherson  38 minutes ago
The sticky position causes problems for keyboard users, and likely for speech control users too.

lauriii  37 minutes ago
did you notice that the element is below the view and it is made sticky after a checkbox has been checked?

andrewmacpherson  37 minutes ago
I found my notes. Grepped the hard drive for the issue number :-)

lauriii  36 minutes ago
I think the target user for the invisible checkbox would be either sighted user who is using keyboard navigation or screen reader user who wants to skip to the bulk operations after checking a checkbox

andrewmacpherson  32 minutes ago
As a sighted keyboard user, I wouldn't want it. To easy to activate accidentally, makes navigation slower.  Screen reader already has several tools to reach the bulk operations, so I don't think there's a compelling case for it.

andrewmacpherson  31 minutes ago
Yes I noticed the sticky position happens after checking a checkbox.

lauriii  30 minutes ago
I guess it’s good to note that the bulk operations are visible below the view even if none of the checkboxes are checked

andrewmacpherson  30 minutes ago
I don't follow you. What do you mean by note?
Do you mean tell the user?

lauriii  27 minutes ago
I was just explaining the behavior to make sure everyone is on the same page since not everyone might have tested the patch. The reason I wanted to highlight that was that on many implementations the bulk operations form is hidden when none of the checkboxes are checked. In our implementation, the form is visible but rendered after the view.

andrewmacpherson  27 minutes ago
I tried a patch a while back, not the most recent one.

bnjmnm  27 minutes ago
The skip-link approach was specific to sparing sighted keyboard users the excessive tabstops needed to reach the form from the beginning of the table.  I had my doubts about this approach and it sounds like those were warranted.

andrewmacpherson  24 minutes ago
TBH the skip links feel like nagging, or excessive hand-holding. I'll go to the submit button when I'm ready. Remember it's _bulk_ operations, so we fully expect users will want to continue browsing to select another row.

lauriii  24 minutes ago
@andrewmacpherson do you have specific concerns on making the bulk operations form float? The reason we believe it’s a good idea is that when you check checkboxes, using bulk operations is the primary action on the page.

andrewmacpherson  24 minutes ago
Invisible controls slow keyboard navigation down, because you have to take extra care not to land on one.

andrewmacpherson  23 minutes ago
The sticky position? Yes, huge problems.
It obscures content!

rain  21 minutes ago
@lauriii @andrewmacpherson I'll take a look at this later today. @lauriii would you like to set up another time to go over this live, as well? I'm flexible on Friday and Monday morning

lauriii  20 minutes ago
@andrewmacpherson do you mean that we shouldn’t use floating elements because they might hide content under?

lauriii  20 minutes ago
I could probably do either, but I would prefer Friday

rain  18 minutes ago
Sounds good. Do you want to send an invite? I'd prefer 8:30 or 9am PT if that isn't too late for you

andrewmacpherson  18 minutes ago
For speech control, "show numbers" will show numbers for the controls in the sticky footer AND also for the controls obscured underneath it. So there's a lot of potential to activate the wrong control, or extra effort to figure it out.Keyboard tabbing takes you to controls which are visually obscured. That's a focus-visible failure. It slows down keyboard navigation because I have to swap between tabbing, and scrolling to see what I tabbed to.  It could be mitigated by using a JS scrolling on focusin. However that won't solve the other problem for speech control users.

bnjmnm  18 minutes ago
It's now very clear why skip links  would not solve my excess tab stops concern - very helpful feedback. It seems like that concern could possibly be addressed by using the grid role such as the examples here https://www.w3.org/TR/wai-aria-practices-1.1/examples/grid/dataGrids.html - but I'm not familiar enough to say that with certainty. Such a change would be out of scope for the Bulk operations changes but I'd like to look into it if this seems like a good way to improve the accessibility of these tables.

lauriii  16 minutes ago
@andrewmacpherson do you know if toolbar or sticky table headers have solved that problem?

andrewmacpherson  15 minutes ago
I saw your comment on the other ARIA grid issue, I'll reply there. I'm skeptical of that.  In any case though, it's not Claro's job.

The ARIA grid would NOT help with the problems of the sticky position. (edited)

The toolbar and sticky headers are also the same problem. Let's not make it worse.

The sticky position of the bulk operations causes some problems for at least two groups of users. In both cases it's because content is obscured.

• Speech control: when using a "show numbers" tool, it will add numbers adjacent to the visible controls in the sticky-positioned box, AND numbers for the obscured controls behind it. This is likely to cause confusion and mistakes; extra cognitive load to figure out which number identifies the control you are interested in. The obscured checkbox, entity title link, and invisible "skip-to-bulk-ops" link occupy the same space as the action select and apply button.
• Sighted keyboard users: Tabbing will focus on controls behind the sticky-positioned box, so users won't know what has focus. That's a failure of WCAG "focus visible" (and perhaps "focus order"). This can be mitigated by using the arrows or page-down keys to scroll the content, and bring the focused control out from behind the sticky-positioned box. However that slows down keyboard navigation significantly; extra effort is expended by having to alternate between tabbing and scrolling. It also increases the risk of accidentally operating the wrong control. This could be mitigated by adjusting the viewport position with a JS focusin listener, but this won't solve the problems for speech control users.

Overall, I'm worried that this creates a UI which reduces scrolling effort needed by pointer users, at the considerable expense of making things more laborious for keyboard and speech control users.

lauriii 16 minutes ago
@andrewmacpherson do you know if toolbar or sticky table headers have solved that problem?

It's a problem for those too, just in the other direction. There's another issue open for the toolbar; another problem there is that operating the #main-content link moves the start of the main content underneath the fixed toolbar. Again, this is mitigated by an awkward manual scrolling step.

Updated the issue summary with some more background on what motivated the changes

We talked to @rain about how to address the speech control issue detailed in #32. She suggested providing a toggle to disable sticky headers. I've provided a patch for this in a separate issue, #3163765: Add option to un-sticky table headers to benefit assistive tech users so it can be reviewed and determined if it's an acceptable approach.

This patch removes the skip-to links and significantly reduces jQuery use - partially achieved by adding two polyfills. All remaining jQuery does not have vanilla equivalents yet (once, :tabbable, and the event implementation)

This patch also attempts to address the accessibility concerns in #32. If focusable elements are underneath the sticky, they are set to display: none;, See display-none-if-under-sticky.gif for an example. The sticky header is at half opacity so it can be seen how the elements are hidden/shown.

If a focusable element is partially under the sticky and it receives focus, the window scrolls so the element is fully visible in the viewport. This is to match the behavior that occurs when an element partially out-of-viewport receives focus. See scroll-focused-elements-into-full-view.gif for an example.

If this approach appears sound, I would create a followup to apply a similar treatment to sticky table headers.

Gifs not embedded as they're large enough that I don't want to insist they load with the page.

Needed to update tests to reflect the changes in #35

Some feedback:

1. 
   When scrolling down, the hidden elements remain hidden for a while so that it's visible for users.
2. Problem with the display: none approach is that those elements cannot be focused by tabbing meaning that those rows get skipped in the tabbing order.

#38.1

When scrolling down, the hidden elements remain hidden for a while so that it's visible for users.

I set debounce to a lower value and added a .5s opacity transition. The visibility is also now applied to the entire row to facilitate item #2. Between these two changes the un-hiding of items feels pretty smooth. Even when it doesn't feel immediate, the behavior isn't likely to be jarring as it resembles an infinite scroll.

#38.2

Problem with the display: none approach is that those elements cannot be focused by tabbing meaning that those rows get skipped in the tabbing order.

This was changed to visiblity: hidden so it could be applied to the table row and not alter it's height, but that doesn't directly address the concern here. This was addressed by adding an handler on the blur event. If blur occurs in a table row preceding a row with visiblity: hidden, it is momentarily unhidden to accept focus. As soon as a new element receives focus rows underneath the sticky are re-hidden.

I'll need to update to Catalina before I can test this manually. There's a very specific edge case with this solution that should be checked. This edge case would happen if:

• an element in the table triggers blur
• that element is in a row preceding a hidden-under-sticky row
• Speech navigation is enabled, and the "show numbers" feature isactive (i'm not even sure if it's possible to keep "show numbers" enabled while moving focus from one element to the next).

In the above circumstance, it is possible that the numbers for items underneath the sticky would be visible for the very brief moment between blur and the next item being focused. Were this the case, I think there are arguments to be made that this is an acceptable tradeoff for significant UX improvements. I think there's a good chance this doesn't even happen, though, but confirmation on that will need to wait until I or another contributor can test this manually with speech recognition software.

I tested #39. It seems like with the reduced debounce wait, the hiding is barely visible. I think we could probably remove the animation.

There's another problem with hiding those elements; the hidden elements are not accessible by screenreader rotor.

Re #40

There's another problem with hiding those elements; the hidden elements are not accessible by screenreader rotor.

Good call. This is yet another approach that should address the accessibility concerns mentioned earlier without hiding items form the screenreader rotor. To address this, table rows that have elements that would appear under the sticky form are pushed further down the page (and out of the viewport) by a spacer preceding the table row. Pushing it out of the viewport. This means the elements will not be seen by "show numbers" within the coordinate range of the sticky form, but they are still visible to the screenreader rotor.

It still seems like a regression to have the table rows appear on the table with a delay. I know the effect is similar to infinite scroll, but it's not exactly the same. Usually infinite scroll shows you rows immediately while scrolling until you run out of items.

I'm also concerned of the performance of the JavaScript because for example if I do page down, the page is jittering. I'm wondering how would this work with older, lower performance computer?

Would be great to hear what other people think.

We should also consider what we want to do with table rows that are partially under the tray. For example, on the gif here the row is partially under the tray and therefore it's hidden so that it can be seen.

Re #35:

If focusable elements are underneath the sticky, they are set to display: none;,

No, we absolutely must not do this. It has the effect of removing controls from the accessibility tree based on the scroll position of the viewport. To a blind screen reader user, this amounts to important information and controls disappearing then reappearing seemingly at random. These include:

• The node title. This is the most important information distinguishing the row.
• The bulk operations checkbox. If this is removed from the accessibility tree, a screen reader user cannot know it's state, and hence cannot be confident about which rows will be affected by pressing the "apply to all selected items" button. This amounts to inflicting an additional data-loss risk which only affects disabled users! That's the exact opposite of inclusion and accessibility. This is particularly bad when you consider that the problem only happens when the user has already checked at least one of the bulk operations checkboxes, i.e. the discriminatory data-loss risk is only inflicted on disabled users after they have started down this user journey.
• The entity operations links (drop button). Removing the single-entity operations links from the accessibility tree means screen reader users cannot use them. The upshot is that screen reader users get single-entity operations links for all rows, except the one which happens to be spatially beneath the sticky content. Remember that there are several ways to use this page. The fact that a user has checked a bulk operations checkbox does not mean they are committed to a bulk operation. It's OK to change your mind halfway through this user journey and decide to use a single-entity operation instead.

This issue is turning into a lesson in how to dig a deeper hole through code. I'm not confident about this design at all.

EDIT: Ah, I see @lauriii picked up on this in #40. It's not just about the "screenreader rotor" though (that's a VoiceOver-specific feature). It also means these controls won't be found by any other screen reader feature, such as "move to next checkbox", or even just reading the page in linear DOM order. When using table navigation (e.g. go down a column) some cells will appear empty. Note, I haven't studied the changes in #41 yet.

I'm having trouble following the screen recordings. I can't pause them, the movement is fast, and I'm not sure where the "start" of the sequence is.

I notice that the sticky footer is transparent in some screenshots/movies, but not in others. Why's that?

+        $label = t('Perform actions on the selected items in the %view_title view', ['%view_title' => $view_title]);
+        $label_id = $key . '_group_label';
+
+        // Group the bulk actions select and submit elements, and add a label
+        // that makes the purpose of these elements more clear to
+        // screenreaders.
+        $form['bulk_actions_container']['#attributes']['role'] = 'group';
+        $form['bulk_actions_container']['#attributes']['aria-labelledby'] = $label_id;
+        $form['bulk_actions_container']['group_label'] = [
+          '#type' => 'container',
+          '#markup' => $label,
+          '#attributes' => [
+            'id' => $label_id,
+            'class' => ['visually-hidden'],
+          ],
+          '#weight' => -1,
+        ];

Lots of issues here:

1. 'Perform actions on the selected items in the %view_title view', - This is quite a long label. Why not something short like "bulk actions"?
2. It also reads more like a set of instructions, rather than an accessible name. It seems more suited to an accessible description relationship.
3. Why is this just for screen readers? Doesn't everybody deserve an explanation of what this is for? If these instructions are important, they should be presented inclusively to everyone. I don't see a justification for making the label visually hidden.
4. Why ARIA role="group", instead of HTML <fieldset>? See the first rule of ARIA use.

Most importantly, this really doesn't belong in Claro. In the "content vs. presentation" separation-of-concerns, form semantics, labels, and instructions are most definitely content.

If the information from Views module is deemed inadequate, it should be addressed in Views module, so that all sites can benefit regardless of what theme is used. By putting this in Claro, all we're doing is making it harder for other themes to keep up; they'll have to duplicate this effort. And while Claro is lovely, I don't expect distros to ditch their own admin themes. So I'm strongly against theme-specific information overrides like this in the name of accessibility.

I do think it's worth filing a separate issue to explore this in Views module. If views had configurable label and/or instruction text here, then it could be usefully tailored to individual views, for admin and public use cases. That would be a very versatile feature, and I think could be implemented in a way that doesn't disrupt existing sites.

+/**
+ * Labels within a bulk operations form are styled the same as .visually-hidden.
+ * This is done instead of giving them the visually-hidden class as this will
+ * work at any nesting level.
+ */
+
+.views-bulk-actions__item .form-item__label {

What's the justification for making this visually hidden? It goes against WCAG SC 3.3.2 Labels or Instructions.

By doing so, you're creating more effort for speech control users. Controls are activated by saying the name of the control. If this is invisible, then it requires the "show numbers, choose 56" method. So a single-step action has been replaced with a two-step action. For more detail see #3083570: Do not hide the label for the text format select input.. This is a repeat of the same mistake.

I think this is also made worse by the proximity of the "2 items selected" text, especially in the narrower viewports. See the screenshots in #17 (not sure if those up to date). Elsewhere, select controls have a bold label adjacent to them. Here the label is invisible, and the bold "2 items selected" occupies a similar position. So my worry is that this may be mistaken as the label for the select control. I think that would amount to a failure of WCAG SC 2.5.3 Label in Name - the text which occupies the usual position of the select label isn't the accessible name of the control. I'm not confident about this assessment though; the proximity bothers me, but on the other hand the "2 items selected" doesn't read like the label for a select. It might be worse in non-English localizations though.

I recommend restoring the visible label for this select.

+          operationsAvailableMessage = Drupal.t(
+            'Bulk actions are now available. These actions will be applied to all selected items. This can be accessed via the "Skip to bulk actions" link that appears after every enabled checkbox. ',

A few issues here:

• "Bulk actions are now available." This isn't necessary, because it isn't really true. The bulk operations controls were already available. Nothing has changed as far as the accessibility tree is concerned. The only difference is the sticky spatial position of these controls, and I don't really think that's worth announcing. It isn't critical for a blind user to know this, and if a sighted screen reader user spots the visual change they can choose to inspect it at their own discretion.
• "These actions will be applied to all selected items." I'm not sure this is relevant at that point in time.
• "This can be accessed via the "Skip to bulk actions" link that appears after every enabled checkbox. '," These links were removed in an earlier patch, so this is leftover cruft. Even when those links were in the patch, this was an excessively verbose message.

Overall, I think the entire message can be removed.

This patch was created when #42 was the most recent comment, it happened that several comments appeared in the window between creating the patch and uploading it here. Will read through the additional feedback and address in an additional patch.

Aha, both working on this at the same time. I'm not done yet, there's a lot going on in this patch. More comments to follow.

Removing some outdated questions from the issue summary...

How do screenreaders respond - particularly when the bulk operations become sticky on the bottom of the screen.

The first part is clause is vague, and the second part isn't relevant after #47.

How are bulk operations accessed via tab navigation

The same way any other control is accessed. Press tab until you get there. I don't think the bulk operations button deserves any special treatment. You may as well ask "How do I get back to the row I selected earlier, because I've changed my mind?".

Should there be a way to easily access bulk operations without having to key through an entire table?

Ye Gods, please no :-) The dynamically-added skip-links were discussed in #32 and have been removed from the patch. It was an over-engineered, untested, and novel pattern with no known precedent. Moreover, it would be bizarre to apply this to a single form. Other forms in Drupal are also long (product entity forms, say).

Besides, pressing tab is already a well-known, straightforward, and reliable way to reach them (at least, in the sense that it doesn't require much thought). I think designers often focus on the number of tab stops, and have an unrealistic impression that tabbing is some great labour. In practice, many habitual keyboard-only users take it in their stride. Invisible controls are a much greater nuisance when tabbing.

Are there tab order concerns since this is sticky?

Yes, much discussion so far. Leaving this item in the issue summary since it's ongoing.

Thank you for the review @andrewmacpherson!

I notice that the sticky footer is transparent in some screenshots/movies, but not in others. Why's that?

I think the footer was made transparent just for demonstration purposes so people could see that there are not elements under the bulk operations tray.

In the comment for scrollResizeHandler():

Since "show numbers" [...] does not display during scroll/resize

I'm not sure about this. Note that some speech control AT (e.g. Android) has persistent numbering; while others show the numbers on demand, then remove the numbers after choosing one.

The scroll handler stuff is interesting, but I'll need time to grok it well. There are lots of scenarios that lead to scroll events, so I'm thinking hard about who's going to be frustrated by this.

Question: is the spatial displacement of rows downwards, or upwards? Hard to tell from the GIFs.

Another comment I don't follow:

When navigating via
+        // keyboard, advancing to an element that is preceded by this spacer div
+        // can result in non-intuitive scrolling to bring the newly focused
+        // element into view. To ensure pleasant scrolling, the spacer is
+        // momentarily removed when blur occurs on rows preceding it.

What are "non-intuitive" and "pleasant" actually referring to here? What's wrong exactly?

I'm not sure about this. Note that some speech control AT (e.g. Android) has persistent numbering; while others show the numbers on demand, then remove the numbers after choosing one.

Is there another app for this in Android than the Voice Access from Google or does Voice Access allow configuring the numbers as persistent? I tested Voice Access and couldn't figure out how to set the numbers as persistent.

Question: is the spatial displacement of rows downwards, or upwards? Hard to tell from the GIFs.

The rows are pushed downwards but it shouldn't be apparent in any way visually on the screen.

Updated comments based on feedback in #53 and #51. The new phrasing may help with the overall understanding of what the new JS is doing, but if another round is needed that's fine - explaining this in a concise manner is tricky and may require a few tries :).

Reroll

Is there another app for this in Android than the Voice Access from Google or does Voice Access allow configuring the numbers as persistent? I tested Voice Access and couldn't figure out how to set the numbers as persistent

Yes, there are various speech control applications for Android, but the one I've used is Voice Access from Google. I'm using Android v9 with Voice Access v4.

Voice Access makes numbers persistent by default (indeed, I don't know how to make them otherwise). When you say "show numbers", they'll persist until you say "hide numbers". The numbers will remain visible on screen whilst scrolling, though their position can fall out of step with the thing they refer to, depending on how fast you scroll.

I wonder if you're making mixed use of speech and finger touch? There is a setting called "cancel on touch", but that's equivalent to "stop listening" rather than "hide numbers". If using "show numbers" at the same time as "cancel on touch", then a finger touch will make the numbers disappear. Numbers will appear again when listening is re-activated.

Just a thought (and sorry for joining the party late): once the user has started selecting checkboxes, it is clear that they intent to perform bulk operations instead of filtering. So instead of having the bulk operations form appear at the bottom of the screen and become sticky, how about we have it so that the filtering controls container at the top is replaced by bulk operations.

In #51 there was uncertainty regarding part of a comment that states:
Since "show numbers" [...] does not display during scroll/resize
And this discussion continued in #54 and #57

The statement was removed in #55, since it was potentially inaccurate and ultimately unnecessary. The functionality has not changed, however. In instances where "show numbers" labels are present during scroll and resize events, these labels might wind up underneath the sticky *during* these events. When these events complete, the spacer is reintroduced and no "show numbers" labels appear underneath the sticky. I feel this is acceptable as "show numbers" labels would not be used during resize or scroll anyway, so the viability of the approach is not dependent on these labels being hidden during these events.

From #58

it is clear that they intent to perform bulk operations instead of filtering. So instead of having the bulk operations form appear at the bottom of the screen and become sticky, how about we have it so that the filtering controls container at the top is replaced by bulk operations.

This is interesting, but there's a few reasons I don't think it would work.

• This compromises visibility of system status, there is no longer any indication of the filter options currently in use.
• Although additional filtering is unlikely once on or more items are selected for bulk operations, filtering and selecting items are not mutually exclusive actions, and one should not prevent the other from happening. The hiding of filters could be perceived as broken, too, as that and selection are not inherently connected.

#56 Patch Failed, Attached screenshot here.

@Sakthivel M Thanks for reviewing this.

We can retest the patch on a particular version to check patch status, instead of posting SS of the terminal.

#64 Please review the patch

The Action button is displaying after the content now. Apply to selected items is visible after the content only.
I have added a before and after patch screenshot for #65.

Updated IS

Reverted the issue summary update in #67. The screenshots added there and in #66 are not accurate. They have the PHP part of the new design, but not the CSS or JavaScript. Most likely a cache needs to be cleared.
The new designs are already provided in the issue summary via a Figma link https://www.figma.com/file/NpZUAp8BEkMJvm5gKBCDBB/Claro?node-id=16591%3A...

patch #65 failed for drupal 9.3.x dev

Re #69

patch #65 failed for drupal 9.3.x dev

This is not accurate. There is no patch in #65, it's a merge request, and it is erroring on files that are no longer being used as of #65. This must be patch #64 or older.

This issue is no longer using patches, use the merge requests instead. #65 is applying fine, otherwise the tests wouldn't be passing.

Verified and tested merge request #65.
Merge request applied successfully but not working as expected

Testing Steps:
# Goto: Appearance > Apply Claro theme
# Goto: admin/content
# Select the multiple contents
# The Action button is not visible

Expected Results:
# The "Action" button is displaying after the content now. "Apply to selected items" is visible after the content only.

Actual Results:
# The "Action" Word is missing.
# "Apply to selected items" button should be under the Action button.
# Black background color should not appear.

Not working as expected.
Moving to Needs work.

Where did you get the expectations used for the review in #71, @chetanbharambe? It seems like you're reviewing based on different criteria than what is in the issue summary and comments.

The things you said didn't meet expectations:

• "The "Action" Word is missing.": Your "after" screenshot show a list of actions, which matches the design in the issue summary https://www.figma.com/file/NpZUAp8BEkMJvm5gKBCDBB/Claro?node-id=16591%3A...
• "Apply to selected items" button should be under the Action button. This is not true, it should be next to the action button, as seen in the designs provided in the issue summary: https://www.figma.com/file/NpZUAp8BEkMJvm5gKBCDBB/Claro?node-id=16591%3A...
• Black background color should not appear.. Yes it should, this change was discussed in #19. The Figma in the issue summary has not yet been updated as the change hasn't received full design sign-off, but it doesn't seem like it was referenced in this review much to begin with.

Feel free to find me on Drupal Slack if you'd like additional guidance with this, and if we know more about the points of confusion we can revise the issue summary to make it easier to understand.

It sees like the speech control concerns expressed by @andrewmacpherson could be moved to a followup if necessary. When I reported similar concerns with sticky navigation + speech controls in Olivero those concerns were among those that @andrewmacpherson
removed from the issue entirely and were not moved to a follow up.

If it's not concern for something that appears on every page in Olivero, I have to imagine it would be acceptable to move those concerns to a followup if the code specific to speech control accommodations is preventing this issue from moving forward.

If it seems like this I'm off base regarding what I believe to be an equivalence between the speech control issues reported here vs the ones removed in #3186349: Major accessibility problems with Olivero header show/hide feature (more interactive elements in the content table etc.), any chance the education/evisceration ratio of the response could lean heavier into the "educuation" part?

Applied patch #64 working fine and looks good for me
For ref sharing screenshots .....

Removing credit for redundant screenshots in #74

I've spent a bit of time reading through the comments and have a couple notes on this:

From @andrewmacpherson in #45

Most importantly, this really doesn't belong in Claro. In the "content vs. presentation" separation-of-concerns, form semantics, labels, and instructions are most definitely content.

I agree 💯💯💯💯💯 to this. Note I also believe this applies to lots more Claro issues.

Unaddressed issues

From @andrewmacpherson in #44

Perform actions on the selected items in the %view_title view', - This is quite a long label. Why not something short like "bulk actions"?

This isn't yet addressed.

Why ARIA role="group", instead of HTML ? See the first rule of ARIA use.

This isn't yet addressed either.

from @andrewmacpherson in #46

What's the justification for making [the bulk operations label] visually hidden? It goes against WCAG SC 3.3.2 Labels or Instructions.

This isn't addressed. However in my opinion, we don't need to see the labels (as its very obvious). Either way, it needs to be addressed.

Lets bring this to accessibility office hours

To get this issue over the finish line, we need to bring it to Accessibility Office Hours and get a complete and actionable list of tasks to complete. If we don't, it's likely that the goal posts will keep moving and this will never be committed.

I'm just appearing is this queue now, but since there hasn't been movement on the issue since January (and really since last year) how do we bring this to a11y office hours?

Brought to accessibility office hours tonight. We discussed the label on the popup's select, and we decided we do need it.

We did a lil' bit of designing in the browser and came up with this, which Mike Gifford (and the rest of us) feels looks good and is accessible.

What's the justification for making [the bulk operations label] visually hidden? It goes against WCAG SC 3.3.2 Labels or Instructions.

Crediting other folks on the accessibility call.

Thanks for doing that in the fly at the office hours last night @mherchel. This addresses the label issue and I think streamlines the process. Would still need to deal with the issues Andrew in #44 but I think it's a step ahead.

Should be obvious from the screenshot, but also note the proposal to remove the separate “3 items selected” counter and to update the button label accordingly. That way, we always know how many things we’re going to touch if we press the button. The info you need is always right there.

one further detail about the blue button on the black background i've noticed and checked right now. it isn't meeting WCAG SC 1.4.11. just took the values from the screenshot in #79 with the color picker in CCA. the blue in the button is #144DC4 and the black in the background color is #22222F leading to a color contrast of 2.2:1 but 3:1 would be required.

The blue used on dark backgrounds should be --color-blue-400 which should pass at 3.61:1
White text to blue 400 would be around 4.29:1

I rebased the MR on 10.1.x. Also addressed the feedback from #88 and #79.

This is how the bulk operations look like currently:

This looks great! Thanks for all the hard work everyone! 💪 I'm removing the "Needs accessibility review" because of #79

I tested this in RTL, on Chrome, FF, and Safari. I tested narrow screens.

I also tested this in forced colors emulation.

I took special care to look though the accessibility of the new functionality including colors, focus handling, focus states, etc.

The only issue that I found is that the button text only has a 4.25:1 contrast ratio. It should be at least 4.5:1. If this is a larger issue than this specific button, we can create a followup. But in the meantime, I'm setting this to needs work. Once this color issue is resolved, this should be good to RTBC!

Addressed the issue with the button contrast based on input from @ckrina.

the only problem i see, the latest changes fix the color contrast of the white text on the blue button color but at the same time it reduces the color contrast of the blue button in contrast to the blackish background. rgb(0,78,255) and rgb (35,36,41) lead to a color contrast of 2,61:1 and it should have at least 3:1 (WCAG SC 1.4.11 - see #87)

and i've noticed one other detail. i've learned about how to emulate the high contrast mode in blink based browsers in yesterdays project browsers a11y call by @mherchel (thanks for that again!). i've tested on macos 12.6.1 and the latest edge (darkmode, forced color active, prefers contrast more set in the rendering tab).

if i compare the screenshot:

with

in #93

the separation lines are greyish and so is the drop button. not sure if that is due to some emulation errors or a misbehavior in edge in general. i have no prior experience testing the high contrast mode as i've mentioned. therefore not sure if it is something relevant to deal with.

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.

Moving back to NW as there appear to be some issues brought up.
The MR appears to have a build error also.

#95: Based on https://www.w3.org/WAI/WCAG21/Understanding/non-text-contrast, the current design may be considered a success against WCAG SC 1.4.1. Quoting the relevant part:

A button which has a distinguishing indicator such as position, text style, or context does not need a contrasting visual indicator to show that it is a button, although some users are likely to identify a button with an outline that meets contrast requirements more easily.

The background color used here also has a significantly higher contrast compared to the default grey button: #3272266: Grey button's background color has a too low contrast ratio against page background which is used for the button currently. Because of that, I don't this should not consist of a regression to accessibility.

In #93 I state

I'm removing the "Needs accessibility review" because of #79

In this comment, I'll actually remove that tag!

I've tested this locally and everything worked fine on all breakpoints. So +1 to this getting in.
Since it's such a big issue it'd be great to get another pair of eyes to RTBC this.

Testing https://git.drupalcode.org/project/drupal/-/merge_requests/2902#note_140393

And the bulk operations bar is certainly more noticeable now.

Tested with the various chrome emulators (ipad, ipad pro, iphone10, etc) and all looks good.

+1 for RTBC

Went through comments and updating issue credits.

Committed d757e2c and pushed to 10.1.x.

We've been working together with @lauriii reviewing this issue and all the sing-offs. Since he's a Claro maintainer, I'm UX and Claro maintainer, and this issue has been discussed at the Accessibility office hours and all the issues raised have been addressed, we considered this has gone trough all the gates to get in. It is an important UI change, but since we still have the option to revert it we've considered it's more important to get that in and properly tested with enough time rather than waiting for more reviews in the issue. It'll give more time to any contrib or custom module maintainer to work on any needed fix.

Tagging for 10.1.0 release highlights.

Updating tag

Some side effects from this do indeed affect VBO

#3378091: Disabled primary button in views area has grey text on blue background (bad contrast)
#3378090: Styling broken in VBO form on Drupal 10.1