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

StatementsHash has side-effect of sorting rule.Statements #6

Open
facefunk opened this issue Sep 17, 2019 · 9 comments
Open

StatementsHash has side-effect of sorting rule.Statements #6

facefunk opened this issue Sep 17, 2019 · 9 comments
Labels

Comments

@facefunk
Copy link

sort.Slice(rule.Statements, func(i, j int) bool {

Is this desirable? Obviously this behaviour does, in many cases, preclude utilising statement precedence within a single rule but given that the operation we are performing when generating these hashes is combining duplicate rules, an operation that despite my personal fondness is inherently lossy, could we actually be observing a Scarlet convention here? Is Scarlet designed for a strict implementation of a type of OOCSS whose best practices obviate the need for statement precedence? I must admit to having a certain attraction to this idea as well as scepticism about its practicality. Whilst there have been many attempts in the last decade to develop a CSS methodology that minimises the C in CSS, I'm not aware of any that have eliminated it completely and while I can envision such a system, I would expect its style sheets to grow in size with the complex hierarchy of classes that would become necessary.

@akyoto
Copy link
Member

akyoto commented Sep 18, 2019

This is intended behavior, by design.

The idiomatic way to write scarlet styles is to use CSS selector priorities and never depend on statement order.

@facefunk
Copy link
Author

facefunk commented Sep 18, 2019

I love this. It removes access to a load of nasty old hacks but I'm glad to see the back of those and they're mostly only needed for <=IE6 anyway.

How would you approach the case where you wish to have a CSS class that overrides some previous statement in a more general class?

.foo
  margin 10px

.bar
  margin 5px

.baz
  margin 10px

produces:

.foo, .baz {
	margin: 10px;
}

.bar {
	margin: 5px;
}

Which gives a 5px margin to:

<div class="bar baz"></div>

Instead of the expected 10px margin.

Obviously this is a massively simplified example but I have many cases in production code where this would cause a problem.

@akyoto
Copy link
Member

akyoto commented Sep 19, 2019

The order of classes mentioned in the HTML code matters.

baz will always have a higher priority than bar if they are referenced as class="bar baz".

In every browser I tested thus far, the last class always takes the highest priority.

Any other behavior is either a browser bug or I might be misinformed (apologize if that's the case!).

@facefunk
Copy link
Author

Unfortunately not. It's always been the case that the order of classes specified in an HTML element's class attribute is irrelevant, this order is not specified as relevant in the W3C spec. Old versions of IE that did some janky stuff with compound class selectors like .class1.class2 are the only exceptions to this rule that I am aware of. The classes are applied in the order they are specified in the style sheet.

https://jsfiddle.net/hc9au2sm/

No apologies necessary, browser tech is full of gotchas like this, simultaneously the love of my career and the bane of my existence!

In my branch I'm moving combineDuplicates out to a separate operation with added warnings for the time being. I'm sad because I think this is a really neat feature. I've wracked my brain to get it to work. Scanning templates for used tags is one thing but determining the resulting hierarchy would require some impractical brute force testing, in my application at least. The only answer I can think of would be some sort of strict OOCSS style methodology but with specified class inheritance either through some added metadata or some class name microformat like BEM.

@akyoto
Copy link
Member

akyoto commented Sep 25, 2019

I'll have to think about how to fix this, this is quite the mistake I made.

Thank you for bringing this up.

@akyoto akyoto added the bug label Nov 2, 2019
@akyoto
Copy link
Member

akyoto commented Nov 8, 2019

I believe the reason why I haven't encountered this problem in production is because I am using single-class definitions a lot. If I need to mix classes, I use mixins.

Instead of having 2 classes generic and derived, I create the mixin generic and use it within derived. The HTML element only uses derived.

At the very least it counts as a workaround.

@facefunk
Copy link
Author

facefunk commented Dec 2, 2019

Thanks, yes. I used a technique similar to this in Stylus and on one of my biggest projects, it worked well in terms of my ability to comprehend the code. However I was able to reduce the overall download size by using a multi-class pattern in some places. Many methodologies and articles, including this one: https://mrmrs.cc/writing/scalable-css/ suggest the multi-class pattern.

@akyoto
Copy link
Member

akyoto commented Dec 2, 2019

Yep, I hear you. I haven't dismissed this problem yet.

To be honest, I'm currently too busy with other projects to give this issue the attention it needs, but maybe the stars will align at some point.

@facefunk
Copy link
Author

facefunk commented Dec 2, 2019

That's cool man, I'm in the same boat. I'm just mulling this over every so often.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants