search

goatslacker / iso

Deprecated in favor of https://github.com/airbnb/hypernova
MIT License
329 stars 32 forks source link

Issue #9: Added element and classname configuration options #15

Closed michaelBenin closed 9 years ago

michaelBenin commented 9 years ago

Config properties in constructor:

this.markupClassName = config.markupClassName || "___iso-html___";
this.markupElement = config.markupElement || "div";
this.dataClassName = config.dataClassName || "___iso-state___";
this.dataElement = config.dataElement || "div";

Also I saw that on render, we create a new Iso, is this intentional? Also added configuration option to the render call as well.

Examples updated. Manually tested it on date-time. Would like to see more tests, linting, and documentation.

michaelBenin commented 9 years ago

Ah there's an issue with the date-time. Let me dig into this further.

michaelBenin commented 9 years ago

Ok I figured it out, the context of this was broken in the class instance.

Specifically: https://github.com/goatslacker/iso/pull/15/files#diff-338ebbd84df23821b105676cbab7154bR51

I'm just now moving into using ES6/ES2015/Babel.

Fixed it with:

  static bootstrap(onNode, markupClassName='___iso-html___', dataClassName='___iso-state___') {

Is there a better way?

Added those arguements at the end to make it backwards compatible.

Will update the render method as well.

Fix should be in later tonight around 8 or 9 EST.

michaelBenin commented 9 years ago

I'll reopen the pr and squash the commits when it's all cleaned up.

goatslacker commented 9 years ago

I might end up switching this from class to prototypes to reduce code size.

Can you update to latest babel on this PR as well?

michaelBenin commented 9 years ago

The on method should also take the config.

michaelBenin commented 9 years ago

@goatslacker - this should be good to go.

Pending we're good, let me know if I should squash the commits.

goatslacker commented 9 years ago

lgtm