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 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 diff --git a/core/src/components/reorder-group/readme.md b/core/src/components/reorder-group/readme.md index 53410257e7..689dbc146b 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`. + ## Usage with Virtual Scroll The reorder group requires a scroll container to function. When using a virtual scrolling solution, you will need to disable scrolling on the `ion-content` and indicate which element container is responsible for the scroll container with the `.ion-content-scroll-host` class target. 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', () => {