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 the class directive #1685

Merged
merged 3 commits into from
Aug 25, 2018
Merged

Adds the class directive #1685

merged 3 commits into from
Aug 25, 2018

Conversation

jacwright
Copy link
Contributor

Allows <div class:active="user.active"> to simplify templates littered with ternary statements.

Addresses #890

Allows `<div class:active="user.active">` to simplify templates littered with ternary statements.

Addresses #890

export function toggleClass(element, name, toggle) {
element.classList.toggle(name, Boolean(toggle));
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the Boolean here? In Chrome at least it coerce the value for us

Copy link
Member

Choose a reason for hiding this comment

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

Update: tested in Chrome, Firefox and Safari. I reckon we can do without — I'll delete it

@Rich-Harris
Copy link
Member

Thank you!

@jacwright
Copy link
Contributor Author

@Rich-Harris the Boolean is necessary. The toggle method takes an optional second parameter to force the class to be toggled on or off, depending if that value is truthy or falsey, otherwise it will just toggle the class. If the value is undefined then the default behavior is run. E.g.

node.classList.toggle('foo', true); // class="foo"
node.classList.toggle('foo', undefined); // class=""
node.classList.toggle('foo', false); // class=""
node.classList.toggle('foo', undefined); // class="foo"

The Boolean ensures that undefined is treated falsey instead of as an empty parameter.

@lukeed
Copy link
Member

lukeed commented Aug 25, 2018

^ Yup! It bugs me that undefined is not treated as falsey in this case 😠

We can do !!toggle to save bytes over Boolean.

@jacwright
Copy link
Contributor Author

Oh yeah, of course. !! is better.

@tivac
Copy link
Contributor

tivac commented Aug 25, 2018

Disagree on !! being better than Boolean(), any minifier worth its salt should do that for you and Boolean() is much more clear for anyone reading the code.

Since it's generated code it probably isn't very important but I'll always advocate for code clarity over tricks, even well-known ones.

@Rich-Harris
Copy link
Member

Whoops. Good catch. Fixed in 2.13.1 — I went with !!, since Uglify/Terser don't rewrite Boolean(...) (though Closure does)

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.

None yet

4 participants