These are chat archives for assetgraph/assetgraph

10th
Jun 2017
Andreas Lind
@papandreou
Jun 10 2017 09:06
@Munter sounds like you need the inverse of https://www.npmjs.com/package/postcss-merge-longhand
Peter Müller
@Munter
Jun 10 2017 09:10
Yeah. I think this one might do what I need. I'm essentially messing with the postcss ast anyway, so I might not need a full plugin to run https://www.npmjs.com/package/parse-css-font
This lib is using it. The example shows shorthand followed by longhand override
Currently trying to figure out if the behavior I have implemented is actually correct for duplicate properties in the first place. If so, I might be able to piggyback on that
Peter Müller
@Munter
Jun 10 2017 09:18
The current behavior for duplicate properties is to return an array of these object I make with selector, specificity, prop, value, important. So the values are both in there. Given my correct implementation of evaluating order and importance the end result is correct. There might be room for an optimization where I only return a single entry with the actual value, so I don't fill up the arrays of selectors by property
Peter Müller
@Munter
Jun 10 2017 09:25
This might be better https://www.npmjs.com/package/css-shorthand-expand
Retains the CSS property casing
Peter Müller
@Munter
Jun 10 2017 09:33
It's also generic. For if we want to extend this to non-fonts one day
And of course there's already a transform. Looks like I need to do very little work \o/ https://www.npmjs.com/package/postcss-shorthand-expand
Andreas Lind
@papandreou
Jun 10 2017 09:45
Hmm, doesn't the transform have to reset all the other properties that aren't included in the shorthand?
Peter Müller
@Munter
Jun 10 2017 09:46
If they do it right they'll inject declaration nodes at the location of the shorthand. So later decalration nodes can override
I'll have to test that of course
Andreas Lind
@papandreou
Jun 10 2017 09:47
In the first example of the readme it should also reset background-position when expanding, I mean.
Which would mean 0% 0% I guess.
Peter Müller
@Munter
Jun 10 2017 09:50
If nothing is defined in a shorthand it just implies default values. No need to set those
Andreas Lind
@papandreou
Jun 10 2017 10:28

What I mean is that if you have the following stylesheet:

.foo {
    font-family: foo;
    font-style: italic;
}
.bar {
    font: 12px bar;
}

And this markup:

<div class="foo bar">hello</div>

And you expand the font property in the .bar rule without also including font-style: normal even though it's not explicitly mentioned, you'll alter the semantics. In this case the font subsetting will end up thinking that "hello" is to be rendered in italics, I think?

Peter Müller
@Munter
Jun 10 2017 10:31
Bram might want to know if this is the case. Turns out that part of the library is his code, copied. Having a Twitter back and forth with him now
Andreas Lind
@papandreou
Jun 10 2017 10:31
Should be pretty easy to test in a browser
He probably thought of the defaulting as out of scope for his lib
Peter Müller
@Munter
Jun 10 2017 10:33
From intuition I would say your example should actually be rendered as italic. I'll test in a few minutes
Andreas Lind
@papandreou
Jun 10 2017 10:33
Cool :)
Those shorthand versions of the properties were probably a spec mistake
Peter Müller
@Munter
Jun 10 2017 10:34
Writing tooling that has to take them into account I tend to agree :P
Andreas Lind
@papandreou
Jun 10 2017 10:35
:)
Peter Müller
@Munter
Jun 10 2017 11:00
Tested and verified. The shorthand should expand to also define properties that are not mentioned
Andreas Lind
@papandreou
Jun 10 2017 11:00
Okay
Maybe Bram could be convinced to accept a PR for that as an alternative mode? Or we could just _.defaults it on the result.
Peter Müller
@Munter
Jun 10 2017 11:01
What defaults though. CSS initials?
Andreas Lind
@papandreou
Jun 10 2017 11:01
Yeah
Peter Müller
@Munter
Jun 10 2017 11:03
I feel that there's an npm module missing that contains all the CSS initials from the spec
Andreas Lind
@papandreou
Jun 10 2017 11:03
In this case, I think it's something like { 'font-style': 'normal', 'font-weight': 'normal', 'font-variant': 'normal' } would be enough.
Since font-family is mandatory for the shorthand to even be valid.
And line-height and font-size are "don't cares" from the perspective of the subsetting code.
Peter Müller
@Munter
Jun 10 2017 11:06
Yeah, I can easily fill the props out I want
Andreas Lind
@papandreou
Jun 10 2017 11:06
Ah, and font-size is also required for the shorthand to be valid.
Peter Müller
@Munter
Jun 10 2017 11:11
I guess I could write a scraper script with assetgraph to go from https://www.w3.org/TR/CSS/#properties to a full list of property initial values. Then make an npm module of it
Peter Müller
@Munter
Jun 10 2017 12:44
@papandreou I think I got the font shorthand implemented: assetgraph/assetgraph@ab4e149
Unless you can come up with missing test cases
Just added another test to cover the case of an invalid shorthand value
Andreas Lind
@papandreou
Jun 10 2017 12:49
Great, I was about to ask about that :)
Andreas Lind
@papandreou
Jun 10 2017 12:56
Seems like there are some glitches wrt. the ordering of properties inside a rule.

If I change this test:

         it('register the longhand value from a valid shorthand', function () {
-            var result = getRules(['font-family', 'font-size'], 'h1 { font: 15px serif; }');
+            var result = getRules(['font-family', 'font-size'], 'h1 { font-size: 16px; font: 15px serif; }');

... I would expect the same result, but then 16px is inferred.

Similarly, the font-size: 16px should take precedence if it comes after the font shorthand, but that does seem to work, although that might be a coincidence
I verified in a browser that the ordering is significant.
Peter Müller
@Munter
Jun 10 2017 19:15
Hmm. I have a test a bit further down that verifies correct order with values both before an after a shorthand. I guess we need a bit more detail
Andreas Lind
@papandreou
Jun 10 2017 19:34
:+1:
Peter Müller
@Munter
Jun 10 2017 20:27
What you should be seeing is two entries in the font size array. One with 16px, the next with 15px.
I'm not reducing the results
But since the last entry of a selector of equal specificity wins, it works out
Andreas Lind
@papandreou
Jun 10 2017 21:01
Hmm, why should it lead to two entries?
It's just the last one that should win?
Andreas Lind
@papandreou
Jun 10 2017 21:10
It seemed like an easy fix. Can't you just iterate through the properties of the style rule in order and update an object with font-family, font-weight, and font-style keys?