Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2 class/className attribute API #754

Closed
jorgebucaran opened this issue Sep 1, 2018 · 15 comments
Closed

V2 class/className attribute API #754

jorgebucaran opened this issue Sep 1, 2018 · 15 comments
Labels
discussion enhancement New feature or request
Milestone

Comments

@jorgebucaran
Copy link
Owner

jorgebucaran commented Sep 1, 2018

Summary

I find myself using classcat quite often when I want to add or remove a class to/from a DOM element. The idea of conditionally adding classes in a declarative style suits the nature of the VDOM well.

With V2 I want to introduce the ability to toggle element classes on and off through the class attribute. This means that in addition to a string, you will be able to use an object or array for this attribute.

The behavior will be adapted from classcat specifically for Hyperapp. Classcat will continue to exist as an independent package which can be used with other frameworks or as a standalone library.

Example

class as a string

import { h } from "hyperapp"

export const HoverButton = state => (
  <button
    class={`btn ${state.isPressed ? "btnPressed" : ""} ${
      !state.isPressed && state.isHovered ? "btnOver" : ""
    }`}
  >
    {state.label}
  </button>
)

class as an object

import { h } from "hyperapp"

export const HoverButton = state => (
  <button
    class={{
      btn: true,
      btnPressed: state.isPressed,
      btnOver: !state.isPressed && state.isHovered
    }}
  >
    {state.label}
  </button>
)

class as an array

import { h } from "hyperapp"

export const HoverButton = state => (
  <button
    class={[
      "btn",
      {
        btnPressed: state.isPressed,
        btnOver: !state.isPressed && state.isHovered
      }
    ]}
  >
    {state.label}
  </button>
)

Prior Art


👋 Do you think you'll find this useful? Do you have any ideas for improvements? What else would you like to see?

@jorgebucaran jorgebucaran added enhancement New feature or request discussion labels Sep 1, 2018
@SkaterDad
Copy link
Contributor

I'd find this very handy to have built in.

Most of my CSS lately is with utility libs like Tailwind, and manually building up the class combinations is tedious. I should probably use classcat, but sometimes laziness wins!

I assume regular strings will still be useable also?

@jorgebucaran
Copy link
Owner Author

jorgebucaran commented Sep 1, 2018

I assume regular strings will still be useable also?

Yup. You can use a string | array | object. See above, I added more examples.

@tetradice
Copy link
Contributor

Great! This is exactly the function to be built-in 👏

@okwolf
Copy link
Contributor

okwolf commented Sep 1, 2018

I misread the title to mean JavaScript classes and was briefly like: 🤯

@jorgebucaran jorgebucaran changed the title V2 class API V2 class/className attribute API Sep 2, 2018
@jonaskuske
Copy link

Oh, I love this feature in Vue!
Might be reason enough to upgrade my hyperapp v1.x site to V2 after all 🎉

@frenzzy
Copy link
Contributor

frenzzy commented Sep 2, 2018

Since classes concatenation feature is not always necessary and useless for css-in-js approaches which usually calculate a single class name in runtime, I am worried about performance impact.

Browser's DOM contains classList style data structure besides className. Could we also consider to add special classList prop which will work with arrays?

What about composable components, will you need to use something like classcat anyway to make/use third-party reusable components?

@jorgebucaran
Copy link
Owner Author

jorgebucaran commented Sep 2, 2018

@frenzzy Adding a classList attribute is a reasonable request. Should we also make the style attribute accept only a string and introduce a new styleObject? If avoidable, I'd rather not. Using class as an array/object, in addition to a string, is a powerful feature as we can derive from classcat / classnames monthly downloads.

Note: Because this change only affects the class attribute, you'd be able to use className for applying a single class name. But you shouldn't rely on this implementation detail.

Let's measure that

We don't want to optimize prematurely, so let's ask the real question here, what/which is faster?

https://jsperf.com/classlist-vs-classname-test

screen shot 2018-09-03 at 8 11 23

screen shot 2018-09-03 at 8 19 45

The results indicate that className vs V2 className are virtually the same and that classList is the slowest.

@jorgebucaran
Copy link
Owner Author

What about composable components, will you need to use something like classcat anyway to make/use third-party reusable components?

Can you show us an example? I am not sure I understand what you mean.

@jonaskuske
Copy link

jonaskuske commented Sep 3, 2018

@jorgebucaran

Hmm, so if I write a component that always adds its own classes but is also supposed to accept classes via props, I'd have to do

const Component = props => {
  const { class: classProp } = props;
  let className;

  if (Array.isArray(classProp)) {
    className = [...classProp, 'predefined classes'];
  } else {
  className = [classProp, 'predefined classes'];
  }

  return (
    <somemarkup class={className} />
  );
};

Or will hyperapp always flatten the Array so you can wrap the received class prop in an Array and add your styles to it no matter its type?

Edit: You can of course make it a little more concise with a ternary and shorter names, like

const Component = ({ class: cls }) => {
  const className = [Array.isArray(cls) ? ...cls : cls, 'predefined classes'];
  
  // and so on
};

but I still don't think it's the optimal solution – unless I'm missing something here :)

@jorgebucaran
Copy link
Owner Author

jorgebucaran commented Sep 3, 2018

@jonaskuske 👍Arrays will be recursively flattened.

<div class={[[["foo", ["bar", ["baz", [{ bam: true }]]]]]]}>Hello</div> 

//=> <div class="foo bar baz">Hello</div>

So, your Component can be written as follows.

const Component = props => {
  return (
    <somemarkup class={[props.class, "predefined", "classes"]} />
  );
};

@frenzzy
Copy link
Contributor

frenzzy commented Sep 3, 2018

Almost no effect on performance and super easy classes extension..

const Button = (props) => <button {...props} class={['material-ui-btn', props.class]} />

💯 😍

@TheNando
Copy link

Do these get rendered inline or do they compile to a stylesheet with generated classes?

@SkaterDad
Copy link
Contributor

@TheNando This topic is just about the class list, not inline styles.

Inline styles get attached to the DOM node properties.

@jorgebucaran
Copy link
Owner Author

@TheNando ...compile to a stylesheet with generated classes

Nope, just added to the className property.

@jorgebucaran jorgebucaran added this to the V2 milestone Oct 3, 2018
@frenzzy
Copy link
Contributor

frenzzy commented Nov 1, 2018

Repository owner locked as resolved and limited conversation to collaborators Nov 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants