From 3f3a2bcfcef93d7f7bdbda0522a21a3a9cd76459 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Fri, 8 Apr 2022 13:06:02 -0400 Subject: [PATCH 1/4] docs(reorder): improve docs on how to use complete method (#25086) --- core/src/components/reorder-group/readme.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core/src/components/reorder-group/readme.md b/core/src/components/reorder-group/readme.md index 1708ac5941..081120d642 100644 --- a/core/src/components/reorder-group/readme.md +++ b/core/src/components/reorder-group/readme.md @@ -6,6 +6,16 @@ Once the user drags an item and drops it in a new position, the `ionItemReorder` The `detail` property of the `ionItemReorder` event includes all of the relevant information about the reorder operation, including the `from` and `to` indexes. In the context of reordering, an item moves `from` an index `to` a new index. +## Completing a Reorder + +When the `ionItemReorder` event is dispatched, developers have the option to call the `complete()` method on `ion-reorder-group`. This will complete the reorder operation. + +By default, the `complete()` method will re-order the DOM nodes inside of `ion-reorder-group`. + +For developers who need to sort an array based on the order of the items in `ion-reorder-group`, we recommend passing the array as a parameter in `complete()`. Ionic will sort and return the array so that it can be reassigned. + +In some cases, it may be necessary for an app to re-order both the array and the DOM nodes on its own. When this happens, it is recommended to pass `false` to the `complete()` method. This will prevent Ionic from re-ordering any DOM nodes inside of `ion-reorder-group`. + ## Interfaces ### ItemReorderEventDetail From da896848776105ba1f7035c4412495786199bade Mon Sep 17 00:00:00 2001 From: Amanda Smith <90629384+amandaesmith3@users.noreply.github.com> Date: Fri, 8 Apr 2022 16:01:08 -0500 Subject: [PATCH 2/4] fix(menu): preserve scroll position when focusing on open (#25044) --- core/src/components/menu/menu.tsx | 14 ++++++--- core/src/components/menu/test/basic/e2e.ts | 31 ++++++++++++++++++- .../src/components/menu/test/basic/index.html | 15 +++++++++ .../components/menu/test/focus-trap/e2e.ts | 4 +-- .../menu/test/focus-trap/index.html | 2 +- 5 files changed, 58 insertions(+), 8 deletions(-) diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index e0d4091082..c30108dcf5 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -398,6 +398,7 @@ export class Menu implements ComponentInterface, MenuI { } this.beforeAnimation(shouldOpen); + await this.loadAnimation(); await this.startAnimation(shouldOpen, animated); this.afterAnimation(shouldOpen); @@ -619,12 +620,17 @@ export class Menu implements ComponentInterface, MenuI { // emit open event this.ionDidOpen.emit(); - // focus menu content for screen readers - if (this.menuInnerEl) { - this.focusFirstDescendant(); + /** + * Move focus to the menu to prepare focus trapping, as long as + * it isn't already focused. Use the host element instead of the + * first descendant to avoid the scroll position jumping around. + */ + const focusedMenu = document.activeElement?.closest('ion-menu'); + if (focusedMenu !== this.el) { + this.el.focus(); } - // setup focus trapping + // start focus trapping document.addEventListener('focus', this.handleFocus, true); } else { // remove css classes and unhide content from screen readers diff --git a/core/src/components/menu/test/basic/e2e.ts b/core/src/components/menu/test/basic/e2e.ts index 171d4cb590..dd83dfeacf 100644 --- a/core/src/components/menu/test/basic/e2e.ts +++ b/core/src/components/menu/test/basic/e2e.ts @@ -28,11 +28,40 @@ test('menu: focus trap', async () => { await menu.waitForVisible(); let activeElID = await getActiveElementID(page); - expect(activeElID).toEqual('start-menu-button'); + expect(activeElID).toEqual('start-menu'); await page.keyboard.press('Tab'); activeElID = await getActiveElementID(page); expect(activeElID).toEqual('start-menu-button'); + + // do it again to make sure focus stays inside menu + await page.keyboard.press('Tab'); + activeElID = await getActiveElementID(page); + expect(activeElID).toEqual('start-menu-button'); +}); + +test('menu: preserve scroll position', async () => { + const page = await newE2EPage({ url: '/src/components/menu/test/basic?ionic:_testing=true' }); + + await page.click('#open-first'); + const menu = await page.find('#start-menu'); + await menu.waitForVisible(); + + await page.$eval('#start-menu ion-content', (menuContentEl: any) => { + return menuContentEl.scrollToPoint(0, 200); + }); + + await menu.callMethod('close'); + + await page.click('#open-first'); + await menu.waitForVisible(); + + const scrollTop = await page.$eval('#start-menu ion-content', async (menuContentEl: any) => { + const contentScrollEl = await menuContentEl.getScrollElement(); + return contentScrollEl.scrollTop; + }); + + expect(scrollTop).toEqual(200); }); /** diff --git a/core/src/components/menu/test/basic/index.html b/core/src/components/menu/test/basic/index.html index 6012e3a119..722be74614 100644 --- a/core/src/components/menu/test/basic/index.html +++ b/core/src/components/menu/test/basic/index.html @@ -49,6 +49,21 @@ Menu Item Menu Item Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item + Menu Item diff --git a/core/src/components/menu/test/focus-trap/e2e.ts b/core/src/components/menu/test/focus-trap/e2e.ts index 0c06348292..af0a01211e 100644 --- a/core/src/components/menu/test/focus-trap/e2e.ts +++ b/core/src/components/menu/test/focus-trap/e2e.ts @@ -18,7 +18,7 @@ test('menu: focus trap with overlays', async () => { await menu.callMethod('open'); await ionDidOpen.next(); - expect(await getActiveElementID(page)).toEqual('open-modal-button'); + expect(await getActiveElementID(page)).toEqual('menu'); const openModal = await page.find('#open-modal-button'); await openModal.click(); @@ -45,7 +45,7 @@ test('menu: focus trap with content inside overlays', async () => { await menu.callMethod('open'); await ionDidOpen.next(); - expect(await getActiveElementID(page)).toEqual('open-modal-button'); + expect(await getActiveElementID(page)).toEqual('menu'); const openModal = await page.find('#open-modal-button'); await openModal.click(); diff --git a/core/src/components/menu/test/focus-trap/index.html b/core/src/components/menu/test/focus-trap/index.html index e07ba0c0ff..0611466001 100644 --- a/core/src/components/menu/test/focus-trap/index.html +++ b/core/src/components/menu/test/focus-trap/index.html @@ -14,7 +14,7 @@ - + Menu From a0054a7cbd52def24c18fd2dadfd2e49a42b8980 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 12 Apr 2022 10:22:36 -0400 Subject: [PATCH 3/4] fix(vue): ensure that only tab pages get added to the tab navigation stack (#25045) resolves #24859 --- packages/vue-router/src/router.ts | 25 +++++++++------ packages/vue/test-app/tests/e2e/specs/tabs.js | 32 ++++++++++++++++++- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/packages/vue-router/src/router.ts b/packages/vue-router/src/router.ts index 17ded9b25b..81b95ced7e 100644 --- a/packages/vue-router/src/router.ts +++ b/packages/vue-router/src/router.ts @@ -65,7 +65,6 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) => let currentRouteInfo: RouteInfo; let incomingRouteParams: RouteParams; - let currentTab: string | undefined; // TODO types let historyChangeListeners: any[] = []; @@ -238,8 +237,7 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) => if (action === 'replace') { incomingRouteParams = { routerAction: 'replace', - routerDirection: 'none', - tab: currentTab + routerDirection: 'none' } } else if (action === 'pop') { const routeInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition - delta); @@ -254,16 +252,15 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) => } else { incomingRouteParams = { routerAction: 'pop', - routerDirection: 'none', - tab: currentTab + routerDirection: 'none' } } } + if (!incomingRouteParams) { incomingRouteParams = { routerAction: 'push', - routerDirection: direction || 'forward', - tab: currentTab + routerDirection: direction || 'forward' } } } @@ -288,7 +285,6 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) => } if (isPushed) { - routeInfo.tab = leavingLocationInfo.tab; routeInfo.pushedByRoute = (leavingLocationInfo.pathname !== '') ? leavingLocationInfo.pathname : undefined; } else if (routeInfo.routerAction === 'pop') { const route = locationHistory.findLastLocation(routeInfo); @@ -460,9 +456,18 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) => } } + /** + * This method is invoked by the IonTabs component + * during a history change callback. It is responsible + * for ensuring that tabbed routes have the correct + * "tab" field in its routeInfo object. + * + * IonTabs will determine if the current route + * is in tabs and assign it the correct tab. + * If the current route is not in tabs, + * then IonTabs will not invoke this. + */ const handleSetCurrentTab = (tab: string) => { - currentTab = tab; - const ri = { ...locationHistory.last() }; if (ri.tab !== tab) { ri.tab = tab; diff --git a/packages/vue/test-app/tests/e2e/specs/tabs.js b/packages/vue/test-app/tests/e2e/specs/tabs.js index e00cc50b8b..ba31e949cd 100644 --- a/packages/vue/test-app/tests/e2e/specs/tabs.js +++ b/packages/vue/test-app/tests/e2e/specs/tabs.js @@ -435,7 +435,7 @@ describe('Tabs', () => { }); // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24432 - it('should correct replace a route in a child tab route', () => { + it('should correctly replace a route in a child tab route', () => { cy.visit('http://localhost:8080/tabs'); cy.routerPush('/tabs/tab1/childone'); @@ -446,6 +446,36 @@ describe('Tabs', () => { cy.ionPageVisible('tab1'); cy.ionPageDoesNotExist('tab1childone'); }) + + // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24859 + it('should go back to the root page after navigating between tab and non tab outlets', () => { + cy.visit('http://localhost:8080'); + + cy.routerPush('/tabs/tab1'); + cy.ionPageVisible('tab1'); + cy.ionPageHidden('home'); + + cy.get('ion-tab-button#tab-button-tab2').click(); + cy.ionPageHidden('tab1'); + cy.ionPageVisible('tab2'); + + cy.routerPush('/routing'); + cy.ionPageVisible('routing'); + cy.ionPageHidden('tabs'); + + cy.routerPush('/tabs/tab1'); + cy.ionPageVisible('tabs'); + cy.ionPageVisible('tab1'); + cy.ionPageHidden('routing'); + + cy.get('ion-tab-button#tab-button-tab2').click(); + cy.ionPageHidden('tab1'); + cy.ionPageVisible('tab2'); + + cy.ionBackClick('tab2'); + cy.ionPageVisible('home'); + cy.ionPageDoesNotExist('tabs'); + }); }) describe('Tabs - Swipe to Go Back', () => { From e1b555f286956574876924068304fc44a78c027c Mon Sep 17 00:00:00 2001 From: Amanda Smith <90629384+amandaesmith3@users.noreply.github.com> Date: Tue, 12 Apr 2022 13:57:18 -0500 Subject: [PATCH 4/4] fix(accordion-group): only allow keyboard interaction if header is focused (#25091) --- .../accordion-group/accordion-group.tsx | 11 +++++++++++ core/src/components/accordion/test/a11y/e2e.ts | 18 ++++++++++++++++++ .../components/accordion/test/a11y/index.html | 2 +- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/core/src/components/accordion-group/accordion-group.tsx b/core/src/components/accordion-group/accordion-group.tsx index fd00f64c9a..8d5c9596b0 100644 --- a/core/src/components/accordion-group/accordion-group.tsx +++ b/core/src/components/accordion-group/accordion-group.tsx @@ -101,6 +101,17 @@ export class AccordionGroup implements ComponentInterface { return; } + /** + * Make sure focus is in the header, not the body, of the accordion. This ensures + * that if there are any interactable elements in the body, their keyboard + * interaction doesn't get stolen by the accordion. Example: using up/down keys + * in ion-textarea. + */ + const activeAccordionHeader = activeElement.closest('ion-accordion [slot="header"]'); + if (!activeAccordionHeader) { + return; + } + const accordionEl = activeElement.tagName === 'ION-ACCORDION' ? activeElement : activeElement.closest('ion-accordion'); if (!accordionEl) { diff --git a/core/src/components/accordion/test/a11y/e2e.ts b/core/src/components/accordion/test/a11y/e2e.ts index 1913f017a6..f25141b93a 100644 --- a/core/src/components/accordion/test/a11y/e2e.ts +++ b/core/src/components/accordion/test/a11y/e2e.ts @@ -5,6 +5,11 @@ const getActiveElementText = async (page) => { return page.evaluate((el) => el?.innerText, activeElement); }; +const getActiveInputID = async (page) => { + const activeElement = await page.evaluateHandle(() => document.activeElement); + return page.evaluate((el) => el?.closest('ion-input')?.id, activeElement); +}; + test('accordion: a11y', async () => { const page = await newE2EPage({ url: '/src/components/accordion/test/a11y?ionic:_testing=true', @@ -42,4 +47,17 @@ test('accordion: keyboard navigation', async () => { await page.keyboard.press('ArrowUp'); expect(await getActiveElementText(page)).toEqual('Shipping Address'); + + // open Shipping Address accordion and move focus to the input inside it + await page.keyboard.press('Enter'); + await page.waitForChanges(); + await page.keyboard.press('Tab'); + + const activeID = await getActiveInputID(page); + expect(activeID).toEqual('address1'); + + // ensure keyboard interaction doesn't move focus from body + await page.keyboard.press('ArrowDown'); + const activeIDAgain = await getActiveInputID(page); + expect(activeIDAgain).toEqual('address1'); }); diff --git a/core/src/components/accordion/test/a11y/index.html b/core/src/components/accordion/test/a11y/index.html index 486660cfe6..0c660ef8dd 100644 --- a/core/src/components/accordion/test/a11y/index.html +++ b/core/src/components/accordion/test/a11y/index.html @@ -86,7 +86,7 @@ Address 1 - + Address 2