From 0a9091b5d86476410ba9473bc1b256fcfd8881e8 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 11 Nov 2020 13:39:08 -0500 Subject: [PATCH 1/4] fire tabs lifecycle evnets properly --- packages/vue/src/components/IonTabBar.ts | 12 +- packages/vue/src/components/IonTabs.ts | 19 +- packages/vue/test-app/tests/unit/tabs.spec.ts | 192 ++++++++++++++++++ 3 files changed, 214 insertions(+), 9 deletions(-) create mode 100644 packages/vue/test-app/tests/unit/tabs.spec.ts diff --git a/packages/vue/src/components/IonTabBar.ts b/packages/vue/src/components/IonTabBar.ts index 682c3664831..343fab15585 100644 --- a/packages/vue/src/components/IonTabBar.ts +++ b/packages/vue/src/components/IonTabBar.ts @@ -13,6 +13,10 @@ interface Tab { export const IonTabBar = defineComponent({ name: 'IonTabBar', + props: { + _tabsWillChange: { type: Function, default: () => {} }, + _tabsDidChange: { type: Function, default: () => {} } + }, mounted() { const ionRouter: any = inject('navManager'); const tabState: TabState = { @@ -102,12 +106,16 @@ export const IonTabBar = defineComponent({ } } - const activeChild = childNodes.find((child: VNode) => child.el.tab === activeTab); + const activeChild = childNodes.find((child: VNode) => child.props.tab === activeTab); const tabBar = this.$refs.ionTabBar; - + const tabDidChange = activeTab !== prevActiveTab; if (activeChild && tabBar) { + tabDidChange && this.$props._tabsWillChange(activeTab); + ionRouter.handleSetCurrentTab(activeTab); tabBar.selectedTab = tabState.activeTab = activeTab; + + tabDidChange && this.$props._tabsDidChange(activeTab); } }; diff --git a/packages/vue/src/components/IonTabs.ts b/packages/vue/src/components/IonTabs.ts index 9d4df7ae11c..2871aef344e 100644 --- a/packages/vue/src/components/IonTabs.ts +++ b/packages/vue/src/components/IonTabs.ts @@ -1,10 +1,14 @@ import { h, defineComponent, VNode } from 'vue'; import { IonRouterOutlet } from './IonRouterOutlet'; +const WILL_CHANGE = 'ionTabsWillChange'; +const DID_CHANGE = 'ionTabsDidChange'; + export const IonTabs = defineComponent({ name: 'IonTabs', + emits: [WILL_CHANGE, DID_CHANGE], render() { - const { $slots: slots } = this; + const { $slots: slots, $emit } = this; const slottedContent = slots.default && slots.default(); let childrenToRender = [ h('div', { @@ -25,14 +29,15 @@ export const IonTabs = defineComponent({ * not show above the tab content. */ if (slottedContent && slottedContent.length > 0) { - const topSlottedTabBar = slottedContent.find((child: VNode) => { - const isTabBar = child.type && (child.type as any).name === 'IonTabBar'; - const hasTopSlot = child.props?.slot === 'top'; + const slottedTabBar = slottedContent.find((child: VNode) => child.type && (child.type as any).name === 'IonTabBar'); + const hasTopSlot = slottedTabBar && slottedTabBar.props?.slot === 'top'; - return isTabBar && hasTopSlot; - }); + if (slottedTabBar) { + slottedTabBar.props._tabsWillChange = (tab: string) => $emit(WILL_CHANGE, { tab }); + slottedTabBar.props._tabsDidChange = (tab: string) => $emit(DID_CHANGE, { tab }); + } - if (topSlottedTabBar) { + if (hasTopSlot) { childrenToRender = [ ...slottedContent, ...childrenToRender diff --git a/packages/vue/test-app/tests/unit/tabs.spec.ts b/packages/vue/test-app/tests/unit/tabs.spec.ts new file mode 100644 index 00000000000..8bdf0f85f61 --- /dev/null +++ b/packages/vue/test-app/tests/unit/tabs.spec.ts @@ -0,0 +1,192 @@ +import { mount, flushPromises } from '@vue/test-utils'; +import { createRouter, createWebHistory } from '@ionic/vue-router'; +import { IonicVue, IonApp, IonRouterOutlet, IonPage, IonTabs, IonTabBar, IonTabButton, IonLabel } from '@ionic/vue'; + +const App = { + components: { IonApp, IonRouterOutlet }, + template: '', +} + +const Tabs = { + components: { IonPage, IonTabs, IonTabBar, IonTabButton, IonLabel }, + template: ` + + + + + Tab 1 + + + + Tab 2 + + + + + `, +} + +const Tab1 = { + components: { IonPage }, + template: `Tab 1` +} +const Tab2 = { + components: { IonPage }, + template: `Tab 2` +} + +describe('ion-tabs', () => { + it('should emit will change and did change events when changing tab', async () => { + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { + path: '/', + component: Tabs, + children: [ + { + path: '', + redirect: 'tab1' + }, + { + path: 'tab1', + component: Tab1, + }, + { + path: 'tab2', + component: Tab2 + } + ] + } + ] + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(App, { + global: { + plugins: [router, IonicVue] + } + }); + + const tabs = wrapper.findComponent(IonTabs); + expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); + expect(tabs.emitted().ionTabsWillChange[0]).toEqual([{ tab: 'tab1' }]); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); + expect(tabs.emitted().ionTabsDidChange[0]).toEqual([{ tab: 'tab1' }]); + + router.push('/tab2') + await flushPromises() + + expect(tabs.emitted().ionTabsWillChange.length).toEqual(2); + expect(tabs.emitted().ionTabsWillChange[1]).toEqual([{ tab: 'tab2' }]); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(2); + expect(tabs.emitted().ionTabsDidChange[1]).toEqual([{ tab: 'tab2' }]); + }); + + it('should not emit will change and did change events when going to same tab again', async () => { + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { + path: '/', + component: Tabs, + children: [ + { + path: '', + redirect: 'tab1' + }, + { + path: 'tab1', + component: Tab1, + }, + { + path: 'tab2', + component: Tab2 + } + ] + } + ] + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(App, { + global: { + plugins: [router, IonicVue] + } + }); + + const tabs = wrapper.findComponent(IonTabs); + expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); + expect(tabs.emitted().ionTabsWillChange[0]).toEqual([{ tab: 'tab1' }]); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); + expect(tabs.emitted().ionTabsDidChange[0]).toEqual([{ tab: 'tab1' }]); + + router.push('/tab1') + await flushPromises() + + expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); + }); + + it('should emit will change and did change events when getting redirected', async () => { + const Tab3 = { + components: { IonPage }, + template: `Tab 3` + } + + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { + path: '/', + component: Tabs, + children: [ + { + path: '', + redirect: 'tab1' + }, + { + path: 'tab1', + component: Tab1, + }, + { + path: 'tab2', + component: Tab2 + }, + { + path: 'tab3', + component: Tab3, + beforeEnter: (to, from, next) => { + next({ path: '/tab2' }); + }, + } + ] + } + ] + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(App, { + global: { + plugins: [router, IonicVue] + } + }); + + const tabs = wrapper.findComponent(IonTabs); + expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); + expect(tabs.emitted().ionTabsWillChange[0]).toEqual([{ tab: 'tab1' }]); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); + expect(tabs.emitted().ionTabsDidChange[0]).toEqual([{ tab: 'tab1' }]); + + router.push('/tab3') + await flushPromises() + + expect(tabs.emitted().ionTabsWillChange.length).toEqual(2); + expect(tabs.emitted().ionTabsWillChange[1]).toEqual([{ tab: 'tab2' }]); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(2); + expect(tabs.emitted().ionTabsDidChange[1]).toEqual([{ tab: 'tab2' }]); + }); +}); From 6ee38a2baa448d46e449ad6ba795fd8f40b134c3 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 11 Nov 2020 13:58:56 -0500 Subject: [PATCH 2/4] bug fix --- packages/vue/src/components/IonTabs.ts | 3 + packages/vue/test-app/tests/unit/tabs.spec.ts | 63 ------------------- 2 files changed, 3 insertions(+), 63 deletions(-) diff --git a/packages/vue/src/components/IonTabs.ts b/packages/vue/src/components/IonTabs.ts index 2871aef344e..4692badb6bb 100644 --- a/packages/vue/src/components/IonTabs.ts +++ b/packages/vue/src/components/IonTabs.ts @@ -33,6 +33,9 @@ export const IonTabs = defineComponent({ const hasTopSlot = slottedTabBar && slottedTabBar.props?.slot === 'top'; if (slottedTabBar) { + if (!slottedTabBar.props) { + slottedTabBar.props = {}; + } slottedTabBar.props._tabsWillChange = (tab: string) => $emit(WILL_CHANGE, { tab }); slottedTabBar.props._tabsDidChange = (tab: string) => $emit(DID_CHANGE, { tab }); } diff --git a/packages/vue/test-app/tests/unit/tabs.spec.ts b/packages/vue/test-app/tests/unit/tabs.spec.ts index 8bdf0f85f61..ef544599b6e 100644 --- a/packages/vue/test-app/tests/unit/tabs.spec.ts +++ b/packages/vue/test-app/tests/unit/tabs.spec.ts @@ -6,7 +6,6 @@ const App = { components: { IonApp, IonRouterOutlet }, template: '', } - const Tabs = { components: { IonPage, IonTabs, IonTabBar, IonTabButton, IonLabel }, template: ` @@ -16,7 +15,6 @@ const Tabs = { Tab 1 - Tab 2 @@ -25,7 +23,6 @@ const Tabs = { `, } - const Tab1 = { components: { IonPage }, template: `Tab 1` @@ -129,64 +126,4 @@ describe('ion-tabs', () => { expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); }); - - it('should emit will change and did change events when getting redirected', async () => { - const Tab3 = { - components: { IonPage }, - template: `Tab 3` - } - - const router = createRouter({ - history: createWebHistory(process.env.BASE_URL), - routes: [ - { - path: '/', - component: Tabs, - children: [ - { - path: '', - redirect: 'tab1' - }, - { - path: 'tab1', - component: Tab1, - }, - { - path: 'tab2', - component: Tab2 - }, - { - path: 'tab3', - component: Tab3, - beforeEnter: (to, from, next) => { - next({ path: '/tab2' }); - }, - } - ] - } - ] - }); - - router.push('/'); - await router.isReady(); - const wrapper = mount(App, { - global: { - plugins: [router, IonicVue] - } - }); - - const tabs = wrapper.findComponent(IonTabs); - expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); - expect(tabs.emitted().ionTabsWillChange[0]).toEqual([{ tab: 'tab1' }]); - expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); - expect(tabs.emitted().ionTabsDidChange[0]).toEqual([{ tab: 'tab1' }]); - - router.push('/tab3') - await flushPromises() - - expect(tabs.emitted().ionTabsWillChange.length).toEqual(2); - expect(tabs.emitted().ionTabsWillChange[1]).toEqual([{ tab: 'tab2' }]); - expect(tabs.emitted().ionTabsDidChange.length).toEqual(2); - expect(tabs.emitted().ionTabsDidChange[1]).toEqual([{ tab: 'tab2' }]); - }); }); From ed96f1f7c4b4183a290f80a225240a14b36b8f9e Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 11 Nov 2020 19:26:30 +0000 Subject: [PATCH 3/4] add more tests --- packages/vue/test-app/tests/unit/tabs.spec.ts | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/packages/vue/test-app/tests/unit/tabs.spec.ts b/packages/vue/test-app/tests/unit/tabs.spec.ts index ef544599b6e..0262da6d34b 100644 --- a/packages/vue/test-app/tests/unit/tabs.spec.ts +++ b/packages/vue/test-app/tests/unit/tabs.spec.ts @@ -6,6 +6,7 @@ const App = { components: { IonApp, IonRouterOutlet }, template: '', } + const Tabs = { components: { IonPage, IonTabs, IonTabBar, IonTabButton, IonLabel }, template: ` @@ -33,6 +34,8 @@ const Tab2 = { } describe('ion-tabs', () => { + (HTMLElement.prototype as HTMLIonRouterOutletElement).commit = jest.fn(); + it('should emit will change and did change events when changing tab', async () => { const router = createRouter({ history: createWebHistory(process.env.BASE_URL), @@ -126,4 +129,118 @@ describe('ion-tabs', () => { expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); }); + + it('should not emit will change and did change events when going to a non tabs page', async () => { + const Sibling = { + components: { IonPage }, + template: `Sibling Page` + } + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { + path: '/', + component: Tabs, + children: [ + { + path: '', + redirect: 'tab1' + }, + { + path: 'tab1', + component: Tab1 + }, + { + path: 'tab2', + component: Tab2 + } + ] + }, + { + path: '/sibling', + component: Sibling + } + ] + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(App, { + global: { + plugins: [router, IonicVue] + } + }); + + const tabs = wrapper.findComponent(IonTabs); + expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); + expect(tabs.emitted().ionTabsWillChange[0]).toEqual([{ tab: 'tab1' }]); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); + expect(tabs.emitted().ionTabsDidChange[0]).toEqual([{ tab: 'tab1' }]); + + router.push('/sibling'); + await flushPromises(); + + await new Promise((r) => setTimeout(r, 100)); + + expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); + }); + + it('should not emit will change and did change events when going to child tab page', async () => { + const Child = { + components: { IonPage }, + template: `Child Page` + } + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { + path: '/', + component: Tabs, + children: [ + { + path: '', + redirect: 'tab1' + }, + { + path: 'tab1', + component: Tab1, + children: [ + { + path: 'child', + component: Child + } + ] + }, + { + path: 'tab2', + component: Tab2 + } + ] + } + ] + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(App, { + global: { + plugins: [router, IonicVue] + } + }); + + const tabs = wrapper.findComponent(IonTabs); + expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); + expect(tabs.emitted().ionTabsWillChange[0]).toEqual([{ tab: 'tab1' }]); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); + expect(tabs.emitted().ionTabsDidChange[0]).toEqual([{ tab: 'tab1' }]); + + router.push('/tab1/child'); + await flushPromises(); + + await new Promise((r) => setTimeout(r, 100)); + + expect(tabs.emitted().ionTabsWillChange.length).toEqual(1); + expect(tabs.emitted().ionTabsDidChange.length).toEqual(1); + }); }); From 0c7d7ca0344066ed2d54edba863d05a6bdc53d6b Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Thu, 19 Nov 2020 18:27:02 +0000 Subject: [PATCH 4/4] add more comments --- packages/vue/src/components/IonTabs.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/vue/src/components/IonTabs.ts b/packages/vue/src/components/IonTabs.ts index 4692badb6bb..e292d530354 100644 --- a/packages/vue/src/components/IonTabs.ts +++ b/packages/vue/src/components/IonTabs.ts @@ -30,17 +30,24 @@ export const IonTabs = defineComponent({ */ if (slottedContent && slottedContent.length > 0) { const slottedTabBar = slottedContent.find((child: VNode) => child.type && (child.type as any).name === 'IonTabBar'); - const hasTopSlot = slottedTabBar && slottedTabBar.props?.slot === 'top'; + const hasTopSlotTabBar = slottedTabBar && slottedTabBar.props?.slot === 'top'; if (slottedTabBar) { if (!slottedTabBar.props) { slottedTabBar.props = {}; } + /** + * ionTabsWillChange and ionTabsDidChange are + * fired from `ion-tabs`, so we need to pass these down + * as props so they can fire when the active tab changes. + * TODO: We may want to move logic from the tab bar into here + * so we do not have code split across two components. + */ slottedTabBar.props._tabsWillChange = (tab: string) => $emit(WILL_CHANGE, { tab }); slottedTabBar.props._tabsDidChange = (tab: string) => $emit(DID_CHANGE, { tab }); } - if (hasTopSlot) { + if (hasTopSlotTabBar) { childrenToRender = [ ...slottedContent, ...childrenToRender