fix (picker): fixing pickers tests

Author: joselrio

core/src/components/picker/picker.tsx

@@ -133,63 +133,12 @@ export class Picker implements ComponentInterface {
    * function that has been set in onPointerDown
    * so that we enter/exit input mode correctly.
    */
-  private onClick = (ev: PointerEvent) => {
+  private onClick = () => {
     const { actionOnClick } = this;
     if (actionOnClick) {
       actionOnClick();
       this.actionOnClick = undefined;
     }
-
-    /**
-     * In order to avoid a11y issues we must manage focus
-     * on the picker columns and picker itself.
-     * This is because once picker is clicked we got an issue/warning because
-     * picker input is being focused, and once it has tabindex -1 it can't be focused,
-     * which ends on focusing the picker itself.
-     * During the process above we fall into issues since there is an element
-     * with tabindex -1 and aria-hidden='true' that is focused, which is not allowed.
-     * That said and since onClick is being propagated to the picker itself, we need to
-     * manage focus on the picker columns and picker itself to avoid the issue.
-     */
-    const clickedTarget = ev.target as HTMLElement;
-    let elementToFocus: HTMLElement | null = null;
-
-    switch (clickedTarget.tagName) {
-      case 'ION-PICKER':
-        /**
-         * If the user clicked the picker itself
-         * then we should focus the first picker options
-         * so that users can scroll through them.
-         */
-        const ionPickerColumn = this.el.querySelector('ion-picker-column');
-        elementToFocus = ionPickerColumn?.shadowRoot?.querySelector('.picker-opts') as HTMLElement | null;
-        break;
-
-      case 'ION-PICKER-COLUMN':
-        /**
-         * If the user clicked a picker column
-         * then we should focus its own picker options
-         * so that users can scroll through them.
-         */
-        elementToFocus = clickedTarget.shadowRoot?.querySelector('.picker-opts') as HTMLElement | null;
-        break;
-
-      case 'ION-PICKER-COLUMN-OPTION':
-        /**
-         * If the user clicked a picker column option
-         * then we should focus its picker options parent so that
-         * users can scroll through them.
-         */
-        const ionPickerColumnOption = clickedTarget.closest('ion-picker-column');
-        if (ionPickerColumnOption) {
-          elementToFocus = ionPickerColumnOption.shadowRoot?.querySelector('.picker-opts') as HTMLElement | null;
-        }
-        break;
-    }
-
-    if (elementToFocus) {
-      elementToFocus.focus();
-    }
   };
 
   /**
@@ -461,7 +410,7 @@ export class Picker implements ComponentInterface {
     colEl: HTMLIonPickerColumnElement,
     value: string,
     zeroBehavior: 'start' | 'end' = 'start'
-  ): boolean => {
+  ): boolean => {    
     if (!value) {
       return false;
     }
@@ -588,7 +537,7 @@ export class Picker implements ComponentInterface {
     return (
       <Host
         onPointerDown={(ev: PointerEvent) => this.onPointerDown(ev)}
-        onClick={(ev: PointerEvent) => this.onClick(ev)}
+        onClick={() => this.onClick()}
       >
         <input
           aria-hidden="true"

core/src/components/picker/test/basic/index.html

@@ -70,7 +70,7 @@ <h2>One Numeric Input</h2>
             <h2>Two Numeric Input</h2>
             <ion-picker>
               <ion-picker-column numeric-input="true" id="dual-numeric-first"></ion-picker-column>
-              <ion-picker-column numeric-input="true" id="dual-numeric-second"></ion-picker-column>
+              <ion-picker-column numeric-input="true" id="dual-numeric-second" ></ion-picker-column>
             </ion-picker>
           </div>
           <div class="grid-item">
@@ -177,6 +177,10 @@ <h2>Modal</h2>
         'onion'
       );
 
+      const columnDualNumericFirst = document.querySelector('ion-picker-column#dual-numeric-first');
+      columnDualNumericFirst.addEventListener('ionChange', (ev) => {
+        console.log('Column change', ev.detail);
+      });
       setPickerColumn(
         '#dual-numeric-first',
         [
@@ -195,6 +199,10 @@ <h2>Modal</h2>
         ],
         3
       );
+      const columnDualNumericSecond = document.querySelector('ion-picker-column#dual-numeric-second');
+      columnDualNumericSecond.addEventListener('ionChange', (ev) => {
+        console.log('Column change', ev.detail);
+      });
       setPickerColumn('#dual-numeric-second', minutes, 3);
 
       setPickerColumn(

core/src/components/picker/test/basic/picker.e2e.ts

@@ -106,32 +106,44 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
     });
 
     test('tabbing should correctly move focus between columns', async ({ page }) => {
-      const firstColumn = page.locator('ion-picker-column#first');
-      const secondColumn = page.locator('ion-picker-column#second');
+      const firstColumn = await page.evaluate(() => document.querySelector('ion-picker-column#first'));
+      const secondColumn = await page.evaluate(() => document.querySelector('ion-picker-column#second'));
+
 
       // Focus first column
       await page.keyboard.press('Tab');
-      await expect(firstColumn).toBeFocused();
+    
+      let activeElement = await page.evaluate(() => document.activeElement);
+      expect(activeElement).toEqual(firstColumn);
 
       await page.waitForChanges();
 
       // Focus second column
       await page.keyboard.press('Tab');
-      await expect(secondColumn).toBeFocused();
+
+      activeElement = await page.evaluate(() => document.activeElement);
+      expect(activeElement).toEqual(secondColumn);
     });
 
     test('tabbing should correctly move focus back', async ({ page }) => {
-      const firstColumn = page.locator('ion-picker-column#first');
-      const secondColumn = page.locator('ion-picker-column#second');
+      const firstColumn = await page.evaluate(() => document.querySelector('ion-picker-column#first'));
+      const secondColumn = await page.evaluate(() => document.querySelector('ion-picker-column#second'));
 
-      await secondColumn.evaluate((el: HTMLIonPickerColumnElement) => el.setFocus());
-      await expect(secondColumn).toBeFocused();
+      await page.evaluate((selector) => {
+        const el = document.querySelector(selector) as HTMLElement | null;
+        el?.focus();
+      }, 'ion-picker-column#second');
+
+      let activeElement = await page.evaluate(() => document.activeElement);
+      expect(activeElement).toEqual(secondColumn);
 
       await page.waitForChanges();
 
       // Focus first column
       await page.keyboard.press('Shift+Tab');
-      await expect(firstColumn).toBeFocused();
+
+      activeElement = await page.evaluate(() => document.activeElement);
+      expect(activeElement).toEqual(firstColumn);
     });
   });
 });

core/src/components/picker/test/keyboard-entry/picker.e2e.ts

@@ -35,16 +35,21 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
         </script>
       `,
         config
-      );
-
+      );      
+      
       const column = page.locator('ion-picker-column');
+  
+      const colShadowRoot = await column.evaluateHandle((el) => el.shadowRoot);
+      const columnPickerOpts = await colShadowRoot.evaluateHandle((root) => root?.querySelector('.picker-opts'));
+      
       const ionChange = await page.spyOnEvent('ionChange');
-      await column.evaluate((el: HTMLIonPickerColumnElement) => el.setFocus());
-
+      await columnPickerOpts.evaluate((el) => el && (el as HTMLElement).focus());
+      
       await page.keyboard.press('Digit2');
-
+  
       await expect(ionChange).toHaveReceivedEventDetail({ value: 2 });
       await expect(column).toHaveJSProperty('value', 2);
+      
     });
 
     test('should scroll to and update the value prop for multiple columns', async ({ page }) => {
@@ -99,22 +104,24 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
       );
       const firstColumn = page.locator('ion-picker-column#first');
       const secondColumn = page.locator('ion-picker-column#second');
-      const highlight = page.locator('ion-picker .picker-highlight');
       const firstIonChange = await (firstColumn as E2ELocator).spyOnEvent('ionChange');
       const secondIonChange = await (secondColumn as E2ELocator).spyOnEvent('ionChange');
 
-      const box = await highlight.boundingBox();
-      if (box !== null) {
-        await page.mouse.click(box.x + box.width / 2, box.y + box.height / 2);
-      }
-
-      await expect(firstColumn).toHaveClass(/picker-column-active/);
-      await expect(secondColumn).toHaveClass(/picker-column-active/);
+      const firstColShadowRoot = await firstColumn.evaluateHandle((el) => el.shadowRoot);
+      const columnPickerOpts = await firstColShadowRoot.evaluateHandle((root) => root?.querySelector('.picker-opts'));
+      
+      // Focus first column
+      await columnPickerOpts.evaluate((el) => el && (el as HTMLElement).focus());
 
       await page.keyboard.press('Digit2');
 
       await expect(firstIonChange).toHaveReceivedEventDetail({ value: 2 });
       await expect(firstColumn).toHaveJSProperty('value', 2);
+      
+      // Focus second column
+      await page.keyboard.press('Tab');
+
+      await page.waitForChanges();
 
       await page.keyboard.press('Digit2+Digit4');
 
@@ -155,8 +162,12 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
       );
 
       const column = page.locator('ion-picker-column');
+
+      const colShadowRoot = await column.evaluateHandle((el) => el.shadowRoot);
+      const columnPickerOpts = await colShadowRoot.evaluateHandle((root) => root?.querySelector('.picker-opts'));
+      
       const ionChange = await page.spyOnEvent('ionChange');
-      await column.evaluate((el: HTMLIonPickerColumnElement) => el.setFocus());
+      await columnPickerOpts.evaluate((el) => el && (el as HTMLElement).focus());
 
       await page.keyboard.press('Digit0');