Consider adding Apply and Clear buttons to Categories dropdown

One thing to bear in mind is the buttons could make things more difficult for keyboard users. Some thought would need to be given to make sure they aren't disadvantaged.

I've added the tag for wcag2.2. sc 3.2.2 https://www.w3.org/WAI/WCAG22/Understanding/on-input.html which basically illustrates why the proposed resolution, having an apply and clear button, is important or probably necessary.

re #2 📌 Consider adding Apply and Clear buttons to Categories dropdown Active in what regard would/could the buttons make things more difficult for keyboard users?

I agree that the use of apply/clear buttons would enhance the UX. Going to add this to the roadmap.

@rkoller At the moment the keyboard user can tab onto the category box, press space to open it, then use up/down arrows to move between categories, and space to select/deselect them. Pressing tab will close the drop-down and move to the next control.

I agree that having the modules change behind the drop-down as we currently do is confusing. We just need to ensure there is a clear an intuative UI for getting from the list of categories to the apply/reset buttons.

This is now down to nightwatch test failures.So I'll try to resolve them else i will post a detailed description of the problem with the tests and maybe solution😅.

I don't see a ton here to complain about from the code perspective (I haven't manually tested it), but I do have some questions about using styling classes to hook into functional behaviors. That seems like a red flag to me. But maybe I'm missing something?

Found some issues while testing this MR

• Select/unselect a category multiple times and then click Apply. Category is showing multiple times in label.
• Unable to unselect a category from keyboard.
• Clicking Clear does not remove selected categories.
• Unable to select multiple categories after clicking apply.

All the feedbacks are addressed and tests are also passing.Marking it NR.

I have one small UI request. I believe that we need to keep the Apply/Clear buttons visible always as long as the dropdown is shown.

Right now, they are at the very bottom of the scrolling list in the window, and it will not be obvious that someone will need to click that to make the filter work. I would like to see:

[ Dropdown \/ ]
[ Categories here ] - this part is scrollable
[ Apply | Clear ] - Always fixed at the bottom of the popup when someone scrolls

Something like this:

+1 for making the buttons visible all the time. but there is one potentially tricky problem to consider, the user is navigation the multiselect by the up and down arrow key. how should the transition between the options in the select list and the apply and clear button be accomplished? at the moment if you navigate down to the end of the list the apply button gets into focus but you are unable to reach the clear button by the arrow keys. and if you press the tab key the security advisory coverage select gets into focus. on shift focus you get onto the filter button that was unreachable before. but how to handle things for keyboard users with the position fixed of that apply and clear button i consider tricky.

I have added CSS changes that fixes the position of the Apply and Clear button.See the SS:-

@utkarsh_33 With many, many apologies, I think I really beefed this. I tried to rebase, which of course meant force-pushing. I then tried to restore, but I think my restore point was prior to these final fixes. If you still have the latest you did locally, can you please force-push it?

Changes remaining that I want:
- max-height of the dropdown should be "50vh" and not a pixel-based value
- Apply and clear buttons should not be space-between, but right next to each other, with clear being 0.5rem away (get rid of justify-content and replace with "gap: 0.5rem;" should do it.

That's it for aesthetic fixes; but then we need to address the a11y of keyboard navigation. I THINK the right/logical thing would be IF it's open, we should be able to tab to the apply/clear buttons. If they're closed, tab should jump to the next filter. (??? ¯\_(ツ)_/¯ )

in regards of aesthetics two details. first, i am unable to test the current state since it is broken or at least the buttons are not showing anymore at the moment, but in my previous tests i'Ve noticed a regression that is illustrated in #15 📌 Consider adding Apply and Clear buttons to Categories dropdown Active . we ve agreed on a previous issue that the user should always see only the half of the last visible option if there are more options than vertical available space. that way the user directly knows the list is scrollable. in the screenshot in #15 you see the last visible option fully visible instead. that way it is not directly apparent that the list is visible.

and i wonder if it would make sense to align the background color of that box/bar that contains those two buttons with the dark of the bulk action bar? to visualize that this is a button bar and not part of the multi select list anymore, to make it visualy distinguished from the actual list?

and in regards of the keyboard navigation. would it make sense to move the details about the keyboard navigation into 📌 Improve keyboard navigation for categories dropdown Active if the rest of this issue is ready?

I have adjusted the height to 50vh and also the spacing between the buttons as mentioned in #17 📌 Consider adding Apply and Clear buttons to Categories dropdown Active . Also i the concern raise by @rkoller in #18 📌 Consider adding Apply and Clear buttons to Categories dropdown Active is fixed related to

last visible option fully visible

along with the black background colour for the div containing the 2 buttons (which matches the bulk operation) and now it will be easier for the user to distinguish from the actual list.

Also regarding

and in regards of the keyboard navigation. would it make sense to move the details about the keyboard navigation into #3458844: Improve keyboard navigation/general a11y for categories dropdown if the rest of this issue is ready?

I think we could do that as a part of the that issue.

Attaching the latest SS for ease of reviewing the changes.

The tests are failing because they are not able to interact with the elements(by scrolling).I can't figure out how does the addition of one more element in the scroll list is causing this issue.
I tried adjusting the z-index for the apply and clear button's div but that also didn't help to fix the problem because as you can see the css of the apply and clear button's div is causing the issue (see the SS).

For better understanding i changed the background colour.
I also tried to move the apply and clear button out of the select list(which is not actually we want) but that seems to pose a greater problem as you can see the SS.

If anyone has any idea of what could be reason for the test fails or what else can i try to fix the problem, it would be helpful if they can document it on the issue or give it a try to fix the problem.
Thanks in advance.

https://drupal.slack.com/archives/C01UHB4QG12/p1732655176738539

The problem, which I think you know, is that the element is hidden in the scrollable div that's above the buttons, so you can't ->focus() it or ->click() it.

So, I think you would have to somehow figure out how to take that element and scroll it into view. I have tried looking at Mink documentation, and there doesn't seem to be a method to do this. However, I know there's a JavaScript method for it. I think there is a way to execute raw JS during the test, like we do in getElementText() of the ProjectBrowserUiTestTrait.php:

$this->getSession()->evaluateScript(...);

The following diff seems to at least fix the testCategoryFiltering() method, for example.

diff --git a/tests/src/FunctionalJavascript/ProjectBrowserUiTest.php b/tests/src/FunctionalJavascript/ProjectBrowserUiTest.php
index 775c0a5..0ce1360 100644
--- a/tests/src/FunctionalJavascript/ProjectBrowserUiTest.php
+++ b/tests/src/FunctionalJavascript/ProjectBrowserUiTest.php
@@ -131,6 +131,7 @@ class ProjectBrowserUiTest extends WebDriverTestBase {
     $this->clickWithWait('.pb-filter__multi-dropdown', 'E-commerce', TRUE);
 
     // Click 'E-commerce' checkbox.
+    $this->getSession()->evaluateScript('document.getElementById("104").scrollIntoView()');
     $this->clickWithWait('#104', '', TRUE);
     $this->assertNotEmpty($assert_session->waitForButton('Apply'));
     $this->getSession()->getPage()->pressButton('Apply');
@@ -164,9 +165,11 @@ class ProjectBrowserUiTest extends WebDriverTestBase {
     $this->assertSession()->waitForText('Media');
 
     // Click 'Media' checkbox.
+    $this->getSession()->evaluateScript('document.getElementById("67").scrollIntoView()');
     $this->clickWithWait('#67', '', TRUE);
 
     // Click 'E-commerce' checkbox.
+    $this->getSession()->evaluateScript('document.getElementById("104").scrollIntoView()');
     $this->clickWithWait('#104', '', TRUE);
     $this->assertNotEmpty($assert_session->waitForButton('Apply'));
     $this->getSession()->getPage()->pressButton('Apply');

I tried adding the fix

$this->getSession()->evaluateScript('document.getElementById("67").scrollIntoView()');

,but it's not working for all the tests.

a brief note about color contrast, the apply button has a too low color contrast. I would go with the same color suggested on the bulk action issue, --color-blue-400 which is already used on the bulk action bar on admin/content. the general behavior in the context of keyboard navigation i'll retest later today.

I tried to get the other tests pass but they are not working.

No API considerations, moving to stable. HOWEVER, leaving on the SHOULD HAVEs for -beta1.

I wonder if @lostcarpark could turn this part of the testing into a Nightwatch test? I'm not sure if that would actually help or not. Maybe its scrollIntoView works better, or it handles clicking things that aren't visible better...?

This is extremely out of date, sadly, and needs to be completely refactored to be brought in line with the way the Svelte code currently works, with . I think we can certainly write an automated test of this.

Agree this is beyond a rebase. Needs a new branch from 2.0, and for the changes to be reworked to match.