dparker2 / vue-emotion

Use emotion with Vue.js
MIT License
6 stars 1 forks source link

Overwriting classes instead of merging. #9

Closed Aaron-Pool closed 5 years ago

Aaron-Pool commented 5 years ago

Hey!

Finally got around to playing around with this library, but some issues came up when I tried it as a drop in replacement for legacy vue-emotion, the biggest of which is that you're not taking into account existing classes that might exist on a VNode.

Checkout this line from wrapper.js:

    // Add the target class if need be
    const classes = this.$_targetClass
        ? [emotionClass, this.$_targetClass]
        : [emotionClass]

    return createElement(
        this.$_styledFrom,  // Base component even if styled multiple times
        { ...context.data, class: classes },
        context.children
    );

See? You're taking into account a targetClass for selector targeting, but when the root node of the component being styled already has a class attribute, it's getting wiped out.

Aaron-Pool commented 5 years ago

Oh, and I fixed it locally, if you're interested in a PR. But if you want to handle it yourself, I get that too 👌

dparker2 commented 5 years ago

@Aaron-Pool Hey sorry I've been super busy and just finally noticed this. Thanks for addressing that!

dparker2 commented 5 years ago

should be in v0.0.5 now! thanks!