Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions packages/core/ui/core/properties/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,26 @@ const enum ValueSource {
Keyframe = 4,
}

/** `sourceKey` of every registered `CssProperty`, by css name. */
const cssValueSourceKeys: Record<string, symbol> = Object.create(null);

/**
* Whether the style still carries what the cascade last wrote for the property.
* A `CssProperty` keeps one value: a local value both suppresses the css write and
* takes the slot, so clearing it leaves the property at its default with the css
* value gone. The cascade has to write it again rather than skip it as unchanged.
*/
export function _isCssValueStillApplied(style: unknown, cssLocalName: string, value: unknown): boolean {
const sourceKey = cssValueSourceKeys[cssLocalName];
if (sourceKey === undefined) {
// Applied through the view rather than the style; nothing to verify.
return true;
}

// A reset leaves no source behind, so it is in effect precisely when nothing else claimed the property.
return style[sourceKey] === (isResetValue(value) ? undefined : ValueSource.Css);
}

function print(map) {
const symbols = Object.getOwnPropertySymbols(map);
for (const symbol of symbols) {
Expand Down Expand Up @@ -755,6 +775,8 @@ export class CssProperty<T extends Style, U> {
return;
}

this._localValueVersion++;

const reset = isResetValue(newValue) || newValue === '';
let value: U;

Expand Down Expand Up @@ -939,6 +961,8 @@ export class CssProperty<T extends Style, U> {
if (this.cssLocalName !== this.cssName) {
Object.defineProperty(cls.prototype, this.cssLocalName, this.localValueDescriptor);
}

cssValueSourceKeys[this.cssLocalName] = this.sourceKey;
}

public isSet(instance: T): boolean {
Expand Down Expand Up @@ -1205,14 +1229,20 @@ export class InheritedCssProperty<T extends Style, U> extends CssProperty<T, U>
}
};

const setFunc = (valueSource: ValueSource) =>
function (this: T, boxedValue: any): void {
const setFunc = (valueSource: ValueSource) => {
const isLocalWrite = valueSource === ValueSource.Local;

return function (this: T, boxedValue: any): void {
const view = this.viewRef.get();
if (!view) {
Trace.write(`${boxedValue} not set to view's property because ".viewRef" is cleared`, Trace.categories.Style, Trace.messageType.warn);
return;
}

if (isLocalWrite) {
this._localValueVersion++;
}

const reset = isResetValue(boxedValue) || boxedValue === '';
const currentValueSource: number = this[sourceKey] || ValueSource.Default;

Expand Down Expand Up @@ -1314,6 +1344,7 @@ export class InheritedCssProperty<T extends Style, U> extends CssProperty<T, U>
});
}
};
};

const setDefaultFunc = setFunc(ValueSource.Default);
const setInheritedFunc = setFunc(ValueSource.Inherited);
Expand Down
63 changes: 63 additions & 0 deletions packages/core/ui/styling/css-attribute-match.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { describe, it, expect } from 'vitest';

import { StyleScope } from './style-scope';
import { StackLayout } from '../layouts/stack-layout';
import { Label } from '../label';

function scoped(css: string) {
const scope = new StyleScope();
scope.css = css;

const root: any = new StackLayout();
root._styleScope = scope;

return { scope, root };
}

describe('css matches an attribute assigned after the view was inserted', () => {
function insertedIntoLoadedTree(css: string) {
const { scope, root } = scoped(css);
root._isLoaded = true;

const view: any = new Label();
root.addChild(view);
view._styleScope = scope;

return { root, view };
}

it('applies once the attribute lands on the view', () => {
const { view } = insertedIntoLoadedTree('label[ngcontent] { color: red; }');

view.ngcontent = '';
view._cssState.onLoaded();

expect(view.style.color.toString()).toBe('#FF0000');
});

it('applies once the attribute lands on an ancestor', () => {
const { root, view } = insertedIntoLoadedTree('stacklayout[ngcontent] label { color: red; }');

root.ngcontent = '';
view._cssState.onLoaded();

expect(view.style.color.toString()).toBe('#FF0000');
});

it('applies on a later dynamic update', () => {
const { view } = insertedIntoLoadedTree('label[ngcontent] { color: red; }');

view.ngcontent = '';
view._cssState.updateDynamicState();

expect(view.style.color.toString()).toBe('#FF0000');
});

it('leaves a view without the attribute alone', () => {
const { view } = insertedIntoLoadedTree('label[ngcontent] { color: red; }');

view._cssState.onLoaded();

expect(view.style.color).toBeUndefined();
});
});
15 changes: 14 additions & 1 deletion packages/core/ui/styling/css-selector.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,20 @@ describe('css-selector', () => {
const rule = createOne(`.title[_ngcontent-c7] { color: red; }`);
const node = { cssType: 'label', cssClasses: new Set(['title']), '_ngcontent-c3': '' };

expect(rule.selectors[0].accumulateChanges(<any>node, undefined)).toBe(false);
expect(rule.selectors[0].match(<any>node)).toBe(false);
});

it('stays a candidate so it can match once the attribute is assigned', () => {
// Assigning a plain instance value raises no change event, so a selector
// dropped from the match would never be reconsidered.
const rule = createOne(`.title[_ngcontent-c7] { color: red; }`);
const node: any = { cssType: 'label', cssClasses: new Set(['title']) };

expect(rule.selectors[0].accumulateChanges(node, undefined)).toBe(true);

node['_ngcontent-c7'] = '';

expect(rule.selectors[0].match(node)).toBe(true);
});

it('matches a node carrying the attribute', () => {
Expand Down
9 changes: 5 additions & 4 deletions packages/core/ui/styling/css-selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,11 @@ export class AttributeSelector extends SimpleSelector {
return false;
}
public mayMatch(node: Node): boolean {
// Registered properties live on the prototype and anything already assigned is
// an own value; an attribute on neither can never start matching without the
// css state being invalidated anyway.
return this.attribute in node;
// An attribute the node does not carry yet cannot be ruled out: assigning a
// plain instance value (Angular's `_ngcontent-*` markers, `[attr.x]` bindings)
// neither raises a change event nor invalidates the match, so a selector
// dropped here would never be reconsidered.
return true;
}
public trackChanges(node: Node, map: ChangeAccumulator): void {
if (attributeNotifiesChanges(node, this.attribute)) {
Expand Down
160 changes: 160 additions & 0 deletions packages/core/ui/styling/css-state-recovery.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import { describe, it, expect } from 'vitest';

import { StyleScope, applyInlineStyle } from './style-scope';
import { StackLayout } from '../layouts/stack-layout';
import { Label } from '../label';
import { unsetValue } from '../core/properties';

function scoped(css: string) {
const scope = new StyleScope();
scope.css = css;

const root: any = new StackLayout();
root._styleScope = scope;

return { scope, root };
}

function loaded(css: string) {
const { scope, root } = scoped(css);

const view: any = new Label();
root.addChild(view);
view._styleScope = scope;
view._cssState.onLoaded();

return view;
}

function restyle(view: any): void {
view._cssState.onChange();
view._cssState.onLoaded();
}

/** Counts css writes per `<view>.<property>`, reporting only the ones that happened. */
function countWrites(targets: Array<[view: any, cssProperty: string]>): () => Record<string, number> {
const counts: Record<string, number> = {};

for (const [view, cssProperty] of targets) {
const style = view.style;
const key = `${view.cssType}.${cssProperty}`;
const descriptor = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(style), `css:${cssProperty}`);

Object.defineProperty(style, `css:${cssProperty}`, {
configurable: true,
get: descriptor.get,
set(value: unknown) {
counts[key] = (counts[key] ?? 0) + 1;
descriptor.set.call(this, value);
},
});
}

return () => counts;
}

describe('css survives a local value that shadows it', () => {
it('restores a longhand after a local value is released', () => {
const view = loaded('label { color: red; }');
expect(view.style.color.toString()).toBe('#FF0000');

view.style.color = 'blue';
view.style.color = unsetValue;
view._cssState.updateDynamicState();

expect(view.style.color.toString()).toBe('#FF0000');
});

it.each([
['a shorthand', 'label { margin: 4; }'],
['a longhand', 'label { margin-top: 4; }'],
])('restores margin from %s after a local value is released', (_name, css) => {
const view = loaded(css);
expect(view.style.marginTop).toBe(4);

view.style.marginTop = 20;
view.style.marginTop = unsetValue;
view._cssState.updateDynamicState();

expect(view.style.marginTop).toBe(4);
});

it('restores padding after a local value is released', () => {
const view = loaded('label { padding: 4; }');

view.style.paddingTop = 20;
view.style.paddingTop = unsetValue;
view._cssState.updateDynamicState();

expect(view.style.paddingTop).toBe(4);
expect(view.style.paddingInternal).toBe('4 4 4 4');
});

it('restores a value a local one shadowed before css ever ran', () => {
const { scope, root } = scoped('label { margin: 4; }');

const view: any = new Label();
view.style.marginTop = 20;
root.addChild(view);
view._styleScope = scope;
view._cssState.onLoaded();
expect(view.style.marginTop).toBe(20);

view.style.marginTop = unsetValue;
view._cssState.updateDynamicState();

expect(view.style.marginTop).toBe(4);
});

it('restores a value an inline style shadowed', () => {
const view = loaded('label { margin: 4; }');

applyInlineStyle(view, 'margin-top: 20');
expect(view.style.marginTop).toBe(20);

view.style.marginTop = unsetValue;
view._cssState.updateDynamicState();

expect(view.style.marginTop).toBe(4);
});

it('restores a value on the next full re-match', () => {
const view = loaded('label { margin: 4; } .a { color: red; }');

view.style.marginTop = 20;
view.style.marginTop = unsetValue;

view.className = 'a';
restyle(view);

expect(view.style.marginTop).toBe(4);
});

it('still skips every value nothing disturbed', () => {
// Verifying the write took effect must not cost a re-application: inherited
// values, expanded longhands and the ones a shorthand left unset included.
const { scope, root } = scoped('stacklayout { color: red; font-size: 12; } label { margin: 4; background: blue; }');

const view: any = new Label();
root.addChild(view);
view._styleScope = scope;
root._cssState.onLoaded();
view._cssState.onLoaded();

const writes = countWrites([
[root, 'color'],
[root, 'font-size'],
[view, 'color'],
[view, 'margin-top'],
[view, 'background-color'],
[view, 'background-image'],
]);

for (let i = 0; i < 3; i++) {
root._cssState.updateDynamicState();
view._cssState.updateDynamicState();
}

expect(writes()).toEqual({});
});
});
Loading
Loading