From 698345f171f980ff7026d5a2c75825b378e1769e Mon Sep 17 00:00:00 2001 From: Hristo Deshev Date: Thu, 28 Jan 2016 18:27:31 +0200 Subject: [PATCH] Register ProxyViewContainer children with parent layout. - Fixes a crash when used in a GridLayout on iOS since the layout needs to be notified of any layout children changes. - Adds a _parentChanged hook for all views. --- .../proxy-view-container-tests.ts | 40 ++++++++++++-- ui/core/view-common.ts | 6 ++ ui/core/view.d.ts | 1 + ui/layouts/grid-layout/grid-layout.ios.ts | 18 +----- ui/layouts/layout-base.d.ts | 12 +++- ui/layouts/layout-base.ts | 13 ++++- .../proxy-view-container.ts | 55 ++++++++++++++++--- 7 files changed, 114 insertions(+), 31 deletions(-) diff --git a/apps/tests/ui/proxy-view-container/proxy-view-container-tests.ts b/apps/tests/ui/proxy-view-container/proxy-view-container-tests.ts index 1d2601331..1bd9095d4 100644 --- a/apps/tests/ui/proxy-view-container/proxy-view-container-tests.ts +++ b/apps/tests/ui/proxy-view-container/proxy-view-container-tests.ts @@ -6,7 +6,7 @@ import color = require("color"); import platform = require("platform"); import {ProxyViewContainer} from "ui/proxy-view-container"; -import {View, Button, StackLayout} from "ui"; +import {View, Button, LayoutBase, StackLayout, GridLayout} from "ui"; export function test_add_children_to_attached_proxy() { var outer = new StackLayout(); @@ -27,6 +27,34 @@ export function test_add_children_to_attached_proxy() { helper.buildUIAndRunTest(outer, testAction); } +export function test_children_immediately_registered_in_parent_grid_layout() { + var outer = new GridLayout(); + var proxy = new ProxyViewContainer(); + + function testAction(views: Array) { + outer.addChild(proxy); + proxy.addChild(createBtn("1")); + + assertNativeChildren(outer, ["1"]); + }; + + helper.buildUIAndRunTest(outer, testAction); +} + +export function test_children_registered_in_parent_grid_layout_on_attach() { + var outer = new GridLayout(); + var proxy = new ProxyViewContainer(); + + function testAction(views: Array) { + proxy.addChild(createBtn("1")); + outer.addChild(proxy); + + assertNativeChildren(outer, ["1"]); + }; + + helper.buildUIAndRunTest(outer, testAction); +} + export function test_add_children_to_detached_proxy() { var outer = new StackLayout(); var proxy = new ProxyViewContainer(); @@ -139,16 +167,16 @@ function createBtn(text: string): Button { return b; } -function assertNativeChildren(stack: StackLayout, arr: Array) { - if (stack.android) { - let android: org.nativescript.widgets.StackLayout = stack.android; +function assertNativeChildren(layout: LayoutBase, arr: Array) { + if (layout.android) { + let android: org.nativescript.widgets.LayoutBase = layout.android; TKUnit.assertEqual(android.getChildCount(), arr.length, "Native children"); for (let i = 0; i < arr.length; i++) { let nativeBtn = android.getChildAt(i); TKUnit.assertEqual(nativeBtn.getText(), arr[i]); } - } else if (stack.ios) { - let ios: UIView = stack.ios; + } else if (layout.ios) { + let ios: UIView = layout.ios; TKUnit.assertEqual(ios.subviews.count, arr.length, "Native children"); for (let i = 0; i < arr.length; i++) { let nativeBtn = ios.subviews[i]; diff --git a/ui/core/view-common.ts b/ui/core/view-common.ts index cb6e4e4fd..9a9f73a8c 100644 --- a/ui/core/view-common.ts +++ b/ui/core/view-common.ts @@ -961,6 +961,7 @@ export class View extends ProxyObject implements definition.View { view._parent = this; this._addViewCore(view, atIndex); + view._parentChanged(null); trace.write("called _addView on view " + this._domId + " for a child " + view._domId, trace.categories.ViewHierarchy); } @@ -1014,6 +1015,7 @@ export class View extends ProxyObject implements definition.View { this._removeViewCore(view); view._parent = undefined; + view._parentChanged(this); trace.write("called _removeView on view " + this._domId + " for a child " + view._domId, trace.categories.ViewHierarchy); } @@ -1045,6 +1047,10 @@ export class View extends ProxyObject implements definition.View { view._eachSetProperty(inheritablePropertiesSetCallback); } + public _parentChanged(oldParent: View): void { + //Overridden + } + /** * Method is intended to be overridden by inheritors and used as "protected". */ diff --git a/ui/core/view.d.ts b/ui/core/view.d.ts index e10a1dc0f..00c1b4aaf 100644 --- a/ui/core/view.d.ts +++ b/ui/core/view.d.ts @@ -509,6 +509,7 @@ declare module "ui/core/view" { // TODO: Implement logic for stripping these lines out //@private + _parentChanged(oldParent: View): void; _gestureObservers: any; _isInheritedChange(): boolean; _domId: number; diff --git a/ui/layouts/grid-layout/grid-layout.ios.ts b/ui/layouts/grid-layout/grid-layout.ios.ts index f0bd9c0d4..61ba6cc1a 100644 --- a/ui/layouts/grid-layout/grid-layout.ios.ts +++ b/ui/layouts/grid-layout/grid-layout.ios.ts @@ -35,24 +35,12 @@ export class GridLayout extends common.GridLayout { this.helper.columns.splice(index, 1); } - public addChild(child: View): void { - super.addChild(child); + public _registerLayoutChild(child: View) { this.addToMap(child); } - public insertChild(child: View, atIndex: number): void { - super.insertChild(child, atIndex); - this.addToMap(child); - } - - public removeChild(child: View): void { + public _unregisterLayoutChild(child: View) { this.removeFromMap(child); - super.removeChild(child); - } - - public removeChildren(): void { - this.map.clear(); - super.removeChildren(); } private getColumnIndex(view: View): number { @@ -1021,4 +1009,4 @@ class MeasureHelper { this.itemMeasured(measureSpec, false); } -} \ No newline at end of file +} diff --git a/ui/layouts/layout-base.d.ts b/ui/layouts/layout-base.d.ts index 346019148..207529d7e 100644 --- a/ui/layouts/layout-base.d.ts +++ b/ui/layouts/layout-base.d.ts @@ -50,6 +50,16 @@ */ removeChildren(): void; + /** + * INTERNAL. Used by the layout system. + */ + _registerLayoutChild(child: view.View): void; + + /** + * INTERNAL. Used by the layout system. + */ + _unregisterLayoutChild(child: view.View): void; + /** * Calls the callback for each child that should be laid out. * @param callback The callback @@ -95,4 +105,4 @@ */ paddingTop: number; } -} \ No newline at end of file +} diff --git a/ui/layouts/layout-base.ts b/ui/layouts/layout-base.ts index 97822bae8..3604fd93b 100644 --- a/ui/layouts/layout-base.ts +++ b/ui/layouts/layout-base.ts @@ -40,15 +40,25 @@ export class LayoutBase extends view.CustomLayoutView implements definition.Layo return view.getViewById(this, id); } + public _registerLayoutChild(child: view.View) { + //Overridden + } + + public _unregisterLayoutChild(child: view.View) { + //Overridden + } + public addChild(child: view.View): void { // TODO: Do we need this method since we have the core logic in the View implementation? this._subViews.push(child); this._addView(child); + this._registerLayoutChild(child); } public insertChild(child: view.View, atIndex: number): void { this._subViews.splice(atIndex, 0, child); this._addView(child, atIndex); + this._registerLayoutChild(child); } public removeChild(child: view.View): void { @@ -57,6 +67,7 @@ export class LayoutBase extends view.CustomLayoutView implements definition.Layo // TODO: consider caching the index on the child. var index = this._subViews.indexOf(child); this._subViews.splice(index, 1); + this._unregisterLayoutChild(child); } public removeChildren(): void { @@ -233,4 +244,4 @@ export class LayoutBase extends view.CustomLayoutView implements definition.Layo } } } -} \ No newline at end of file +} diff --git a/ui/proxy-view-container/proxy-view-container.ts b/ui/proxy-view-container/proxy-view-container.ts index 446122cd3..4ab99ef19 100644 --- a/ui/proxy-view-container/proxy-view-container.ts +++ b/ui/proxy-view-container/proxy-view-container.ts @@ -1,8 +1,8 @@ import types = require("utils/types"); -import view = require("ui/core/view"); +import {View} from "ui/core/view"; import definition = require("ui/proxy-view-container"); import trace = require("trace"); -import layout = require("ui/layouts/layout-base"); +import {LayoutBase} from "ui/layouts/layout-base"; /** * Proxy view container that adds all its native children directly to the parent. * To be used as a logical grouping container of views. @@ -16,7 +16,7 @@ import layout = require("ui/layouts/layout-base"); // * Proxy (with children) is removed form the DOM. // - IOS: Handled in _removeFromSuperview - when the proxy is removed, it removes all its children from its parent. // - Android: _onDetached calls _removeViewFromNativeVisualTree recursively when the proxy is removed from its parent. -export class ProxyViewContainer extends layout.LayoutBase implements definition.ProxyViewContainer { +export class ProxyViewContainer extends LayoutBase implements definition.ProxyViewContainer { // No native view for proxy container. get ios(): any { return null; @@ -51,15 +51,15 @@ export class ProxyViewContainer extends layout.LayoutBase implements definition. }); } - public _addViewToNativeVisualTree(child: view.View, atIndex?: number): boolean { - trace.write("ViewContainer._addViewToNativeVisualTree for a child " + view + " ViewContainer.parent: " + this.parent, trace.categories.ViewHierarchy); + public _addViewToNativeVisualTree(child: View, atIndex?: number): boolean { + trace.write("ViewContainer._addViewToNativeVisualTree for a child " + child + " ViewContainer.parent: " + this.parent, trace.categories.ViewHierarchy); super._addViewToNativeVisualTree(child); var parent = this.parent; if (parent) { let baseIndex = 0; let insideIndex = 0; - if (parent instanceof layout.LayoutBase) { + if (parent instanceof LayoutBase) { // Get my index in parent and convert it to native index. baseIndex = parent._childIndexToNativeChildIndex(parent.getChildIndex(this)); } @@ -78,8 +78,8 @@ export class ProxyViewContainer extends layout.LayoutBase implements definition. return false; } - public _removeViewFromNativeVisualTree(child: view.View): void { - trace.write("ProxyViewContainer._removeViewFromNativeVisualTree for a child " + view + " ViewContainer.parent: " + this.parent, trace.categories.ViewHierarchy); + public _removeViewFromNativeVisualTree(child: View): void { + trace.write("ProxyViewContainer._removeViewFromNativeVisualTree for a child " + child + " ViewContainer.parent: " + this.parent, trace.categories.ViewHierarchy); super._removeViewFromNativeVisualTree(child); var parent = this.parent; @@ -108,4 +108,43 @@ export class ProxyViewContainer extends layout.LayoutBase implements definition. return true; }); } + + /* + * Some layouts (e.g. GridLayout) need to get notified when adding and + * removing children, so that they can update private measure data. + * + * We register our children with the parent to avoid breakage. + */ + public _registerLayoutChild(child: View) { + if (this.parent instanceof LayoutBase) { + (this.parent)._registerLayoutChild(child); + } + } + + public _unregisterLayoutChild(child: View) { + if (this.parent instanceof LayoutBase) { + (this.parent)._unregisterLayoutChild(child); + } + } + + /* + * Register/unregister existing children with the parent layout. + */ + public _parentChanged(oldParent: View): void { + const addingToParent = this.parent && !oldParent; + const newLayout = this.parent; + const oldLayout = oldParent; + + if (addingToParent && newLayout instanceof LayoutBase) { + this._eachChildView((child) => { + newLayout._registerLayoutChild(child); + return true; + }); + } else if (oldLayout instanceof LayoutBase) { + this._eachChildView((child) => { + oldLayout._unregisterLayoutChild(child); + return true; + }); + } + } }