Skip to content

Commit 279b039

Browse files
author
vakrilov
committed
fix: subsequent *ngIf break renderer-a
1 parent 3db5cd1 commit 279b039

File tree

7 files changed

+174
-18
lines changed

7 files changed

+174
-18
lines changed

‎e2e/renderer/app/app-routing.module.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { NgIfNoLayoutComponent } from "./ngif-no-layout.component";
88
import { NgIfInbetweenComponent } from "./ngif-inbetween.component";
99
import { NgIfElseComponent } from "./ngifelse.component";
1010
import { NgIfThenElseComponent } from "./ngif-then-else.component";
11+
import { NgIfSubsequent } from "./ngif-subsequent.component";
1112
import { ContentViewComponent } from "./content-view.component";
1213

1314
export const routes = [
@@ -44,6 +45,10 @@ export const routes = [
4445
path: "ngif-then-else",
4546
component: NgIfThenElseComponent,
4647
},
48+
{
49+
path: "ngif-subsequent",
50+
component: NgIfSubsequent,
51+
},
4752
{
4853
path: "content-view",
4954
component: ContentViewComponent,
@@ -58,6 +63,7 @@ export const navigatableComponents = [
5863
NgIfInbetweenComponent,
5964
NgIfElseComponent,
6065
NgIfThenElseComponent,
66+
NgIfSubsequent,
6167
ContentViewComponent,
6268
];
6369

‎e2e/renderer/app/list.component.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { Component } from "@angular/core";
99
<Button text="NgIf inbetween" [nsRouterLink]="['/ngif-inbetween']"></Button>
1010
<Button text="NgIfElse" [nsRouterLink]="['/ngifelse']"></Button>
1111
<Button text="NgIf Then Else" [nsRouterLink]="['/ngif-then-else']"></Button>
12+
<Button text="NgIf Subsequent Ifs" [nsRouterLink]="['/ngif-subsequent']"></Button>
1213
<Button text="Content view" [nsRouterLink]="['/content-view']"></Button>
1314
</FlexboxLayout>
1415
`

‎e2e/renderer/app/ngif-subsequent.component.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { Component} from "@angular/core";
2+
3+
@Component({
4+
moduleId: module.id,
5+
selector: "renderer-test",
6+
template: `
7+
<StackLayout>
8+
<Button text="Toggle first" (tap)="first = !first"></Button>
9+
<Button text="Toggle second" (tap)="second = !second"></Button>
10+
11+
<Label text="== 1 =="></Label>
12+
13+
<Label *ngIf="first" text="first"></Label>
14+
<Label *ngIf="second" text="second"></Label>
15+
16+
<Label text="== 2 =="></Label>
17+
18+
</StackLayout>`
19+
})
20+
export class NgIfSubsequent {
21+
public first: boolean = false;
22+
public second: boolean = false;
23+
}

‎e2e/renderer/e2e/ngif.e2e-spec.ts

Lines changed: 131 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ describe("ngIf scenario", () => {
8989

9090
try {
9191
await driverWrapper.findElementByText("Label", SearchOptions.exact);
92-
} catch(e) {
92+
} catch (e) {
9393
done();
9494
}
9595
})();
@@ -152,7 +152,7 @@ describe("ngIf scenario", () => {
152152

153153
try {
154154
await driverWrapper.findElementByText("Else", SearchOptions.exact);
155-
} catch(e) {
155+
} catch (e) {
156156
done();
157157
}
158158
})();
@@ -215,7 +215,7 @@ describe("ngIf scenario", () => {
215215

216216
try {
217217
await driverWrapper.findElementByText("Else", SearchOptions.exact);
218-
} catch(e) {
218+
} catch (e) {
219219
done();
220220
}
221221
})();
@@ -269,7 +269,7 @@ describe("ngIf scenario", () => {
269269
.then(_ => { throw new Error("Then template found!"); })
270270
.catch(() => done());
271271
});
272-
272+
273273
it("should swap the content when condition is changed", done => {
274274
(async () => {
275275
toggleButton = await toggleButton.refetch();
@@ -278,10 +278,136 @@ describe("ngIf scenario", () => {
278278

279279
try {
280280
await driverWrapper.findElementByText("Else", SearchOptions.exact);
281-
} catch(e) {
281+
} catch (e) {
282282
done();
283283
}
284284
})();
285285
});
286286
});
287+
288+
describe("subsequent ifs", async () => {
289+
let firstButton: ExtendedUIElement;
290+
let secondButton: ExtendedUIElement;
291+
let firstLabel: ExtendedUIElement;
292+
let secondLabel: ExtendedUIElement;
293+
294+
before(async () => {
295+
driver = await createDriver();
296+
driverWrapper = new DriverWrapper(driver);
297+
});
298+
299+
after(async () => {
300+
await driver.quit();
301+
console.log("Driver quits!");
302+
});
303+
304+
it("should navigate to page", async () => {
305+
const navigationButton =
306+
await driverWrapper.findElementByText("NgIf Subsequent Ifs", SearchOptions.exact);
307+
await navigationButton.click();
308+
});
309+
310+
it("should find elements", async () => {
311+
firstButton = await driverWrapper.findElementByText("Toggle first", SearchOptions.exact);
312+
secondButton = await driverWrapper.findElementByText("Toggle second", SearchOptions.exact);
313+
314+
firstLabel = await driverWrapper.findElementByText("== 1 ==", SearchOptions.exact);
315+
secondLabel = await driverWrapper.findElementByText("== 2 ==", SearchOptions.exact);
316+
317+
assert.isDefined(firstButton);
318+
assert.isDefined(secondButton);
319+
assert.isDefined(firstLabel);
320+
assert.isDefined(secondLabel);
321+
});
322+
323+
it("should toggle on first view", async () => {
324+
await firstButton.click();
325+
326+
let conditional = await driverWrapper.findElementByText("first", SearchOptions.exact);
327+
328+
await isAbove(firstLabel, conditional);
329+
await isAbove(conditional, secondLabel);
330+
});
331+
332+
it("should toggle off first view", done => {
333+
(async () => {
334+
await firstButton.click();
335+
336+
driverWrapper.findElementByText("first", SearchOptions.exact, 500)
337+
.then(_ => { throw new Error("first label found!"); })
338+
.catch(() => done());
339+
})();
340+
});
341+
342+
it("should toggle on second view", async () => {
343+
await secondButton.click();
344+
345+
let conditional = await driverWrapper.findElementByText("second", SearchOptions.exact);
346+
await isAbove(firstLabel, conditional);
347+
await isAbove(conditional, secondLabel);
348+
});
349+
350+
it("should toggle off second view", done => {
351+
(async () => {
352+
await secondButton.click();
353+
354+
driverWrapper.findElementByText("first", SearchOptions.exact, 500)
355+
.then(_ => { throw new Error("first label found!"); })
356+
.catch(() => done());
357+
})();
358+
});
359+
360+
it("should toggle on both views", async () => {
361+
await firstButton.click();
362+
await secondButton.click();
363+
364+
let conditional1 = await driverWrapper.findElementByText("first", SearchOptions.exact);
365+
let conditional2 = await driverWrapper.findElementByText("second", SearchOptions.exact);
366+
await isAbove(firstLabel, conditional1);
367+
await isAbove(conditional1, conditional2);
368+
await isAbove(conditional2, secondLabel);
369+
});
370+
371+
it("should toggle off both views", done => {
372+
(async () => {
373+
await firstButton.click();
374+
await secondButton.click();
375+
376+
driverWrapper.findElementByText("first", SearchOptions.exact, 500)
377+
.then(_ => { throw new Error("first label found!"); })
378+
.catch(() => {
379+
driverWrapper.findElementByText("second", SearchOptions.exact, 500)
380+
.then(_ => { throw new Error("second label found!"); })
381+
.catch(() => done());
382+
});
383+
})();
384+
});
385+
386+
it("should toggle on both views in reverse", async () => {
387+
await secondButton.click();
388+
await firstButton.click();
389+
390+
let conditional1 = await driverWrapper.findElementByText("first", SearchOptions.exact);
391+
let conditional2 = await driverWrapper.findElementByText("second", SearchOptions.exact);
392+
await isAbove(firstLabel, conditional1);
393+
await isAbove(conditional1, conditional2);
394+
await isAbove(conditional2, secondLabel);
395+
});
396+
397+
it("should toggle off both views in reverse", done => {
398+
(async () => {
399+
await secondButton.click();
400+
await firstButton.click();
401+
402+
driverWrapper.findElementByText("first", SearchOptions.exact, 500)
403+
.then(_ => { throw new Error("first label found!"); })
404+
.catch(() => {
405+
driverWrapper.findElementByText("second", SearchOptions.exact, 500)
406+
.then(_ => { throw new Error("second label found!"); })
407+
.catch(() => done());
408+
});
409+
})();
410+
});
411+
});
412+
287413
});

‎nativescript-angular/element-registry.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,6 @@ export interface ViewExtensions {
1414
ngCssClasses: Map<string, boolean>;
1515
}
1616

17-
export interface ElementReference {
18-
previous: NgView;
19-
next: NgView;
20-
}
21-
2217
export interface ViewClass {
2318
new (): View;
2419
}

‎nativescript-angular/renderer.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { profile } from "tns-core-modules/profiling";
1313
import { APP_ROOT_VIEW, DEVICE, getRootPage } from "./platform-providers";
1414
import { isBlank } from "./lang-facade";
1515
import { ViewUtil } from "./view-util";
16-
import { NgView, InvisibleNode, ElementReference, isDetachedElement } from "./element-registry";
16+
import { NgView, InvisibleNode, isDetachedElement } from "./element-registry";
1717
import { rendererLog as traceLog } from "./trace";
1818

1919
// CONTENT_ATTR not exported from NativeScript_renderer - we need it for styles application.
@@ -23,6 +23,11 @@ export const HOST_ATTR = `_nghost-${COMPONENT_VARIABLE}`;
2323
export const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`;
2424
const ATTR_SANITIZER = /-/g;
2525

26+
export interface ElementReference {
27+
previous: NgView;
28+
next: NgView;
29+
}
30+
2631
@Injectable()
2732
export class NativeScriptRendererFactory implements RendererFactory2 {
2833
private componentRenderers = new Map<string, NativeScriptRenderer>();
@@ -93,7 +98,7 @@ export class NativeScriptRenderer extends Renderer2 {
9398
@profile
9499
insertBefore(parent: NgView, newChild: NgView, { previous, next }: ElementReference): void {
95100
traceLog(`NativeScriptRenderer.insertBefore child: ${newChild} ` +
96-
`parent: ${parent} previous: ${previous} next: ${next}`);
101+
`parent: ${parent} previous: ${previous} next: ${next}`);
97102
this.viewUtil.insertChild(parent, newChild, previous, next);
98103
}
99104

@@ -119,14 +124,9 @@ export class NativeScriptRenderer extends Renderer2 {
119124
nextSibling(node: NgView): ElementReference {
120125
traceLog(`NativeScriptRenderer.nextSibling of ${node} is ${node.nextSibling}`);
121126

122-
let next = node.nextSibling;
123-
while (next && isDetachedElement(next)) {
124-
next = next.nextSibling;
125-
}
126-
127127
return {
128128
previous: node,
129-
next,
129+
next: node.nextSibling,
130130
};
131131
}
132132

‎nativescript-angular/view-util.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,11 @@ export class ViewUtil {
122122
this.removeLayoutChild(parent, child);
123123
}
124124

125+
// Find next actual element
126+
while (next && isDetachedElement(next)) {
127+
next = next.nextSibling;
128+
}
129+
125130
if (next) {
126131
const index = parent.getChildIndex(next);
127132
parent.insertChild(child, index);

0 commit comments

Comments
 (0)