From e4387178d92a56faf812ed53b150a369ad56009a Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Thu, 21 Nov 2024 10:46:52 -0800 Subject: [PATCH 1/6] fix(menu): make sure Talkback focus rings are displayed --- core/src/utils/menu-controller/animations/overlay.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/utils/menu-controller/animations/overlay.ts b/core/src/utils/menu-controller/animations/overlay.ts index b7ae84b317e..d1693879295 100644 --- a/core/src/utils/menu-controller/animations/overlay.ts +++ b/core/src/utils/menu-controller/animations/overlay.ts @@ -27,6 +27,7 @@ export const menuOverlayAnimation = (menu: MenuI): Animation => { openedX = '0px'; } + menu.menuInnerEl!.style.willChange = 'transform'; menuAnimation.addElement(menu.menuInnerEl!).fromTo('transform', `translateX(${closedX})`, `translateX(${openedX})`); const mode = getIonMode(menu); From 0a626041fabcf5679eb1a37d72ac0d35a15560a8 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Thu, 21 Nov 2024 11:43:33 -0800 Subject: [PATCH 2/6] refactor(menu): move will-change --- core/src/components/menu/menu.scss | 2 ++ core/src/utils/menu-controller/animations/overlay.ts | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/components/menu/menu.scss b/core/src/components/menu/menu.scss index 86b522b50c0..c6e9e7290c2 100644 --- a/core/src/components/menu/menu.scss +++ b/core/src/components/menu/menu.scss @@ -39,6 +39,8 @@ .menu-inner { @include transform(translateX(-9999px)); + will-change: transform; + display: flex; position: absolute; diff --git a/core/src/utils/menu-controller/animations/overlay.ts b/core/src/utils/menu-controller/animations/overlay.ts index d1693879295..b7ae84b317e 100644 --- a/core/src/utils/menu-controller/animations/overlay.ts +++ b/core/src/utils/menu-controller/animations/overlay.ts @@ -27,7 +27,6 @@ export const menuOverlayAnimation = (menu: MenuI): Animation => { openedX = '0px'; } - menu.menuInnerEl!.style.willChange = 'transform'; menuAnimation.addElement(menu.menuInnerEl!).fromTo('transform', `translateX(${closedX})`, `translateX(${openedX})`); const mode = getIonMode(menu); From b9a256c0915c690a534a5f41cccb43e577ad508b Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Thu, 21 Nov 2024 12:12:12 -0800 Subject: [PATCH 3/6] refactor(menu): use left/right instead --- core/src/components/menu/menu.scss | 9 +++++---- core/src/utils/menu-controller/animations/overlay.ts | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/core/src/components/menu/menu.scss b/core/src/components/menu/menu.scss index c6e9e7290c2..56fdf14bdc2 100644 --- a/core/src/components/menu/menu.scss +++ b/core/src/components/menu/menu.scss @@ -37,12 +37,11 @@ .menu-inner { - @include transform(translateX(-9999px)); - - will-change: transform; - display: flex; position: absolute; + right: unset; + /* stylelint-disable property-disallowed-list */ + left: -100%; /* Hide offscreen for left-sided menu */ flex-direction: column; justify-content: space-between; @@ -55,6 +54,8 @@ min-height: var(--min-height); max-height: var(--max-height); + transform: none; /* Avoid using transform */ + background: var(--background); contain: strict; diff --git a/core/src/utils/menu-controller/animations/overlay.ts b/core/src/utils/menu-controller/animations/overlay.ts index b7ae84b317e..cc128e69c17 100644 --- a/core/src/utils/menu-controller/animations/overlay.ts +++ b/core/src/utils/menu-controller/animations/overlay.ts @@ -27,7 +27,7 @@ export const menuOverlayAnimation = (menu: MenuI): Animation => { openedX = '0px'; } - menuAnimation.addElement(menu.menuInnerEl!).fromTo('transform', `translateX(${closedX})`, `translateX(${openedX})`); + menuAnimation.addElement(menu.menuInnerEl!).fromTo('left', `${closedX}`, `${openedX}`); const mode = getIonMode(menu); const isIos = mode === 'ios'; From dec59cfda5d042ed4f61344c9a856bc2aef95f92 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Thu, 21 Nov 2024 16:26:44 -0800 Subject: [PATCH 4/6] refactor(menu): hide from screen readers while animating --- core/src/components/menu/menu.scss | 7 ++----- core/src/components/menu/menu.tsx | 8 ++++++++ core/src/utils/menu-controller/animations/overlay.ts | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/core/src/components/menu/menu.scss b/core/src/components/menu/menu.scss index 56fdf14bdc2..86b522b50c0 100644 --- a/core/src/components/menu/menu.scss +++ b/core/src/components/menu/menu.scss @@ -37,11 +37,10 @@ .menu-inner { + @include transform(translateX(-9999px)); + display: flex; position: absolute; - right: unset; - /* stylelint-disable property-disallowed-list */ - left: -100%; /* Hide offscreen for left-sided menu */ flex-direction: column; justify-content: space-between; @@ -54,8 +53,6 @@ min-height: var(--min-height); max-height: var(--max-height); - transform: none; /* Avoid using transform */ - background: var(--background); contain: strict; diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index d83e53abb39..6614cd3392c 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -631,6 +631,9 @@ export class Menu implements ComponentInterface, MenuI { private beforeAnimation(shouldOpen: boolean, role?: string) { assert(!this.isAnimating, '_before() should not be called while animating'); + // add aria-hidden to the menu + this.el.setAttribute('aria-hidden', 'true'); + // this places the menu into the correct location before it animates in // this css class doesn't actually kick off any animations this.el.classList.add(SHOW_MENU); @@ -690,6 +693,9 @@ export class Menu implements ComponentInterface, MenuI { // emit open event this.ionDidOpen.emit(); + // remove aria-hidden from menu + this.el.removeAttribute('aria-hidden'); + /** * Move focus to the menu to prepare focus trapping, as long as * it isn't already focused. Use the host element instead of the @@ -703,6 +709,8 @@ export class Menu implements ComponentInterface, MenuI { // start focus trapping document.addEventListener('focus', this.handleFocus, true); } else { + this.el.removeAttribute('aria-hidden'); + // remove css classes and unhide content from screen readers this.el.classList.remove(SHOW_MENU); diff --git a/core/src/utils/menu-controller/animations/overlay.ts b/core/src/utils/menu-controller/animations/overlay.ts index cc128e69c17..b7ae84b317e 100644 --- a/core/src/utils/menu-controller/animations/overlay.ts +++ b/core/src/utils/menu-controller/animations/overlay.ts @@ -27,7 +27,7 @@ export const menuOverlayAnimation = (menu: MenuI): Animation => { openedX = '0px'; } - menuAnimation.addElement(menu.menuInnerEl!).fromTo('left', `${closedX}`, `${openedX}`); + menuAnimation.addElement(menu.menuInnerEl!).fromTo('transform', `translateX(${closedX})`, `translateX(${openedX})`); const mode = getIonMode(menu); const isIos = mode === 'ios'; From 5f69f6a079d80bf626c4470ed0f54ddcdb6eb919 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Fri, 22 Nov 2024 11:25:57 -0800 Subject: [PATCH 5/6] refactor(menu): add comments --- core/src/components/menu/menu.tsx | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 6614cd3392c..953c7d61907 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -8,6 +8,7 @@ import type { Attributes } from '@utils/helpers'; import { inheritAriaAttributes, assert, clamp, isEndSide as isEnd } from '@utils/helpers'; import { menuController } from '@utils/menu-controller'; import { BACKDROP, GESTURE, getPresentedOverlay } from '@utils/overlays'; +import { isPlatform } from '@utils/platform'; import { hostContext } from '@utils/theme'; import { config } from '../../global/config'; @@ -631,8 +632,22 @@ export class Menu implements ComponentInterface, MenuI { private beforeAnimation(shouldOpen: boolean, role?: string) { assert(!this.isAnimating, '_before() should not be called while animating'); - // add aria-hidden to the menu - this.el.setAttribute('aria-hidden', 'true'); + /** + * When the menu is presented on an Android device, TalkBack's focus rings + * may appear in the wrong position due to the transition (specifically + * `transform` styles). This occurs because the focus rings are initially + * displayed at the starting position of the elements before the transition + * begins. This workaround ensures the focus rings do not appear in the + * incorrect location. + * + * If this solution is applied to iOS devices, then it leads to a bug where + * the overlays cannot be accessed by screen readers. This is due to + * VoiceOver not being able to update the accessibility tree when the + * `aria-hidden` is removed. + */ + if (isPlatform('android')) { + this.el.setAttribute('aria-hidden', 'true'); + } // this places the menu into the correct location before it animates in // this css class doesn't actually kick off any animations @@ -690,12 +705,18 @@ export class Menu implements ComponentInterface, MenuI { } if (isOpen) { + /** + * When the menu is presented on an Android device, TalkBack's focus rings + * may appear in the wrong position due to the transition (specifically + * `transform` styles). The menu is hidden from screen readers during the + * transition to prevent this. Once the transition is complete, the menu + * is shown again. + */ + this.el.removeAttribute('aria-hidden'); + // emit open event this.ionDidOpen.emit(); - // remove aria-hidden from menu - this.el.removeAttribute('aria-hidden'); - /** * Move focus to the menu to prepare focus trapping, as long as * it isn't already focused. Use the host element instead of the From 9d90e1c39b7cf42af36437e939c2fd743860fc16 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Fri, 22 Nov 2024 11:29:27 -0800 Subject: [PATCH 6/6] refactor(menu, overlays): update comments and add if --- core/src/components/menu/menu.tsx | 4 +++- core/src/utils/overlays.ts | 11 ++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index 953c7d61907..8a70dd8fb1d 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -712,7 +712,9 @@ export class Menu implements ComponentInterface, MenuI { * transition to prevent this. Once the transition is complete, the menu * is shown again. */ - this.el.removeAttribute('aria-hidden'); + if (isPlatform('android')) { + this.el.removeAttribute('aria-hidden'); + } // emit open event this.ionDidOpen.emit(); diff --git a/core/src/utils/overlays.ts b/core/src/utils/overlays.ts index 25a6597d1ad..90a4062840e 100644 --- a/core/src/utils/overlays.ts +++ b/core/src/utils/overlays.ts @@ -971,11 +971,12 @@ export const createTriggerController = () => { * like TalkBack do not announce or interact with the content until the * animation is complete, avoiding confusion for users. * - * If the overlay is being presented, it prevents focus rings from appearing - * in incorrect positions due to the transition (specifically `transform` - * styles), ensuring that when aria-hidden is removed, the focus rings are - * correctly displayed in the final location of the elements. This only - * applies to Android devices. + * When the overlay is presented on an Android device, TalkBack's focus rings + * may appear in the wrong position due to the transition (specifically + * `transform` styles). This occurs because the focus rings are initially + * displayed at the starting position of the elements before the transition + * begins. This workaround ensures the focus rings do not appear in the + * incorrect location. * * If this solution is applied to iOS devices, then it leads to a bug where * the overlays cannot be accessed by screen readers. This is due to