From f2ac6e30644848f0c5c36057d1f55551fb70952f Mon Sep 17 00:00:00 2001 From: "Manu Mtz.-Almeida" Date: Fri, 9 Mar 2018 18:53:14 +0100 Subject: [PATCH] fix(router): fix flickering --- packages/core/src/components/nav/nav-util.ts | 4 +- packages/core/src/components/nav/nav.tsx | 9 ++- packages/core/src/components/router/readme.md | 5 +- .../core/src/components/router/router.tsx | 56 +++++++------ .../components/router/test/common.spec.tsx | 47 ----------- .../components/router/test/matching.spec.tsx | 24 +++++- .../src/components/router/utils/common.ts | 44 ---------- .../core/src/components/router/utils/dom.ts | 16 +++- .../src/components/router/utils/matching.ts | 17 +++- .../src/components/router/utils/parser.ts | 18 +++-- packages/core/src/components/tab/tab.tsx | 29 ++++--- packages/core/src/components/tabs/tabs.tsx | 81 ++++++++++++------- packages/core/src/utils/theme.ts | 4 +- 13 files changed, 180 insertions(+), 174 deletions(-) delete mode 100644 packages/core/src/components/router/test/common.spec.tsx delete mode 100644 packages/core/src/components/router/utils/common.ts diff --git a/packages/core/src/components/nav/nav-util.ts b/packages/core/src/components/nav/nav-util.ts index ac41387006..9d7b50acf6 100644 --- a/packages/core/src/components/nav/nav-util.ts +++ b/packages/core/src/components/nav/nav-util.ts @@ -3,10 +3,10 @@ import { NavControllerBase } from './nav'; import { Transition } from './transition'; -export type Page2 = string | HTMLElement | ViewController; +export type NavParams = {[key: string]: any}; export interface PageMeta { - page: Page2; + page: string | HTMLElement | ViewController; params?: any; } diff --git a/packages/core/src/components/nav/nav.tsx b/packages/core/src/components/nav/nav.tsx index 711cf4545b..8dd3476b42 100644 --- a/packages/core/src/components/nav/nav.tsx +++ b/packages/core/src/components/nav/nav.tsx @@ -1,9 +1,10 @@ -import { Component, Element, Event, EventEmitter, Method, Prop, Watch } from '@stencil/core'; +import { Build, Component, Element, Event, EventEmitter, Method, Prop, Watch } from '@stencil/core'; import { DIRECTION_BACK, DIRECTION_FORWARD, INIT_ZINDEX, NavOptions, + NavParams, NavResult, STATE_ATTACHED, STATE_DESTROYED, @@ -66,7 +67,7 @@ export class NavControllerBase implements NavOutlet { const useRouter = !!document.querySelector('ion-router'); if (!useRouter) { this.setRoot(this.root); - } else { + } else if (Build.isDev) { console.warn(' does not support a root attribute when using ion-router.'); } } @@ -90,7 +91,7 @@ export class NavControllerBase implements NavOutlet { } @Method() - push(page: any, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise { + push(page: any, params?: NavParams, opts?: NavOptions, done?: TransitionDoneFn): Promise { return this._queueTrns({ insertStart: -1, insertViews: [{ page: page, params: params }], @@ -99,7 +100,7 @@ export class NavControllerBase implements NavOutlet { } @Method() - insert(insertIndex: number, page: any, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise { + insert(insertIndex: number, page: any, params?: NavParams, opts?: NavOptions, done?: TransitionDoneFn): Promise { return this._queueTrns({ insertStart: insertIndex, insertViews: [{ page: page, params: params }], diff --git a/packages/core/src/components/router/readme.md b/packages/core/src/components/router/readme.md index 87a6d75f0d..c0731ed779 100644 --- a/packages/core/src/components/router/readme.md +++ b/packages/core/src/components/router/readme.md @@ -31,7 +31,10 @@ boolean ## Methods -#### pushURL() +#### navChanged() + + +#### push() diff --git a/packages/core/src/components/router/router.tsx b/packages/core/src/components/router/router.tsx index f899687f46..2bd559f98c 100644 --- a/packages/core/src/components/router/router.tsx +++ b/packages/core/src/components/router/router.tsx @@ -1,8 +1,8 @@ -import { Component, Element, Listen, Method, Prop } from '@stencil/core'; +import { Build, Component, Element, Listen, Method, Prop } from '@stencil/core'; import { Config, DomController } from '../../index'; import { flattenRouterTree, readRoutes } from './utils/parser'; import { readNavState, writeNavState } from './utils/dom'; -import { chainToPath, parsePath, readPath, writePath } from './utils/path'; +import { chainToPath, generatePath, parsePath, readPath, writePath } from './utils/path'; import { RouteChain } from './utils/interfaces'; import { routerIDsToChain, routerPathToChain } from './utils/matching'; @@ -29,6 +29,16 @@ export class Router { const tree = readRoutes(this.el); this.routes = flattenRouterTree(tree); + if (Build.isDev) { + console.debug('%c[@ionic/core]', 'font-weight: bold', `ion-router registered ${this.routes.length} routes`); + for (const chain of this.routes) { + const path: string[] = []; + chain.forEach(r => path.push(...r.path)); + const ids = chain.map(r => r.id); + console.debug(`%c ${generatePath(path)}`, 'font-weight: bold; padding-left: 20px', '=>\t', `(${ids.join(', ')})`); + } + } + // perform first write this.dom.raf(() => { console.debug('[OUT] page load -> write nav state'); @@ -45,32 +55,32 @@ export class Router { this.writeNavStateRoot(); } - @Listen('body:ionNavChanged') - protected onNavChanged(ev: CustomEvent) { - if (this.busy) { - return; - } - console.debug('[IN] nav changed -> update URL'); - const { ids, pivot } = this.readNavState(); - const chain = routerIDsToChain(ids, this.routes); - if (chain) { - if (chain.length > ids.length) { - // readNavState() found a pivot that is not initialized - console.debug('[IN] pivot uninitialized -> write partial nav state'); - this.writeNavState(pivot, chain.slice(ids.length), 0); - } + @Method() + navChanged(isPop: boolean) { + if (!this.busy) { + console.debug('[IN] nav changed -> update URL'); + const { ids, pivot } = this.readNavState(); + const chain = routerIDsToChain(ids, this.routes); + if (chain) { + const path = chainToPath(chain); + this.writePath(path, isPop); - const isPop = ev.detail.isPop === true; - const path = chainToPath(chain); - this.writePath(path, isPop); - } else { - console.warn('no matching URL for ', ids.map(i => i.id)); + if (chain.length > ids.length) { + // readNavState() found a pivot that is not initialized + console.debug('[IN] pivot uninitialized -> write partial nav state'); + return this.writeNavState(pivot, chain.slice(ids.length), 0); + } + } else { + console.warn('no matching URL for ', ids.map(i => i.id)); + } } + return Promise.resolve(); } + @Method() - pushURL(url: string, isPop = false) { - this.writePath(parsePath(url), isPop); + push(url: string, backDirection = false) { + this.writePath(parsePath(url), backDirection); return this.writeNavStateRoot(); } diff --git a/packages/core/src/components/router/test/common.spec.tsx b/packages/core/src/components/router/test/common.spec.tsx deleted file mode 100644 index 57f9a0ccc4..0000000000 --- a/packages/core/src/components/router/test/common.spec.tsx +++ /dev/null @@ -1,47 +0,0 @@ -import { RouterSegments, breadthFirstSearch } from '../utils/common'; - -describe('RouterSegments', () => { - it ('should initialize with empty array', () => { - const s = new RouterSegments([]); - expect(s.next()).toEqual(''); - expect(s.next()).toEqual(''); - expect(s.next()).toEqual(''); - expect(s.next()).toEqual(''); - expect(s.next()).toEqual(''); - }); - - it ('should initialize with array', () => { - const s = new RouterSegments(['', 'path', 'to', 'destination']); - expect(s.next()).toEqual(''); - expect(s.next()).toEqual('path'); - expect(s.next()).toEqual('to'); - expect(s.next()).toEqual('destination'); - expect(s.next()).toEqual(''); - expect(s.next()).toEqual(''); - }); -}); - -describe('breadthFirstSearch', () => { - it('should search in order', () => { - const n1 = { tagName: 'ION-TABS', children: [] as any }; - const n2 = { tagName: 'DIV', children: [n1] }; - const n3 = { tagName: 'ION-NAV', children: [n2] }; - const n4 = { tagName: 'ION-TABS', children: [] as any }; - const n5 = { tagName: 'DIV', children: [n4] }; - const n6 = { tagName: 'DIV', children: [n5, n3] }; - const n7 = { tagName: 'DIV', children: [] as any }; - const n8 = { tagName: 'DIV', children: [n6] }; - const n9 = { tagName: 'DIV', children: [n8, n7] }; - - expect(breadthFirstSearch(n9 as any)).toBe(n3); - expect(breadthFirstSearch(n8 as any)).toBe(n3); - expect(breadthFirstSearch(n7 as any)).toBe(null); - expect(breadthFirstSearch(n6 as any)).toBe(n3); - expect(breadthFirstSearch(n5 as any)).toBe(n4); - expect(breadthFirstSearch(n4 as any)).toBe(n4); - expect(breadthFirstSearch(n3 as any)).toBe(n3); - expect(breadthFirstSearch(n2 as any)).toBe(n1); - expect(breadthFirstSearch(n1 as any)).toBe(n1); - }); -}); - diff --git a/packages/core/src/components/router/test/matching.spec.tsx b/packages/core/src/components/router/test/matching.spec.tsx index 0370bcb508..4dc5ac7e0f 100644 --- a/packages/core/src/components/router/test/matching.spec.tsx +++ b/packages/core/src/components/router/test/matching.spec.tsx @@ -1,5 +1,5 @@ import { RouteChain } from '../utils/interfaces'; -import { matchesIDs, matchesPath, mergeParams, routerPathToChain } from '../utils/matching'; +import { RouterSegments, matchesIDs, matchesPath, mergeParams, routerPathToChain } from '../utils/matching'; import { parsePath } from '../utils/path'; const CHAIN_1: RouteChain = [ @@ -190,6 +190,28 @@ describe('mergeParams', () => { }); }); + +describe('RouterSegments', () => { + it ('should initialize with empty array', () => { + const s = new RouterSegments([]); + expect(s.next()).toEqual(''); + expect(s.next()).toEqual(''); + expect(s.next()).toEqual(''); + expect(s.next()).toEqual(''); + expect(s.next()).toEqual(''); + }); + + it ('should initialize with array', () => { + const s = new RouterSegments(['', 'path', 'to', 'destination']); + expect(s.next()).toEqual(''); + expect(s.next()).toEqual('path'); + expect(s.next()).toEqual('to'); + expect(s.next()).toEqual('destination'); + expect(s.next()).toEqual(''); + expect(s.next()).toEqual(''); + }); +}); + // describe('matchRoute', () => { // it('should match simple route', () => { // const path = ['path', 'to', 'component']; diff --git a/packages/core/src/components/router/utils/common.ts b/packages/core/src/components/router/utils/common.ts deleted file mode 100644 index a766a3fd51..0000000000 --- a/packages/core/src/components/router/utils/common.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { NavOutletElement } from './interfaces'; - -export class RouterSegments { - private path: string[]; - constructor(path: string[]) { - this.path = path.slice(); - } - - next(): string { - if (this.path.length > 0) { - return this.path.shift() as string; - } - return ''; - } -} - -const navs = ['ION-NAV', 'ION-TABS']; -export function breadthFirstSearch(root: HTMLElement): NavOutletElement | null { - if (!root) { - console.error('search root is null'); - return null; - } - // we do a Breadth-first search - // Breadth-first search (BFS) is an algorithm for traversing or searching tree - // or graph data structures.It starts at the tree root(or some arbitrary node of a graph, - // sometimes referred to as a 'search key'[1]) and explores the neighbor nodes - // first, before moving to the next level neighbours. - - const queue = [root]; - let node: HTMLElement | undefined; - while (node = queue.shift()) { - // visit node - if (navs.indexOf(node.tagName) >= 0) { - return node as NavOutletElement; - } - - // queue children - const children = node.children; - for (let i = 0; i < children.length; i++) { - queue.push(children[i] as NavOutletElement); - } - } - return null; -} diff --git a/packages/core/src/components/router/utils/dom.ts b/packages/core/src/components/router/utils/dom.ts index 76f998bcc5..bf3c0fa106 100644 --- a/packages/core/src/components/router/utils/dom.ts +++ b/packages/core/src/components/router/utils/dom.ts @@ -1,12 +1,11 @@ -import { breadthFirstSearch } from './common'; -import { NavOutlet, RouteChain, RouteID } from './interfaces'; +import { NavOutlet, NavOutletElement, RouteChain, RouteID } from './interfaces'; export function writeNavState(root: HTMLElement, chain: RouteChain|null, index: number, direction: number): Promise { if (!chain || index >= chain.length) { return Promise.resolve(); } const route = chain[index]; - const node = breadthFirstSearch(root); + const node = searchNavNode(root); if (!node) { return Promise.resolve(); } @@ -32,7 +31,7 @@ export function readNavState(node: HTMLElement) { const ids: RouteID[] = []; let pivot: NavOutlet|null; while (true) { - pivot = breadthFirstSearch(node); + pivot = searchNavNode(node); if (pivot) { const id = pivot.getRouteId(); if (id) { @@ -47,3 +46,12 @@ export function readNavState(node: HTMLElement) { } return {ids, pivot}; } + +const QUERY = 'ion-nav,ion-tabs'; + +function searchNavNode(root: HTMLElement): NavOutletElement { + if (root.matches(QUERY)) { + return root as NavOutletElement; + } + return root.querySelector(QUERY); +} diff --git a/packages/core/src/components/router/utils/matching.ts b/packages/core/src/components/router/utils/matching.ts index 12a5dea8b5..39e0480cc5 100644 --- a/packages/core/src/components/router/utils/matching.ts +++ b/packages/core/src/components/router/utils/matching.ts @@ -1,4 +1,3 @@ -import { RouterSegments } from './common'; import { RouteChain, RouteID } from './interfaces'; export function matchesIDs(ids: string[], chain: RouteChain): number { @@ -12,7 +11,6 @@ export function matchesIDs(ids: string[], chain: RouteChain): number { return i; } - export function matchesPath(path: string[], chain: RouteChain): RouteChain | null { const segments = new RouterSegments(path); let matchesDefault = false; @@ -107,3 +105,18 @@ export function routerPathToChain(path: string[], chains: RouteChain[]): RouteCh } return match; } + +export class RouterSegments { + private path: string[]; + constructor(path: string[]) { + this.path = path.slice(); + } + + next(): string { + if (this.path.length > 0) { + return this.path.shift() as string; + } + return ''; + } +} + diff --git a/packages/core/src/components/router/utils/parser.ts b/packages/core/src/components/router/utils/parser.ts index 19e39a393d..2c2a500438 100644 --- a/packages/core/src/components/router/utils/parser.ts +++ b/packages/core/src/components/router/utils/parser.ts @@ -5,12 +5,18 @@ import { parsePath } from './path'; export function readRoutes(root: Element): RouteTree { return (Array.from(root.children) as HTMLIonRouteElement[]) .filter(el => el.tagName === 'ION-ROUTE') - .map(el => ({ - path: parsePath(readProp(el, 'path')), - id: readProp(el, 'component').toLowerCase(), - params: el.params, - children: readRoutes(el) - })); + .map(el => { + const path = parsePath(readProp(el, 'path')); + if (path.includes(':id')) { + console.warn('Using ":id" is not recommended in `ion-route`, it causes conflicts in the DOM'); + } + return { + path: path, + id: readProp(el, 'component').toLowerCase(), + params: el.params, + children: readRoutes(el) + }; + }); } export function readProp(el: HTMLElement, prop: string): string|undefined { diff --git a/packages/core/src/components/tab/tab.tsx b/packages/core/src/components/tab/tab.tsx index 2836d5d39a..b7efd00a9d 100644 --- a/packages/core/src/components/tab/tab.tsx +++ b/packages/core/src/components/tab/tab.tsx @@ -1,4 +1,4 @@ -import { Component, Element, Event, EventEmitter, Method, Prop, State, Watch } from '@stencil/core'; +import { Build, Component, Element, Event, EventEmitter, Method, Prop, State, Watch } from '@stencil/core'; @Component({ tag: 'ion-tab', @@ -83,6 +83,16 @@ export class Tab { */ @Event() ionSelect: EventEmitter; + componentWillLoad() { + if (Build.isDev) { + if (this.component && this.el.childElementCount > 0) { + console.error('You can not use a lazy-loaded component in a tab and inlineed content at the same time.' + + `- Remove the component attribute in: ` + + ` or` + + `- Remove the embeded content inside the ion-tab: `); + } + } + } @Method() getTabId(): string|null { if (this.name) { @@ -95,11 +105,13 @@ export class Tab { } @Method() - setActive(): Promise { - return this.prepareLazyLoaded().then(() => this.showTab()); + setActive(): Promise { + return this.prepareLazyLoaded().then(() => { + this.active = true; + }); } - private prepareLazyLoaded(): Promise { + private prepareLazyLoaded(): Promise { if (!this.loaded && this.component) { this.loaded = true; return attachViewToDom(this.el, this.component); @@ -107,11 +119,6 @@ export class Tab { return Promise.resolve(); } - private showTab(): Promise { - this.active = true; - return Promise.resolve(this.el); - } - hostData() { return { 'aria-labelledby': this.btnId, @@ -125,12 +132,12 @@ export class Tab { } -function attachViewToDom(container: HTMLElement, cmp: string): Promise { +function attachViewToDom(container: HTMLElement, cmp: string): Promise { const el = document.createElement(cmp) as HTMLStencilElement; el.classList.add('ion-page'); container.appendChild(el); if (el.componentOnReady) { return el.componentOnReady(); } - return Promise.resolve(); + return Promise.resolve(el); } diff --git a/packages/core/src/components/tabs/tabs.tsx b/packages/core/src/components/tabs/tabs.tsx index 96e8dc6642..e3c4fafca9 100644 --- a/packages/core/src/components/tabs/tabs.tsx +++ b/packages/core/src/components/tabs/tabs.tsx @@ -1,4 +1,4 @@ -import { Component, Element, Event, EventEmitter, Listen, Method, Prop, State } from '@stencil/core'; +import { Build, Component, Element, Event, EventEmitter, Listen, Method, Prop, State } from '@stencil/core'; import { Config, NavOutlet } from '../../index'; import { RouteID } from '../router/utils/interfaces'; @@ -11,8 +11,8 @@ export class Tabs implements NavOutlet { private ids = -1; private transitioning = false; - private routingView: HTMLIonTabElement; private tabsId: number = (++tabIds); + private leavingTab: HTMLIonTabElement | undefined; @Element() el: HTMLElement; @@ -81,7 +81,7 @@ export class Tabs implements NavOutlet { componentDidUnload() { this.tabs.length = 0; - this.selectedTab = this.routingView = undefined; + this.selectedTab = this.leavingTab = undefined; } @Listen('ionTabbarClick') @@ -95,8 +95,22 @@ export class Tabs implements NavOutlet { */ @Method() select(tabOrIndex: number | HTMLIonTabElement): Promise { - return this.setActive(tabOrIndex) - .then(selectedTab => this.tabSwitch(selectedTab)); + const selectedTab = this.getTab(tabOrIndex); + if (!this.shouldSwitch(selectedTab)) { + return Promise.resolve(false); + } + return this.setActive(selectedTab) + .then(() => this.notifyRouter()) + .then(() => this.tabSwitch()); + } + + @Method() + setRouteId(id: string): Promise { + const selectedTab = this.getTab(id); + if (!this.shouldSwitch(selectedTab)) { + return Promise.resolve(false); + } + return this.setActive(selectedTab).then(() => true); } @Method() @@ -118,21 +132,10 @@ export class Tabs implements NavOutlet { return this.selectedTab; } - @Method() - setRouteId(id: string): Promise { - return this.setActive(id).then(tab => { - if (tab) { - this.routingView = tab; - return true; - } - return false; - }); - } @Method() markVisible(): Promise { - this.tabSwitch(this.routingView); - this.routingView = null; + this.tabSwitch(); return Promise.resolve(); } @@ -142,10 +145,9 @@ export class Tabs implements NavOutlet { return id ? {id} : null; } - @Method() getContainerEl(): HTMLElement { - return this.routingView || this.selectedTab; + return this.selectedTab; } private initTabs() { @@ -162,6 +164,13 @@ export class Tabs implements NavOutlet { private initSelect(): Promise { if (document.querySelector('ion-router')) { + if (Build.isDev) { + const selectedTab = this.tabs.find(t => t.selected); + if (selectedTab) { + console.warn('When using a router (ion-router) makes no difference' + + 'Define routes properly the define which tab is selected'); + } + } return Promise.resolve(); } // find pre-selected tabs @@ -191,13 +200,13 @@ export class Tabs implements NavOutlet { } } - private setActive(tabOrIndex: string | number | HTMLIonTabElement): Promise { + private setActive(selectedTab: HTMLIonTabElement): Promise { if (this.transitioning) { - return Promise.resolve(null); + return Promise.reject('transitioning already happening'); } - const selectedTab = this.getTab(tabOrIndex); - if (!selectedTab || selectedTab === this.selectedTab) { - return Promise.resolve(null); + + if (!selectedTab) { + return Promise.reject('no tab is selected'); } // Reset rest of tabs @@ -208,21 +217,26 @@ export class Tabs implements NavOutlet { } this.transitioning = true; + this.leavingTab = this.selectedTab; + this.selectedTab = selectedTab; return selectedTab.setActive(); } - private tabSwitch(selectedTab: HTMLIonTabElement | null): boolean { + private tabSwitch(): boolean { + const selectedTab = this.selectedTab; + const leavingTab = this.leavingTab; + + this.leavingTab = undefined; this.transitioning = false; if (!selectedTab) { return false; } - const leavingTab = this.selectedTab; + selectedTab.selected = true; if (leavingTab !== selectedTab) { if (leavingTab) { leavingTab.active = false; } - this.selectedTab = selectedTab; this.ionChange.emit(selectedTab); this.ionNavChanged.emit({isPop: false}); return true; @@ -230,6 +244,19 @@ export class Tabs implements NavOutlet { return false; } + private notifyRouter() { + const router = document.querySelector('ion-router'); + if (router) { + return router.navChanged(false); + } + return Promise.resolve(); + } + + private shouldSwitch(selectedTab: HTMLIonTabElement) { + const leavingTab = this.selectedTab; + return selectedTab && selectedTab !== leavingTab && !this.transitioning; + } + render() { const dom = [
diff --git a/packages/core/src/utils/theme.ts b/packages/core/src/utils/theme.ts index adb80cb8fc..20b8d87925 100644 --- a/packages/core/src/utils/theme.ts +++ b/packages/core/src/utils/theme.ts @@ -63,11 +63,11 @@ export function getClassMap(classes: string | undefined): CssClassMap { } export function openURL(url: string, ev: Event, isPop = false) { - if (url && url.indexOf('://') === -1) { + if (url && url[0] !== '#' && url.indexOf('://') === -1) { const router = document.querySelector('ion-router'); if (router) { ev && ev.preventDefault(); - return router.componentOnReady().then(() => router.pushURL(url, isPop)); + return router.componentOnReady().then(() => router.push(url, isPop)); } } return Promise.resolve();