Closed ckirkendall closed 10 years ago
I'm really happy that you've been able to make sense of the code base. The only issue I've with this patch is that style may behave weirdly when you have a semicolon inside a string.
Le samedi 18 janvier 2014, Creighton Kirkendall notifications@github.com a écrit :
I added some tests to make it a little easier to validate any modifications I was making. I also added a style transform to make it easy to set style attributes on a node. It follows the same structure as you class transform with a custom selector turning the style attributes into a map.
(style :attr1 :path1 :att2 :path2)
You can merge this Pull Request by running
git pull https://github.com/ckirkendall/enliven master
Or view, comment on, or merge it at:
https://github.com/cgrand/enliven/pull/1 Commit Summary
- removed core_test.clj it is out dated and core.clj no longer exists
- add tests for base html transforms
- added style transform
- added test for style transform
File Changes
- M src/enliven/html.cljhttps://github.com/cgrand/enliven/pull/1/files#diff-0(28)
- D test/enliven/core_test.cljhttps://github.com/cgrand/enliven/pull/1/files#diff-1(28)
- A test/enliven/html_test.cljhttps://github.com/cgrand/enliven/pull/1/files#diff-2(38)
Patch Links:
— Reply to this email directly or view it on GitHubhttps://github.com/cgrand/enliven/pull/1 .
On Clojure http://clj-me.cgrand.net/ Clojure Programming http://clojurebook.com Training, Consulting & Contracting http://lambdanext.eu/
I corrected the issue. I added a test to validate the fix also. I think I also found a bug in jsoup. If you quote with single quotes and the internal string uses double quotes, the internal string can't contain a single quote.
jsoup produces invalid structure for this.
style='content:url(welcome), \"testing; ' \" url(logo);'
About phloc-css (that I now use instead of batik to parse selectors): what i'd like to see is a style
segment that returns a map of properties to vectors of values.
However, CSS syntax is not very regular; there are some usages of the slash and comma (see http://www.w3.org/TR/CSS2/propidx.html).
So maybe a map if strings is enough. However i'm not sur to now how to correctly update styles: a given property may appear several times and the merging is non obvious -- especially for shorthand properties.
In truth there's a simple answer: just append the new styles and let the browser sorts the mess out :-)
I can't see how a property -> vector map would work given the variance in syntax. It seems representing it as a {:property "string"} makes more sense. I am not sure I understand why its difficult to update the styles. I don't think we should try to tease out the individual properties of the short hand representations.
I vote for the append model where we represent the styles as an ordered map that appends new keys at the end or updates existing keys in place. This is closer to how people use css.
If we choose tease out properties from the short hand representations we would have to build a sub parser for each shorthand and represent the underlying structure as base properties only. In this model merging is clear because order of precedence implies replace.
I went and read the spec (2.1 and the current draft) and it appears that I had a wrong model of how conflicting properties were processed. If you have the same shorthand twice and specifying separate sub-properties, the first shorthand is ignored, they are not merged. It means that we can safely keep only the last value in the map returned by style
.
In your proposal (append new properties, update in place existing ones) I'd like to understand the consequences of the update in place. If we have a style="border: 1px solid black; border-bottom: 20px"
what's the least surprising result after setting border to "10px solid red"
: style="border: 10px solid red; border-bottom: 20px"
or style="border-bottom: 20px; border: 10px solid red"
?
Plus "append always (and remove the original)" seems to be the behaviour to expect when setting .style.property on an element in the browser.
What are the arguments in favor of in-place when present?
BTW no need for an ordered map: the segment could restore the order.
I think you are right with append always. We need to match what is expected result is from style.property and appending is does this.
I am closing this pull request. I will open another one with an implementation based on phloc-css.
I added some tests to make it a little easier to validate any modifications I was making. I also added a style transform to make it easy to set style attributes on a node. It follows the same structure as you class transform with a custom selector turning the style attributes into a map.