Skip to content

Commit 6d7c1ff

Browse files
PanayotCankovSvetoslavTsenov
PanayotCankov
authored and
SvetoslavTsenov
committed
Avoid applying CSS multiple times (NativeScript#4784)
* Move the applyStyleFromScope to onLoaded, when the views are created and id or className properties are set the CSS selectors are queried and applied multiple times * Condense the changes when applying properties
1 parent b057772 commit 6d7c1ff

File tree

25 files changed

+531
-519
lines changed

25 files changed

+531
-519
lines changed

‎apps/app/ui-tests-app/main-page.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,4 @@
1515
</ListView.itemTemplate>
1616
</ListView>
1717
</GridLayout>
18-
</Page>
18+
</Page>

‎tests/app/testRunner.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ function showReportPage(finalMessage: string) {
359359
setTimeout(() => {
360360
messageContainer.dismissSoftInput();
361361
(<android.view.View>messageContainer.nativeViewProtected).scrollTo(0, 0);
362-
});
362+
}, 10);
363363
}
364364
}
365365

‎tests/app/ui/lifecycle/lifecycle-tests.ts‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as helper from "../helper";
22
import * as btnCounter from "./pages/button-counter";
33
import * as TKUnit from "../../TKUnit";
4-
import { isIOS } from "tns-core-modules/platform";
4+
import { isIOS, isAndroid } from "tns-core-modules/platform";
55

66
// Integration tests that asser sertain runtime behavior, lifecycle events atc.
77

@@ -118,4 +118,51 @@ export function test_navigating_away_does_not_excessively_reset() {
118118
// NOTE: Recycling may mess this up so feel free to change the test,
119119
// but ensure a reasonable amount of native setters were called when the views navigate away
120120
assert(1);
121-
}
121+
}
122+
123+
export function test_css_sets_properties() {
124+
const page = helper.navigateToModule("ui/lifecycle/pages/page-two");
125+
const buttons = ["btn1", "btn2", "btn3", "btn4"].map(id => page.getViewById<btnCounter.Button>(id));
126+
buttons.forEach(btn => {
127+
TKUnit.assertEqual(btn.colorSetNativeCount, 1, `Expected ${btn.id}'s native color to propagated exactly once when inflating from xml.`);
128+
TKUnit.assertEqual(btn.colorPropertyChangeCount, 1, `Expected ${btn.id}'s colorChange to be fired exactly once when inflating from xml.`);
129+
});
130+
131+
buttons.forEach(btn => {
132+
btn.className = "";
133+
});
134+
135+
const expectedChangesAfterClearingClasses = [1, 2, 2, 2];
136+
for (var i = 0; i < buttons.length; i++) {
137+
TKUnit.assertEqual(buttons[i].colorSetNativeCount, expectedChangesAfterClearingClasses[i], `Expected ${buttons[i].id} native set after clear.`);
138+
TKUnit.assertEqual(buttons[i].colorPropertyChangeCount, expectedChangesAfterClearingClasses[i], `Expected ${buttons[i].id} change notifications after clear.`);
139+
}
140+
141+
buttons[0].className = "nocolor";
142+
buttons[1].className = "red";
143+
buttons[2].className = "blue";
144+
buttons[3].className = "red blue";
145+
146+
const expectedChangesAfterResettingClasses = [1, 3, 3, 3];
147+
for (let i = 0; i < buttons.length; i++) {
148+
TKUnit.assertEqual(buttons[i].colorSetNativeCount, expectedChangesAfterResettingClasses[i], `Expected ${buttons[i].id} native set after classes are reapplied.`);
149+
TKUnit.assertEqual(buttons[i].colorPropertyChangeCount, expectedChangesAfterResettingClasses[i], `Expected ${buttons[i].id} change notifications classes are reapplied.`);
150+
}
151+
152+
const stack: any = page.getViewById("stack");
153+
page.content = null;
154+
155+
for (let i = 0; i < buttons.length; i++) {
156+
TKUnit.assertEqual(buttons[i].colorSetNativeCount, expectedChangesAfterResettingClasses[i], `Expected ${buttons[i].id} native set to not be called when removed from page.`);
157+
TKUnit.assertEqual(buttons[i].colorPropertyChangeCount, expectedChangesAfterResettingClasses[i], `Expected ${buttons[i].id} change notifications for css properties to not occur when removed from page.`);
158+
}
159+
160+
page.content = stack;
161+
162+
// TODO: The check counts here should be the same as the counts before removing from the page.
163+
const expectedNativeSettersAfterReaddedToPage = isAndroid ? [2, 4, 4, 4] : expectedChangesAfterResettingClasses;
164+
for (let i = 0; i < buttons.length; i++) {
165+
TKUnit.assertEqual(buttons[i].colorSetNativeCount, expectedNativeSettersAfterReaddedToPage[i], `Expected ${buttons[i].id} native set to not be called when added to page.`);
166+
TKUnit.assertEqual(buttons[i].colorPropertyChangeCount, expectedChangesAfterResettingClasses[i], `Expected ${buttons[i].id} change notifications for css properties to not occur when added to page.`);
167+
}
168+
}

‎tests/app/ui/lifecycle/pages/button-counter.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ export class Button extends button.Button {
55
nativeBackgroundRedraws = 0;
66
backgroundInternalSetNativeCount = 0;
77
fontInternalSetNativeCount = 0;
8+
colorSetNativeCount = 0;
9+
colorPropertyChangeCount = 0;
10+
11+
constructor() {
12+
super();
13+
this.style.on("colorChange", () => this.colorPropertyChangeCount++);
14+
}
815

916
[view.backgroundInternalProperty.setNative](value) {
1017
this.backgroundInternalSetNativeCount++;
@@ -18,5 +25,9 @@ export class Button extends button.Button {
1825
this.nativeBackgroundRedraws++;
1926
super._redrawNativeBackground(value);
2027
}
28+
[view.colorProperty.setNative](value) {
29+
this.colorSetNativeCount++;
30+
return super[view.colorProperty.setNative](value);
31+
}
2132
}
2233
Button.prototype.recycleNativeView = "never";

‎tests/app/ui/lifecycle/pages/page-two.css‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
Button {
2+
color: orange;
3+
}
4+
5+
.red {
6+
color: red;
7+
}
8+
9+
.blue {
10+
color: blue;
11+
}
12+
13+
.nocolor {
14+
background: red;
15+
}

‎tests/app/ui/lifecycle/pages/page-two.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<Page xmlns:btnCount="ui/lifecycle/pages/button-counter">
2+
<StackLayout id="stack">
3+
<btnCount:Button id="btn1" text="one" class="nocolor" />
4+
<btnCount:Button id="btn2" text="two" class="red" />
5+
<btnCount:Button id="btn3" text="three" class="blue" />
6+
<btnCount:Button id="btn4" text="four" class="red blue" />
7+
</StackLayout>
8+
</Page>

‎tests/app/ui/segmented-bar/segmented-bar-tests-native.ios.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,10 @@ export function getNativeItemsCount(bar: segmentedBarModule.SegmentedBar): numbe
55
}
66

77
export function checkNativeItemsTextColor(bar: segmentedBarModule.SegmentedBar): boolean {
8-
var isValid = true;
9-
108
var attrs = (<UISegmentedControl>bar.nativeViewProtected).titleTextAttributesForState(UIControlState.Normal);
11-
isValid = bar.color && attrs && attrs.valueForKey(NSForegroundColorAttributeName) === bar.color.ios;
12-
13-
return isValid;
9+
var nativeViewColor = bar.color && attrs && attrs.valueForKey(NSForegroundColorAttributeName);
10+
var barColor = bar.color.ios;
11+
return barColor.isEqual(nativeViewColor);
1412
}
1513

1614
export function setNativeSelectedIndex(bar: segmentedBarModule.SegmentedBar, index: number): void {

‎tests/app/ui/styling/style-tests.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export function test_applies_css_changes_to_application_rules_after_page_load()
6767
helper.buildUIAndRunTest(label1, function (views: Array<viewModule.View>) {
6868
application.addCss(".applicationChangedLabelAfter { color: blue; }");
6969
label1.className = "applicationChangedLabelAfter";
70+
console.log("IsLoaded: " + label1.isLoaded);
7071
helper.assertViewColor(label1, "#0000FF");
7172
});
7273
}
@@ -615,7 +616,7 @@ export function test_setInlineStyle_setsLocalValues() {
615616
stack.addChild(testButton);
616617

617618
helper.buildUIAndRunTest(stack, function (views: Array<viewModule.View>) {
618-
(<any>testButton)._applyInlineStyle("color: red;");
619+
(<any>testButton).style = "color: red;";
619620
helper.assertViewColor(testButton, "#FF0000");
620621
});
621622
}
@@ -624,7 +625,7 @@ export function test_setStyle_throws() {
624625
const testButton = new buttonModule.Button();
625626

626627
TKUnit.assertThrows(function () {
627-
(<any>testButton).style = "background-color: red;";
628+
(<any>testButton).style = {};
628629
}, "View.style property is read-only.");
629630
}
630631

‎tests/app/ui/styling/value-source-tests.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,19 @@ import * as helper from "../helper";
55
import * as TKUnit from "../../TKUnit";
66
import { unsetValue } from "tns-core-modules/ui/core/view";
77

8+
export var test_value_Inherited_after_unset = function () {
9+
let page = helper.getCurrentPage();
10+
page.css = "StackLayout { color: #FF0000; } .blue { color: #0000FF; }";
11+
let btn = new button.Button();
12+
let testStack = new stack.StackLayout();
13+
page.content = testStack;
14+
testStack.addChild(btn);
15+
btn.className = "blue";
16+
helper.assertViewColor(btn, "#0000FF");
17+
btn.className = "";
18+
helper.assertViewColor(btn, "#FF0000");
19+
}
20+
821
export var test_value_Inherited_stronger_than_Default = function () {
922
let page = helper.getCurrentPage();
1023
let btn = new button.Button();

‎tns-core-modules/ui/builder/component-builder/component-builder.ts‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,6 @@ const applyComponentCss = profile("applyComponentCss", (instance: View, moduleNa
125125
cssApplied = true;
126126
}
127127
}
128-
129-
if (!cssApplied) {
130-
// Called only to apply application css.
131-
// If we have page css (through file or cssAttribute) we have appCss applied.
132-
(<any>instance)._refreshCss();
133-
}
134128
}
135129
});
136130

@@ -211,13 +205,7 @@ export function setPropertyValue(instance: View, instanceModule: Object, exports
211205
instance[propertyName] = exports[propertyValue];
212206
}
213207
else {
214-
let attrHandled = false;
215-
if (!attrHandled && instance._applyXmlAttribute) {
216-
attrHandled = instance._applyXmlAttribute(propertyName, propertyValue);
217-
}
218-
if (!attrHandled) {
219-
instance[propertyName] = propertyValue;
220-
}
208+
instance[propertyName] = propertyValue;
221209
}
222210
}
223211

0 commit comments

Comments
 (0)