Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(vue): tabs no longer get unmounted when navigating back to a tabs context #24337

Merged
merged 3 commits into from
Dec 15, 2021
Merged

fix(vue): tabs no longer get unmounted when navigating back to a tabs context #24337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4f3780
fix(vue): do not unmount old tab when navigating back to tabs context
liamdebeasi Dec 9, 2021
86b63c8
chore(): move fix down for better coverage
liamdebeasi Dec 9, 2021
ef6eb07
fix(): add undefined check
liamdebeasi Dec 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion packages/vue/src/components/IonRouterOutlet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import { AnimationBuilder, LIFECYCLE_DID_ENTER, LIFECYCLE_DID_LEAVE, LIFECYCLE_W
import { matchedRouteKey, routeLocationKey, useRoute } from 'vue-router';
import { fireLifecycle, generateId, getConfig } from '../utils';

const isViewVisible = (enteringEl: HTMLElement) => {
return !enteringEl.classList.contains('ion-page-hidden') && !enteringEl.classList.contains('ion-page-invisible');
}

let viewDepthKey: InjectionKey<0> = Symbol(0);
export const IonRouterOutlet = defineComponent({
name: 'IonRouterOutlet',
Expand Down Expand Up @@ -230,13 +234,35 @@ export const IonRouterOutlet = defineComponent({
See https://ionicframework.com/docs/vue/navigation#ionpage for more information.`);
}

if (enteringViewItem === leavingViewItem) return;

if (!leavingViewItem && prevRouteLastPathname) {
leavingViewItem = viewStacks.findViewItemByPathname(prevRouteLastPathname, id, usingDeprecatedRouteSetup);
}

/**
* If the entering view is already
* visible, then no transition is needed.
* This is most common when navigating
* from a tabs page to a non-tabs page
* and then back to the tabs page.
* Even when the tabs context navigated away,
* the inner tabs page was still active.
* This also avoids an issue where
* the previous tabs page is incorrectly
* unmounted since it would automatically
* unmount the previous view.
*
* This should also only apply to entering and
* leaving items in the same router outlet (i.e.
* Tab1 and Tab2), otherwise this will
* return early for swipe to go back when
* going from a non-tabs page to a tabs page.
*/
if (isViewVisible(enteringEl) && leavingViewItem !== undefined && !isViewVisible(leavingViewItem.ionPageElement)) {
return;
}

fireLifecycle(enteringViewItem.vueComponent, enteringViewItem.vueComponentRef, LIFECYCLE_WILL_ENTER);

if (leavingViewItem && enteringViewItem !== leavingViewItem) {
Expand Down
29 changes: 28 additions & 1 deletion packages/vue/test-app/tests/e2e/specs/tabs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ describe('Tabs', () => {
cy.url().should('include', '/tabs/tab1/child-one?key=value');
});

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/23699
// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24353
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from another fix I did, but I noticed I copy and pasted the reference issue incorrectly

it('should handle clicking tab multiple times without query string', () => {
cy.visit('http://localhost:8080/tabs/tab1');

Expand All @@ -329,6 +329,33 @@ describe('Tabs', () => {
cy.ionPageVisible('tab2');
cy.ionPageHidden('tab1');
});

// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24332
it('should not unmount tab 1 when leaving tabs context', () => {
cy.visit('http://localhost:8080/tabs');
cy.ionPageVisible('tab1');

// Dynamically add tab 4 because tab 3 redirects to tab 1
cy.get('#add-tab').click();

cy.get('ion-tab-button#tab-button-tab4').click();
cy.ionPageHidden('tab1');
cy.ionPageVisible('tab4');

cy.get('ion-tab-button#tab-button-tab2').click();
cy.ionPageHidden('tab4');
cy.ionPageVisible('tab2');

cy.get('[data-pageid="tab2"] #routing').click();
cy.ionPageVisible('routing');
cy.ionPageHidden('tabs');

cy.ionBackClick('routing');
cy.ionPageDoesNotExist('routing');
cy.ionPageVisible('tabs');
cy.ionPageVisible('tab2');
cy.ionPageHidden('tab1');
});
})

describe('Tabs - Swipe to Go Back', () => {
Expand Down