MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
13.99k stars 926 forks source link

Classhelper into core #1967

Closed StephanHoyer closed 7 years ago

StephanHoyer commented 7 years ago

I'm heavily using a small helper to generate classnames.

const classy = def => Object.keys(def).filter(class => def[class]).join(' ')

I think it would be great to have that built in.

snabdom has support for this too.

Expected Behavior

m('div', { class: {
  isActive,
  someRandomClassname: someRandomCheck(),
  [someStringVar]: true
}, 'Hello world'})

should be equivalent to

m('div', { class: compact([
  isActive ? 'isActive' : null,
  someRandomCheck() ? 'someRandomClassname' : null,
  someStringVar
]).join(' ')}, 'Hello world'})

Current Behavior

Does not work.

Possible Solution

add this to class handling hyperscript.js#L54-60. Would love to create a PR if wanted.

dead-claudia commented 7 years ago

-1 from me. Try https://github.com/JedWatson/classnames, if you don't want to roll your own.

On Wed, Sep 13, 2017, 05:09 Stephan Hoyer notifications@github.com wrote:

I heavily using a small helper to generate classnames.

const classy = def => Object.keys(def).filter(class => def[class]).join(' ')

I think it would be great to have that built in.

snabdom https://github.com/snabbdom/snabbdom#the-class-module has support for this too. Expected Behavior

m('div', { class: { isActive, someRandomClassname: someRandomCheck(),

}, 'Hello world'})

should be equivalent to

m('div', { class: compact([ isActive ? 'isActive' : null, someRandomCheck() ? 'someRandomClassname' : null, someStringVar ]).join(' ')}, 'Hello world'})

Current Behavior

Does not work. Possible Solution

add this to class handling hyperscript.js#L54-60

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MithrilJS/mithril.js/issues/1967, or mute the thread https://github.com/notifications/unsubscribe-auth/AERrBC8wsMddw2ELBzMr_JjLsKDAtDu0ks5sh5vfgaJpZM4PVzUN .

cavemansspa commented 7 years ago

this seems like a practical addition to me considering that generating conditional class name lists are commonplace in a mithril app.

maybe just add as m.classy as a convenience function?

there was further discussion related to this on gitter regarding @porsager bss lib. was that also a consideration for moving under mithril community?

tivac commented 7 years ago

Utilities like these shouldn't be in mithril.

classnames is a perfect example of a small, targeted, optimized lib that handles this use-case excellently.

tivac commented 7 years ago

I just saw classwrap which seems promising as well.

StephanHoyer commented 7 years ago

I know all those libs and I still find all of them way to much. The proposed one is much simpler and a allows everything that those libs allow but in a uniform simple way. No flattening, no mix and match of arrays and objects. Just one object. And sure, I can roll my own, thats what I already do.

But since this is a pretty obvious use case and has only a small performance impact (one string/object check) I thought it would be worth adding.

StephanHoyer commented 7 years ago

maybe one could use a property classes for this.

m('div', {
  classes: { ... }
}, 'hello')
maranomynet commented 7 years ago

Adding a magic classes prop would bring a fourth way to declare classNames on elements.

m('div.class1', {
        'class': 'class2',
        className: 'class3',
        classes: { class4: true },
    },
    'Class War!'
);

Regardless of that, IMO this feature should be handled outside of mithril core.

StephanHoyer commented 7 years ago

just created https://github.com/StephanHoyer/classies