Style module not handling 'hooked' properties

[SkaterDad]

I tried this out on a project today, and found the style.js function is not handling snabbdom's hooked properties (delayed, destroy, remove, ...).

For example, this is a style object I put on several vNodes which causes them to fade in & out.

style: {
  opacity: 0,
  transition: 'opacity 0.5s ease-in-out',
  delayed: {
    opacity: 1,
  },
  remove: {
    opacity: 0,
  },
  destroy: {
    opacity: 0,
  },
}

The style function put this string into the HTML:

style="opacity: 0; transition: opacity 0.5s ease-in-out; remove: [object Object]; destroy: [object Object]"

My preference would be to merge the "initial" style properties with the delayed properties, and return that. On clients with javascript disabled, this would allow the nodes to render in their 'post-animation' state, and the site would be usable.

I played around a bit and came to this solution. It doesn't seem very elegant, but perhaps you'lll have ideas on how to clean it up? If you're only targeting Node, perhaps some more ES6 syntax could remove some of the lodash dependencies?

var forOwn = require('lodash.forown')
var escape = require('lodash.escape')
var kebabCase = require('lodash.kebabcase')
var omitBy = require('lodash.omitby')
var merge = require('lodash.merge')

// data.style

module.exports = function style (vnode) {
  if (!vnode.data.style) {
    return ''
  }
  //Style properties on vnode init
  var initial = omitBy(vnode.data.style, function(value) {
    return typeof value === 'object'
  })
  //Style properties in the 'delayed' hook
  var delayed = vnode.data.style.delayed

  //Merge the initial and delayed styles.
  var mergedStyles = merge(initial, delayed)

  //Push merged styles into a string array
  var styles = []

  forOwn(mergedStyles, function (value, key) {
    styles.push(`${kebabCase(key)}: ${escape(value)}`)
  })

  //Return HTML tag attribute string
  return styles.length ? `style="${styles.join('; ')}"` : ''
}

The return value for my example style:

style="opacity: 1; transition: opacity 0.5s ease-in-out"

[acstll]

@SkaterDad thanks for opening this. That's true, hooks are not handled at all…

Your solution looks good, are you using it already replacing the "built-in" style module?

[SkaterDad]

@acstll Yes, I tested out the snippet above by overwriting the code in npm_modules/snabbdom-to-html/modules/style.js, and npm installing the 2 new lodash dependencies.

It seemed to work well for my needs.

Can you think of any use cases where it would not be preferred to merge the 'initial' and 'delayed' styles? If so, this could be done with an option boolean or something.

[acstll]

Can you think of any use cases where it would not be preferred to merge the 'initial' and 'delayed' styles?

That's a tricky question. In your case it's clear if you want to have a usable page for people with JS off. But I can imagine a situation where you just want to leave it as-is.

If so, this could be done with an option boolean or something.

you mean passing a boolean or even an options object as a second param to the toHTML function?

Also, about overriding the module. You didn't need to overwrite the source files, you could do it like this, setting up your own toHTML function, à la snabbdom:

var init = require('snabbdom-to-html/init')
var toHTML = init([
  require('snabbdom-to-html/modules/attributes'),
  require('./your-own-modules/styles')
])

Check this section of the REAME out. :-)

[TylorS]

@acstll He doesn't have access to the toHTML from cycle-snabbdom :worried:

I need to fix that

[acstll]

@TylorS mmm, so regarding #6, would it be useful (or enough) if a transpiled-to-ES5-build was just the main toHTML function with the built-in modules? (no "advanced" mode)?

Transpiling while retaining the advanced flexibility isn't that straight-forward, I'm realizing now. In such case, getting rid of the few ES6 stuff would be easier, although it'd feel like a step backwards.

[TylorS]

@acstll I'm not sure I follow how you'd loose the 'advanced' functionality. You wouldn't have to browserify it or anything like that, just babel src/ -d lib/ assuming you moved the source code to src/. That's all up to you. But that would just be a simple es6 -> es5 build. Then in the package.json you could change index.js -> lib/index.js

[acstll]

Of course, let me try that. (I was thinking about using browserify and individual modules not being part of any bundle… :-))

[acstll]

@TylorS works perfectly, thanks.

Would you commit this new build folder or have it .gitignored and only present in the npm package? (I never did this before!)

[TylorS]

I usually add lib/ to my .gitignore and add src/ to my .npmignore

[acstll]

@SkaterDad I added a fix for this in the latest 2.1.0. I finally went with merging in the delayed object (after reading again the snabbdom docs). We can review this later if it turns out a bad decision.

Thanks again!

[SkaterDad]

Thanks for the new release! :+1:

Webpack is complaining when i try to build, saying it can't find the babel object assign transform plugin. I think if you publish the 'lib' directory of es5'd code, it should prevent issues like that.

I can get around this for now by installing the babel object assign transform plugin in my local project.