Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure StyleElementCustomPropertyManager works with disconnected elements #4112

Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 2 additions & 2 deletions packages/web-components/fast-foundation/docs/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ export interface CustomPropertyManager {
}

// @public
export interface CustomPropertyManagerClient extends FASTElement {
export interface CustomPropertyManagerClient extends FASTElement, HTMLElement {
cssCustomPropertyDefinitions: Map<string, CSSCustomPropertyDefinition>;
evaluate(definition: CSSCustomPropertyDefinition): string;
}
Expand Down Expand Up @@ -782,7 +782,7 @@ export class StyleElementCustomPropertyManager extends CustomPropertyManagerBase
// (undocumented)
protected customPropertyTarget: CSSStyleDeclaration;
// (undocumented)
readonly sheet: CSSStyleSheet;
get sheet(): CSSStyleSheet | null;
// (undocumented)
readonly styles: HTMLStyleElement;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,39 @@ describe("StyleElementCustomPropertyManager", () => {
await disconnect();
});

it("should not throw when constructed with a disconnected element", () => {
const element = document.createElement("fast-client") as Client;

expect(() => {
new StyleElementCustomPropertyManager(
document.createElement("style"),
element
);
}).not.to.throw();
});

it("should queue and apply properties set prior to connection once connected", () => {
const element = document.createElement("fast-client") as Client;
element.cssCustomPropertyDefinitions.set("my-property", {
name: "my-property",
value: "value",
});
const client = new StyleElementCustomPropertyManager(
document.createElement("style"),
element
);

assert.equal(
window.getComputedStyle(element).getPropertyValue("--my-property"),
""
);
document.body.appendChild(element);
assert.equal(
window.getComputedStyle(element).getPropertyValue("--my-property"),
"value"
);
});

it("should connect the style element to the DOM during construction", async () => {
const { element, connect, disconnect } = await setup();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { DOM, ElementStyles, FASTElement } from "@microsoft/fast-element";
import {
DOM,
ElementStyles,
FASTElement,
Observable,
observable,
} from "@microsoft/fast-element";
import { CSSCustomPropertyDefinition } from "./behavior";

const hostSelector = ":host{}";
Expand All @@ -9,7 +15,7 @@ const hostSelector = ":host{}";
*
* @public
*/
export interface CustomPropertyManagerClient extends FASTElement {
export interface CustomPropertyManagerClient extends FASTElement, HTMLElement {
/**
* All registered CSSCustomPropertyDefinitions.
*/
Expand Down Expand Up @@ -87,10 +93,15 @@ export interface CustomPropertyManager {
}

abstract class CustomPropertyManagerBase implements CustomPropertyManager {
/**
* A queue of additions and deletions. Operations will be queued when customPropertyTarget is null
*/
protected queue: Set<Function> = new Set();

/**
* The CSSStyleDeclaration to which all CSS custom properties are written
*/
protected abstract customPropertyTarget: CSSStyleDeclaration;
protected abstract customPropertyTarget: CSSStyleDeclaration | null = null;

/**
* {@inheritdoc CustomPropertyManager.owner}
Expand Down Expand Up @@ -153,25 +164,29 @@ abstract class CustomPropertyManagerBase implements CustomPropertyManager {
/**
* {@inheritdoc CustomPropertyManager.set}
*/
public set = (definition: CSSCustomPropertyDefinition) => {
public set(definition: CSSCustomPropertyDefinition) {
if (this.owner) {
this.customPropertyTarget.setProperty(
`--${definition.name}`,
this.owner.evaluate(definition)
);
this.customPropertyTarget
? this.customPropertyTarget.setProperty(
`--${definition.name}`,
this.owner.evaluate(definition)
)
: this.queue.add(this.set.bind(this, definition));
}
};
}

/**
* Removes a CSS custom property from the provider.
* @param name - the name of the property to remove
*/
public remove = (name: string): void => {
this.customPropertyTarget.removeProperty(`--${name}`);
};
public remove(name: string): void {
this.customPropertyTarget
? this.customPropertyTarget.removeProperty(`--${name}`)
: this.queue.add(this.remove.bind(this, name));
}

/**
* {@inheritdoc CustomPropertyManager.set}
* {@inheritdoc CustomPropertyManager.setAll}
*/
public setAll() {
if (this.ticking) {
Expand Down Expand Up @@ -262,28 +277,64 @@ export class ConstructableStylesCustomPropertyManager extends CustomPropertyMana
* @public
*/
export class StyleElementCustomPropertyManager extends CustomPropertyManagerBase {
public readonly sheet: CSSStyleSheet;
private _sheet: CSSStyleSheet | null = null;
public get sheet(): CSSStyleSheet | null {
return this._sheet;
}

@observable
protected customPropertyTarget: CSSStyleDeclaration;
private customPropertyTargetChanged(
prev: CSSStyleDeclaration | null,
next: CSSStyleDeclaration | null
) {
if (!prev && this.queue.size) {
this.queue.forEach(fn => fn());
this.queue.clear();
}
}

public readonly styles: HTMLStyleElement;

constructor(style: HTMLStyleElement, client: CustomPropertyManagerClient) {
super();
const controller = client.$fastController;

// For HTMLStyleElements we need to attach the element
// to the DOM prior to accessing the HTMLStyleElement.sheet
// because the property evaluates null if disconnected
client.$fastController.addStyles(style);
this.sheet = style.sheet!;
this.styles = style;

this.customPropertyTarget = (this.sheet.rules[
this.sheet.insertRule(hostSelector)
] as CSSStyleRule).style;
controller.addStyles(style);

this.styles = style;
this._owner = client;

// If the element isn't connected when the manager is created, the sheet can be null.
// In those cases, set up notifier for when the element is connected and set up the customPropertyTarget
// then.
client.isConnected
? this.handleConnection.handleChange()
: Observable.getNotifier(controller).subscribe(
this.handleConnection,
"isConnected"
);

client.cssCustomPropertyDefinitions.forEach(def => {
this.register(def);
});
}

private handleConnection = {
handleChange: () => {
this._sheet = this.styles.sheet!;

this.customPropertyTarget = (this.sheet!.rules[
this.sheet!.insertRule(hostSelector)
] as CSSStyleRule).style;

Observable.getNotifier(this._owner?.$fastController).unsubscribe(
this.handleConnection,
"isConnected"
);
},
};
}