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

Adds classMap and styleMap directives #486

Merged
merged 2 commits into from
Sep 11, 2018
Merged

Adds classMap and styleMap directives #486

merged 2 commits into from
Sep 11, 2018

Conversation

sorvell
Copy link
Member

@sorvell sorvell commented Sep 10, 2018

Fixes #467

for (const name in oldInfo) {
if (!(name in styleInfo)) {
// setting to `null` to "unset" a previously applied value.
((part.committer.element as HTMLElement).style as any)[name] = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

element.style.foo = null does not work in IE11 :( you have to use foo.style.removeProperty(foo)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work in testing. Do you know if there are there certain properties for which this doesn't work? (removeProperty doesn't accept the camelCased property name, so would love to avoid that if possible).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try this jsbin: http://jsbin.com/kosikijule/edit?html,output the background color should be removed after 2 sec

I notice now that that it works for height but not for backgroundColor, but it can't be due to casing because I've had this issue a lot with properties like overflow when writing css animations.

I understand not wanting to use removeProperty as you'd need to convert camel to dash...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can reset by setting the CSS property to an empty string, that should work everywhere including IE11. e.g.
element.style.foo = '';

* sets these properties to the element's style.
* @param styleInfo {StyleInfo}
*/
export const styleMap = (styleInfo: StyleInfo): Directive<AttributePart> =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should split these into separate modules

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

const classMapStatics = new WeakSet();

/**
* A directive that applies css classes. This must be used in the `class` attribute.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

css -> CSS

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

[name: string]: string;
}

const classMapCache = new WeakMap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document these

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// handle static classes
if (!classMapStatics.has(part)) {
part.committer.element.className =
part.committer.strings.reduce((a, c) => `${a} ${c}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use join instead of reduce

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// handle static styles
if (!styleMapStatics.has(part)) {
(part.committer.element as HTMLElement).style.cssText =
part.committer.strings.reduce((a, c) => `${a} ${c}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use join

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

});

const styleMapCache = new WeakMap();
const styleMapStatics = new WeakSet();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment these

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@justinfagnani justinfagnani merged commit b6da58d into master Sep 11, 2018
@justinfagnani justinfagnani deleted the css-directives branch September 11, 2018 22:35
neuronetio pushed a commit to neuronetio/lit-html that referenced this pull request Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants