From 47f9ab366372f3a9dd8408bd79e3eda4a36b3e83 Mon Sep 17 00:00:00 2001 From: Brian Soumakian Date: Sat, 8 Apr 2017 03:25:04 -0700 Subject: [PATCH 1/8] fix null/undefined references --- src/util/module-loader.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/util/module-loader.ts b/src/util/module-loader.ts index 9e2db848a7..1d83e3bcc8 100644 --- a/src/util/module-loader.ts +++ b/src/util/module-loader.ts @@ -78,23 +78,17 @@ export interface LoadedModule { * @hidden */ export function setupPreloadingImplementation(config: Config, deepLinkConfig: DeepLinkConfig, moduleLoader: ModuleLoader) { - if (config.getBoolean('preloadModules')) { + if (config.getBoolean('preloadModules') && deepLinkConfig && deepLinkConfig.links) { const linksToLoad = deepLinkConfig.links.filter(link => !!link.loadChildren && link.priority !== 'off'); // Load the high priority modules first - const highPriorityPromises = linksToLoad.map(link => { - if (link.priority === 'high') { - return moduleLoader.load(link.loadChildren); - } - }); + const highPriorityPromises = linksToLoad.filter(link => link.priority === 'high') + .map(link => moduleLoader.load(link.loadChildren)); Promise.all(highPriorityPromises).then(() => { // Load the low priority modules after the high priority are done - const lowPriorityPromises = linksToLoad.map(link => { - if (link.priority === 'low') { - return moduleLoader.load(link.loadChildren); - } - }); + const lowPriorityPromises = linksToLoad.filter(link => link.priority === 'low') + .map(link => moduleLoader.load(link.loadChildren)); return Promise.all(lowPriorityPromises); }).catch(err => { console.error(err.message); From 665e44f6335158c56d2a0f067b5b1737bb945379 Mon Sep 17 00:00:00 2001 From: Amit Moryossef Date: Sun, 30 Apr 2017 19:09:55 +0300 Subject: [PATCH 2/8] fix(rtl): change item reorder offset for RTL (#11395) --- src/components/item/item-reorder-gesture.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/item/item-reorder-gesture.ts b/src/components/item/item-reorder-gesture.ts index 26b007d8fb..374fdf164f 100644 --- a/src/components/item/item-reorder-gesture.ts +++ b/src/components/item/item-reorder-gesture.ts @@ -134,7 +134,8 @@ export class ItemReorderGesture { } private itemForCoord(coord: PointerCoordinates): HTMLElement { - const x = this.offset.x - 100; + const sideOffset = this.plt.isRTL ? 100 : -100; + const x = this.offset.x + sideOffset; const y = coord.y; const element = this.plt.getElementFromPoint(x, y); return findReorderItem(element, this.reorderList.getNativeElement()); From 19a3b6616ec7de8cd6198a0316f94476b2b6d240 Mon Sep 17 00:00:00 2001 From: Brian Soumakian Date: Sun, 30 Apr 2017 15:24:09 -0700 Subject: [PATCH 3/8] add unit tests --- src/util/module-loader.ts | 30 ++++---- src/util/test/module-loader.spec.ts | 113 +++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 16 deletions(-) diff --git a/src/util/module-loader.ts b/src/util/module-loader.ts index 9131c95ef3..7d5b2dd3c9 100644 --- a/src/util/module-loader.ts +++ b/src/util/module-loader.ts @@ -74,23 +74,25 @@ export interface LoadedModule { /** * @hidden */ -export function setupPreloadingImplementation(config: Config, deepLinkConfig: DeepLinkConfig, moduleLoader: ModuleLoader) { +export function setupPreloadingImplementation(config: Config, deepLinkConfig: DeepLinkConfig, moduleLoader: ModuleLoader): Promise { if (config.getBoolean('preloadModules') && deepLinkConfig && deepLinkConfig.links) { - const linksToLoad = deepLinkConfig.links.filter(link => !!link.loadChildren && link.priority !== 'off'); + const linksToLoad = deepLinkConfig.links.filter(link => !!link.loadChildren && link.priority !== 'off'); - // Load the high priority modules first - const highPriorityPromises = linksToLoad.filter(link => link.priority === 'high') + // Load the high priority modules first + const highPriorityPromises = linksToLoad.filter(link => link.priority === 'high') + .map(link => moduleLoader.load(link.loadChildren)); + + return Promise.all(highPriorityPromises).then(() => { + // Load the low priority modules after the high priority are done + const lowPriorityPromises = linksToLoad.filter(link => link.priority === 'low') .map(link => moduleLoader.load(link.loadChildren)); - - Promise.all(highPriorityPromises).then(() => { - // Load the low priority modules after the high priority are done - const lowPriorityPromises = linksToLoad.filter(link => link.priority === 'low') - .map(link => moduleLoader.load(link.loadChildren)); - return Promise.all(lowPriorityPromises); - }).catch(err => { - console.error(err.message); - }); - } + return Promise.all(lowPriorityPromises); + }).catch(err => { + console.error(err.message); + }); + } else { + return Promise.resolve(); + } } /** diff --git a/src/util/test/module-loader.spec.ts b/src/util/test/module-loader.spec.ts index a64dd492d6..a8d8aba897 100644 --- a/src/util/test/module-loader.spec.ts +++ b/src/util/test/module-loader.spec.ts @@ -1,6 +1,8 @@ -import { ModuleLoader, LAZY_LOADED_TOKEN } from '../module-loader'; -import { mockModuleLoader, mockNgModuleLoader } from '../mock-providers'; +import { ModuleLoader, LAZY_LOADED_TOKEN, setupPreloadingImplementation } from '../module-loader'; +import { mockModuleLoader, mockNgModuleLoader, mockConfig } from '../mock-providers'; import { NgModuleLoader } from '../ng-module-loader'; +import { Config } from '../../config/config'; +import { DeepLinkConfig } from '../../navigation/nav-util'; describe('module-loader', () => { @@ -115,12 +117,119 @@ describe('module-loader', () => { }); + describe('setupPreloadingImplementation', () => { + + it('should return a promise', (done: Function) => { + let promise = setupPreloadingImplementation(config, null, moduleLoader); + promise.then((response) => { + done(); + }).catch((err: Error) => { + fail(err); + done(err); + }); + }); + + it('should not call ModuleLoader when preloading disabled', (done: Function) => { + spyOn(moduleLoader, 'load').and.returnValue(Promise.resolve()); + + config.set('preloadModules', false); + const deepLinkConfig: DeepLinkConfig = { + links: [] + }; + let promise = setupPreloadingImplementation(config, deepLinkConfig, moduleLoader); + promise.then((response) => { + expect(moduleLoader.load).not.toHaveBeenCalled(); + done(); + }).catch((err: Error) => { + fail(err); + done(err); + }); + }); + + it('should not call ModuleLoader when deepLinkConfig missing', (done: Function) => { + spyOn(moduleLoader, 'load').and.returnValue(Promise.resolve()); + + config.set('preloadModules', true); + let promise = setupPreloadingImplementation(config, null, moduleLoader); + promise.then((response) => { + expect(moduleLoader.load).not.toHaveBeenCalled(); + done(); + }).catch((err: Error) => { + fail(err); + done(err); + }); + }); + + it('should not call ModuleLoader when no low or high priority links', (done: Function) => { + spyOn(moduleLoader, 'load').and.returnValue(Promise.resolve()); + + config.set('preloadModules', true); + const deepLinkConfig: DeepLinkConfig = { + links: [{ + loadChildren: 'offString', + priority: 'off' + }] + }; + let promise = setupPreloadingImplementation(config, deepLinkConfig, moduleLoader); + promise.then((response) => { + expect(moduleLoader.load).not.toHaveBeenCalled(); + done(); + }).catch((err: Error) => { + fail(err); + done(err); + }); + }); + + it('should call ModuleLoader when has low priority links', (done: Function) => { + spyOn(moduleLoader, 'load').and.returnValue(Promise.resolve()); + + config.set('preloadModules', true); + const deepLinkConfig: DeepLinkConfig = { + links: [{ + loadChildren: 'lowString', + priority: 'low' + }] + }; + let promise = setupPreloadingImplementation(config, deepLinkConfig, moduleLoader); + promise.then((response) => { + expect(moduleLoader.load).toHaveBeenCalledWith('lowString'); + done(); + }).catch((err: Error) => { + fail(err); + done(err); + }); + }); + + it('should call ModuleLoader when has high priority links', (done: Function) => { + spyOn(moduleLoader, 'load').and.returnValue(Promise.resolve()); + + config.set('preloadModules', true); + const deepLinkConfig: DeepLinkConfig = { + links: [{ + loadChildren: 'highString', + priority: 'high' + }] + }; + let promise = setupPreloadingImplementation(config, deepLinkConfig, moduleLoader); + promise.then((response) => { + expect(moduleLoader.load).toHaveBeenCalledWith('highString'); + done(); + }).catch((err: Error) => { + fail(err); + done(err); + }); + }); + + }); + var moduleLoader: ModuleLoader; var ngModuleLoader: NgModuleLoader; + var config: Config; beforeEach(() => { ngModuleLoader = mockNgModuleLoader(); moduleLoader = mockModuleLoader(ngModuleLoader); + config = mockConfig(); }); }); From ed66591db6488f419ccdbb9556c61b71a4407632 Mon Sep 17 00:00:00 2001 From: "Manu Mtz.-Almeida" Date: Mon, 1 May 2017 02:19:07 +0200 Subject: [PATCH 4/8] test(view-controller): adds isOverlay asserts --- src/components/app/app.ts | 4 +++- src/components/app/overlay-portal.ts | 7 +++++-- src/navigation/nav-controller-base.ts | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/components/app/app.ts b/src/components/app/app.ts index 883f1cb43b..aa76a86375 100644 --- a/src/components/app/app.ts +++ b/src/components/app/app.ts @@ -4,7 +4,7 @@ import { Title, DOCUMENT } from '@angular/platform-browser'; import { IonicApp } from './app-root'; import * as Constants from './app-constants'; import { ClickBlock } from './click-block'; -import { runInDev } from '../../util/util'; +import { runInDev, assert } from '../../util/util'; import { Config } from '../../config/config'; import { isNav, NavOptions, DIRECTION_FORWARD, DIRECTION_BACK } from '../../navigation/nav-util'; import { MenuController } from './menu-controller'; @@ -226,6 +226,8 @@ export class App { * @hidden */ present(enteringView: ViewController, opts: NavOptions, appPortal?: number): Promise { + assert(enteringView.isOverlay, 'presented view controller needs to be an overlay'); + const portal = this._appRoot._getPortal(appPortal); // Set Nav must be set here in order to dimiss() work synchnously. diff --git a/src/components/app/overlay-portal.ts b/src/components/app/overlay-portal.ts index 0bc79493a8..78b213efc3 100644 --- a/src/components/app/overlay-portal.ts +++ b/src/components/app/overlay-portal.ts @@ -9,6 +9,7 @@ import { Keyboard } from '../../platform/keyboard'; import { NavControllerBase } from '../../navigation/nav-controller-base'; import { Platform } from '../../platform/platform'; import { TransitionController } from '../../transitions/transition-controller'; +import { ViewController } from '../../navigation/view-controller'; /** * @hidden @@ -40,8 +41,10 @@ export class OverlayPortal extends NavControllerBase { // on every page change make sure the portal has // dismissed any views that should be auto dismissed on page change - app.viewDidLeave.subscribe((ev: any) => { - !ev.isOverlay && this.dismissPageChangeViews(); + app.viewDidLeave.subscribe((view: ViewController) => { + if (!view.isOverlay) { + this.dismissPageChangeViews(); + } }); } diff --git a/src/navigation/nav-controller-base.ts b/src/navigation/nav-controller-base.ts index 99f8c209c0..03c1f94a27 100644 --- a/src/navigation/nav-controller-base.ts +++ b/src/navigation/nav-controller-base.ts @@ -1117,7 +1117,7 @@ export class NavControllerBase extends Ion implements NavController { dismissPageChangeViews() { for (let view of this._views) { if (view.data && view.data.dismissOnPageChange) { - view.dismiss().catch(null); + view.dismiss().catch(() => {}); } } } From 2afc5cfeda96a822f0dce5c7aa289002ebae8bd1 Mon Sep 17 00:00:00 2001 From: "Manu Mtz.-Almeida" Date: Mon, 1 May 2017 02:25:45 +0200 Subject: [PATCH 5/8] fix(module-loader): null references --- src/util/module-loader.ts | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/util/module-loader.ts b/src/util/module-loader.ts index 7d5b2dd3c9..fcd918cde4 100644 --- a/src/util/module-loader.ts +++ b/src/util/module-loader.ts @@ -75,24 +75,26 @@ export interface LoadedModule { * @hidden */ export function setupPreloadingImplementation(config: Config, deepLinkConfig: DeepLinkConfig, moduleLoader: ModuleLoader): Promise { - if (config.getBoolean('preloadModules') && deepLinkConfig && deepLinkConfig.links) { - const linksToLoad = deepLinkConfig.links.filter(link => !!link.loadChildren && link.priority !== 'off'); - - // Load the high priority modules first - const highPriorityPromises = linksToLoad.filter(link => link.priority === 'high') - .map(link => moduleLoader.load(link.loadChildren)); - - return Promise.all(highPriorityPromises).then(() => { - // Load the low priority modules after the high priority are done - const lowPriorityPromises = linksToLoad.filter(link => link.priority === 'low') - .map(link => moduleLoader.load(link.loadChildren)); - return Promise.all(lowPriorityPromises); - }).catch(err => { - console.error(err.message); - }); - } else { + if (!deepLinkConfig || !deepLinkConfig.links || !config.getBoolean('preloadModules')) { return Promise.resolve(); } + const linksToLoad = deepLinkConfig.links.filter(link => !!link.loadChildren && link.priority !== 'off'); + + // Load the high priority modules first + const highPriorityPromises = linksToLoad + .filter(link => link.priority === 'high') + .map(link => moduleLoader.load(link.loadChildren)); + + return Promise.all(highPriorityPromises).then(() => { + // Load the low priority modules after the high priority are done + const lowPriorityPromises = linksToLoad + .filter(link => link.priority === 'low') + .map(link => moduleLoader.load(link.loadChildren)); + + return Promise.all(lowPriorityPromises); + }).catch(err => { + console.error(err.message); + }); } /** From a04b577f1d8353caafe905ef8cdf587b6b3cea40 Mon Sep 17 00:00:00 2001 From: Hayden Braxton Date: Sun, 30 Apr 2017 20:34:38 -0400 Subject: [PATCH 6/8] feat(select): close select programatically * implement ability to close select programmatically * refactor(select): made changes based on PR feedback, namely assigning overlay to the overlay property at then end of the open function, applying DeMorgan's Law to a condition, and returning the overlay dismiss promise from the close function * refactor(select): made additional changes to closing select component programmatically to resolve issue 11318 --- src/components/select/select.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/components/select/select.ts b/src/components/select/select.ts index 61eb75f439..2ae817ca9e 100644 --- a/src/components/select/select.ts +++ b/src/components/select/select.ts @@ -151,6 +151,7 @@ export class Select extends BaseInput implements OnDestroy { _multi: boolean = false; _options: QueryList