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

Consider class name format change #8

Closed
necolas opened this issue Jan 18, 2013 · 12 comments
Closed

Consider class name format change #8

necolas opened this issue Jan 18, 2013 · 12 comments

Comments

@necolas
Copy link
Contributor

necolas commented Jan 18, 2013

Liking the format that montagejs is using. The structures are identical but they reflect it differently:

.namespace-ComponentName-componentChild--modifier {}

e.g.

.suit-Button {}
.suit-Button--small {}

.suit-ButtonGroup {}
.suit-ButtonGroup-item {}
@kaelig
Copy link

kaelig commented Mar 3, 2013

I see you've adopted the montagejs notation on all your new suit components.

The only issue I would see is that using camelCase or PascalCase leaves one open to ambiguous interpretation (since the spec doesn't specify if identifiers should be case-sensitive: by implementation CSS became case-insensitive).

I find the montagejs notation more elegant than the one using double underscores, but also a lot less obvious.

@necolas
Copy link
Contributor Author

necolas commented Mar 3, 2013

Not sure what you mean. I don't see where the ambiguity is.

@kaelig
Copy link

kaelig commented Mar 3, 2013

Sorry my post wasn't very clear. One can write <div class="c-Module-childElement"> or <div class="c-module-childelement">. Both will work since CSS identifiers are case-insensitive in all browsers (excluding some from the NS4 era), leading to potential ambiguous implementations.

Using underscores and double dashes, on the other hand, leaves not doubt to how an identifier should be written.

Does that make more sense?

@necolas
Copy link
Contributor Author

necolas commented Mar 3, 2013

I'm not seeing that: http://codepen.io/anon/pen/AILal

@kaelig
Copy link

kaelig commented Mar 3, 2013

Wow, thanks for that wake up call. In my defense, it actually used to be different a decade ago, when I learned the basics of the language: https://developer.mozilla.org/en-US/docs/Case_Sensitivity_in_class_and_id_Names

However, I recommend this blog from Harry about camelCase — if you haven't already read it.

@necolas
Copy link
Contributor Author

necolas commented Mar 3, 2013

I think every claim in that blog post is dubious.

@kaelig
Copy link

kaelig commented Mar 3, 2013

I guess it's up to personal preference then, like the good old tabs versus spaces debate.

@necolas
Copy link
Contributor Author

necolas commented Mar 3, 2013

I don't think the author even holds to those claims these days.

CSS is a hyphen-delimited syntax

These are HTML class names. There's nothing inconsistent about using your own notation for class names. You only have to be consistent in that realm.

XHTML is a lower-case language

The class attribute accepts upper and lower case values.

Inconsistency within your own rules

This isn't an inconsistency, but, I use Pascal case for components anyway. You could argue that it is inconsistent to use hyphens both to separate words and communicate structure in a class name.

It's harder to read

Camel case is used in dozens of languages without too much problem. HTML class names are usually fairly short too. When you use structured class names like the one proposed above, hyphens are "meaningful" and not used just to separate words.

Scannability

This is completely subjective and "scannability" is far down the priority list when you write short, discrete components.

Hyphens work better in text editors

Again, pretty far down the priority list. But changing from the previous notation to this notation makes it easier to work with hyphens. Each non-hyphenated part of the name easy to select and has its own meaning.

I think the arguments that came up in the MontageJS discussion are good. I didn't make the change for personal preference :). I think their notation is clearer, easier to further extend with a namespace, uses one fewer character type (by getting rid of underscores), and opens the door to better communicate other relationships in the names. So I'm going to try it out for a bit.

@kaelig
Copy link

kaelig commented Mar 3, 2013

Thank you for this thorough answer. I had not read the montagejs discussion.

I have given a try to double underscores and dashes, and it worked well for me, but it looks like other developers at my company really like the montagejs notation better so we'll probably give it a try as well for a new project (where they already started implementing it). Maybe it is not that much of a problem and I'm just being too conservative.

@necolas
Copy link
Contributor Author

necolas commented Mar 3, 2013

Yep, underscores and hyphens worked for me too. But it's kind of normal for the specific notation to evolve as more people work this way :)

@necolas
Copy link
Contributor Author

necolas commented Mar 25, 2013

Done.

@necolas necolas closed this as completed Mar 25, 2013
@oskarrough
Copy link

Is the ComponentName capitalized to avoid conflicts with the namespace only? Trying it out in some projects and the efficient (read: lazy) me would like to avoid the extra capitalized letter for every selector.

mlnmln pushed a commit to mlnmln/suit that referenced this issue Dec 30, 2018
They blur the lines between utility and theming, because of the margin.

Fix suitcssgh-8
mlnmln pushed a commit to mlnmln/suit that referenced this issue Dec 30, 2018
mlnmln pushed a commit to mlnmln/suit that referenced this issue Dec 30, 2018
Disabled whitespace wrapping; useful utility to apply to button
components, or containers that horizontally scroll inline block
elements.

Fix suitcssgh-8
mlnmln pushed a commit to mlnmln/suit that referenced this issue Dec 30, 2018
Rewrite and Change in generator API
Fixes suitcss#4
mlnmln pushed a commit to mlnmln/suit that referenced this issue Dec 30, 2018
Update npmpub v3.x compatibility
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

No branches or pull requests

3 participants