fix: when closeOnTab w/OK button, it should take 2 Tabs to close (#388)

Author: ghiscoding

packages/multiple-select-vanilla/src/MultipleSelectInstance.ts

@@ -278,16 +278,6 @@ export class MultipleSelectInstance {
         'body-click',
       );
     }
-
-    // listen to Tab and call `onBlur` when that happens
-    this._bindEventService.bind(this.dropElm, 'keyup', ((e: KeyboardEvent) => {
-      if (e.code === 'Tab') {
-        this.options.onBlur(e);
-        if (this.options.closeOnTab) {
-          this.close('blur');
-        }
-      }
-    }) as EventListener);
   }
 
   protected initData() {
@@ -1125,22 +1115,26 @@ export class MultipleSelectInstance {
               break;
             }
             case 'Tab': {
-              // when clicking Tab, we'll focus on OK button when available
-              // or with Shift+Tab we'll either focus first option when coming
-              // from OK button or close drop if we're already in the lsit
+              // when clicking "Tab" and we're using a multiple select and we're not focusing on "OK" button yet, we'll do focus to it.
+              // otherwise we'll call the `onBlur` callback and/or `closeOnTab` is enabled we'll do that too.
               e.preventDefault();
-              if (e.shiftKey) {
-                if (document.activeElement === this.okButtonElm) {
-                  this.focusSelectAllOrList();
-                  this.highlightCurrentOption();
-                } else {
-                  this.close('key.shift+tab');
-                  this.choiceElm.focus();
-                }
-              } else {
+
+              // when using multiple select with "OK" button
+              const isMultiple = !this.options.single && this.options.showOkButton;
+              if (isMultiple && e.shiftKey && document.activeElement === this.okButtonElm) {
+                this.focusSelectAllOrList();
+                this.highlightCurrentOption();
+                this.filterParentElm?.querySelector('input')?.focus();
+              } else if (isMultiple && !e.shiftKey && document.activeElement !== this.okButtonElm) {
                 this.changeCurrentOptionHighlight();
                 this.okButtonElm?.focus();
+              } else {
+                this.options.onBlur(e);
+                if (this.options.isOpen && this.options.closeOnTab) {
+                  this.close('blur');
+                }
               }
+
               break;
             }
           }

packages/multiple-select-vanilla/src/models/multipleSelectOption.interface.ts

@@ -73,7 +73,11 @@ export interface MultipleSelectOption extends MultipleSelectLocale {
   /** The class prefix of select. */
   classPrefix?: string;
 
-  /** Should we close the drop when Tab key is pressed. */
+  /**
+   * Should we close the drop when Tab key is pressed.
+   * Side note: when using a multiple select with the "OK" button enabled,
+   * it will require 2 Tabs (1. focus on "OK", 2. close drop)
+   */
   closeOnTab?: boolean;
 
   /** HTML container to use for the drop menu, e.g. 'body' */

playwright/e2e/options25.spec.ts

@@ -28,7 +28,7 @@ test.describe('Options 25 - Show OK Button', () => {
   });
 
   test('Tab and Shift+Tab to switch from OK button to List', async ({ page }) => {
-    // 1st select
+    // 1st Select
     await page.goto('#/options25');
     await page.locator('[data-test=select1].ms-parent').click();
     await page.getByRole('option', { name: 'April' }).click();
@@ -39,14 +39,26 @@ test.describe('Options 25 - Show OK Button', () => {
     await expect(page.locator('div[data-test=select1] .ms-ok-button')).toBeFocused();
     await page.keyboard.press('Shift+Tab');
     await expect(page.locator('div[data-test=select1] .ms-select-all input')).toBeFocused();
-    await page.keyboard.press('Shift+Tab');
+    await page.keyboard.press('Tab');
+    await page.keyboard.press('Enter');
     await expect(page.locator('div[data-test=select1] .ms-drop')).toBeHidden();
 
-    // last select Shift+Tab will close drop
+    // 3rd Select
+    await page.locator('[data-test=select3].ms-parent').click();
+    await page.locator('div:nth-child(2) > label > .icon-checkbox-container').click();
+    await page.keyboard.press('Tab');
+    await expect(page.locator('div[data-test=select3] .ms-ok-button')).toBeFocused();
+    await page.keyboard.press('Shift+Tab');
+    await expect(page.locator('div[data-test=select3] .ms-search input')).toBeFocused();
+    await page.keyboard.press('Tab');
+    await page.keyboard.press('Enter');
+    await expect(page.locator('div[data-test=select3] .ms-drop')).toBeHidden();
+
+    // 4th Select Enter key will close drop
     await page.locator('[data-test=select4].ms-parent').click();
     await page.keyboard.press('ArrowDown');
     await page.keyboard.press('ArrowDown');
-    await page.keyboard.press('Shift+Tab');
+    await page.keyboard.press('Enter');
     await expect(page.locator('div[data-test=select4] .ms-drop')).toBeHidden();
   });
 });

playwright/e2e/options34.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect } from '@playwright/test';
+import { expect, test } from '@playwright/test';
 
 test.describe('Options 34 - Show Search Clear button', () => {
   test('filtering on all type of select & clear search input', async ({ page }) => {

playwright/e2e/options43.spec.ts

@@ -37,8 +37,10 @@ test.describe('Options 43 - Close on Tab', () => {
     // 5th Select
     await page.locator('div[data-test=data1] .ms-choice').click();
     await expect(page.locator('div[data-test=data1] .ms-drop')).toBeVisible();
-    await page.keyboard.press('Tab');
+    await page.keyboard.press('Tab'); // 1st Tab will focus on "OK" button
+    await expect(page.locator('div[data-test=data1] .ms-drop')).toBeVisible();
+    await expect(page.locator('div[data-test=data1] .ms-ok-button')).toBeFocused();
+    await page.keyboard.press('Tab'); // 2nd Tab will close the drop
     await expect(page.locator('div[data-test=data1] .ms-drop')).not.toBeVisible();
-    // });
   });
 });