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

Question: Should CSS class names be less generic? #29

Open
Peque opened this issue Dec 7, 2021 · 8 comments
Open

Question: Should CSS class names be less generic? #29

Peque opened this issue Dec 7, 2021 · 8 comments

Comments

@Peque
Copy link
Contributor

Peque commented Dec 7, 2021

I am integrating lil-gui inside a Vuetify application. Without altering the CSS in my application, this is the result:

Screenshot from 2021-12-07 02-16-44

It seems Vuetify is overriding the .title class style:

Screenshot from 2021-12-07 02-16-05

Since .title seems like a very generic class name, and in order to avoid other frameworks overriding the style, would it make sense to use more specific class names? Like .lil-gui-title for example.

Low priority, since I can very easily modify the styles in my application, but maybe it would be more user-friendly the other way? 😊

Feel free to close if you think it is not worth it. 👍

@georgealways
Copy link
Owner

I'd actually argue that this is Vuetify being overly aggressive: it's only happening because those declarations have !important. .lil-gui .title would normally be the more specific selector here.

Sure, we could add more characters to the class names to try and avoid collisions, but at the end of the day, the library can't do anything to keep its styles in the face of !important or a super broad selector. There's a lot of 'generic' class names in the stylesheet that we'd have to prefix by this same logic, and I'd rather the class names be intuitive for people to extend.

@Peque
Copy link
Contributor Author

Peque commented Dec 7, 2021

@georgealways Note that font-weight and line-height were also being overwritten, and they do not have the !important flag (it seems the .lil-gui .title was not being the more specific selector?).

@georgealways
Copy link
Owner

You're right, I thought since .lil-gui was closer in the hierarchy, that it would have precedence, but apparently definition order wins over that. https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity#tree_proximity_ignorance

Still feel like it's a bit heavy handed on the framework's part--I imagine that v-application is the body? Ideally GUI wouldn't live in that container.

@Peque
Copy link
Contributor Author

Peque commented Dec 7, 2021

Yeah, v-app is everything. Everything is contained inside it. 😊

@georgealways
Copy link
Owner

Gotcha—I'll have to think about if the library should account for this type of thing. I'll keep it open in case anyone else wants to chime in.

@georgealways
Copy link
Owner

Revisiting this—prefixing only solves some of the issues. Making the selectors less generic (.button => .lg-button) wouldn't do anything in the face of a framework that applies styles to the raw button element.

The most robust solution to these CSS conflicts would be to encapsulate lil-gui in the shadow DOM, but that's a pretty complicated change, and it would prevent people from styling the GUI with anything except the exposed CSS vars.

For example, CSS like this would break because it can't pierce the shadow DOM:

.lil-gui .title { text-decoration: underline; }

Branching so that the shadow DOM is optional adds even more complexity, so I'm at a bit of a stalemate with this issue.

My best attempt at a shadow DOM port is here. Things were still breaking, but I forget what exactly.

@Peque
Copy link
Contributor Author

Peque commented Dec 27, 2022

@georgealways Thanks for sharing the update. I understand the difficulties.

Perhaps using less-generic selectors as a first step can help improve things without having to switch to shadow DOM or branching (I agree that making it optional would be too much complication for lil-gui and using a single approach would be best for maintainability).

Feel free to ignore this issue if it is too much work (or even close it at any time). Or maybe leave it open in case other users want to share their feedback (knowing other use cases could help making the right decision here). 😊

@georgealways
Copy link
Owner

Yes you're right, it doesn't have to be all or nothing. Avoiding class name collisions at the least would still be a good thing.

The other major pitfall to 'namespacing' the classes like this would be breaking user CSS like the line I wrote above:

.lil-gui .title { text-decoration: underline; }

That doesn't work if we change it to .lg-title of course.

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

2 participants