markup: Multiple Class or ClassMap usage now appends additional classes #155
Conversation
Codecov Report
@@ Coverage Diff @@
## master #155 +/- ##
==========================================
+ Coverage 42.47% 46.38% +3.91%
==========================================
Files 4 4
Lines 598 595 -3
==========================================
+ Hits 254 276 +22
+ Misses 312 287 -25
Partials 32 32
Continue to review full report at Codecov.
|
dom.go
Outdated
| // Classes | ||
| classList := h.node.Get("classList") | ||
| for name := range h.classes { | ||
| classList.Call("add", name) |
There was a problem hiding this comment.
It may be good to assume the DOM API is slow than a map check, i.e. before calling add check prev:
if _, ok := prev.classes[name]; !ok {
classList.Call("add", name)
}
But this is just a hunch / best practice in my mind. It's probably plenty fast so I am OK with this as-is if you don't wish to update.
There was a problem hiding this comment.
It's probably plenty fast, but I'm fine with the change as I haven't benchmarked.
markup.go
Outdated
| @@ -142,19 +142,36 @@ func Data(key, value string) Applyer { | |||
| }) | |||
| } | |||
|
|
|||
| // Class returns an Appyer which applies the provided classes. Subsequent | |||
| @@ -229,33 +246,3 @@ func Namespace(uri string) Applyer { | |||
| h.namespace = uri | |||
| }) | |||
| } | |||
|
|
|||
| // join is extracted from the stdlib `strings` package | |||
| func join(a []string, sep string) string { | |||
|
Looks great to me. A few inline comments, though. I'll leave the decision of #155 (comment) up to you and we'll go with whatever you decide :) Could you add a note to https://github.com/gopherjs/vecty/blob/master/doc/CHANGELOG.md ? |
This enables component libraries and composition of elements with classes. BREAKING: The function `prop.Class(string)` has been removed and replaced with `vecty.Class(...string)`. Migrating users must use the new function and split their classes into separate strings, rather than a single space-separated string.
|
Added tests that were missing for classes, too. |
This change updates Go Package Store to use the new Vecty API, which had breaking changes in hexops/vecty#155 and hexops/vecty#158. The two main changes include: - prop.Class(string) has been replaced with vecty.Class(...string). - vecty.Component.Render signature has changed from Render() *vecty.HTML to Render() vecty.ComponentOrHTML. Reference: https://github.com/gopherjs/vecty/blob/19a11ec3acc3ae6c2e095b1ace2b7134c5f28c10/doc/CHANGELOG.md#nov-4-2017-pr-158-major-breaking-change. Fixes #90.
This enables component libraries and composition of elements with
classes.
BREAKING: The function
prop.Class(string)has been removed andreplaced with
vecty.Class(...string). Migrating users must use thenew function and split their classes into separate strings, rather than
a single space-separated string.
Fixes #80