Skip to content

[Glimmer 2] Dasherize someClass to some-class#13401

Merged
wycats merged 1 commit intoemberjs:masterfrom
chadhietala:class-names
Apr 27, 2016
Merged

[Glimmer 2] Dasherize someClass to some-class#13401
wycats merged 1 commit intoemberjs:masterfrom
chadhietala:class-names

Conversation

@chadhietala
Copy link
Contributor

@chadhietala chadhietala commented Apr 23, 2016

This will dasherize someClass to some-class when a binding is set on
a component invocation for the class attribute.

@chadhietala chadhietala force-pushed the class-names branch 3 times, most recently from d270b34 to 70e8fe6 Compare April 25, 2016 17:34

Choose a reason for hiding this comment

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

Wouldn't it be better to use className as it is when explicitly passed? (Ideally to use dasherized attribute name by default for helpers as well, but feels like it is not possible?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The behavior today in Ember is if {{foo-bar class=fooBar}} is executed and fooBar evaluates to true we have to reflect on the binding e.g. fooBar and turn the binding name to foo-bar.

Choose a reason for hiding this comment

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

Yeah, I meant that for case class=fooBar it should transform into foo-bar, but in case (-class fooBar "fooBar"), when class passed explicitly by consumer, it should stay fooBar (to solve this you could pass dasherized version from the component and do not dasherize in the helper)

Copy link
Contributor

Choose a reason for hiding this comment

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

the last arg is the name of the ref passed in, the ref is the className unless it is a boolean.

Copy link
Contributor

Choose a reason for hiding this comment

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

another way to think of it is the last arg is metadata, essential reflection of the path that was bound. the first argument is the className binding, this is an internal helper refined from the class={{boundProp}} to add this additional metadata to handle the case of the boundProp not being a string but a boolean

@chadhietala
Copy link
Contributor Author

This will supersede #13338

Copy link
Contributor

Choose a reason for hiding this comment

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

this should be propName since it isn't actually the class name at this point

This will dasherize `someClass` to `some-class` when a binging is set on
a component invocation for the class attribute.
return dasherize(propName.value());
}

if (value === false) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once glimmerjs/glimmer-vm#150 lands this can go away.

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