Decisiv / styled-components-modifiers

A library to enable BEM flavored modifiers (and responsive modifiers) in styled components.
MIT License
297 stars 11 forks source link

Enhance `modifiers` prop #21

Closed andrewtpoe closed 5 years ago

andrewtpoe commented 5 years ago

PROPOSED BEHAVIOR

Currently there are two types of modifiers props:

This proposal is to enhance the modifiers prop to also support accepting the object who's keys must match with a size prop. The behavior of modifiers will be unchanged if the value is a string or an array of strings.

The styleModifierPropTypes utility will also require an enhancement to support the new functionality.

Although removing the responsiveModifiers prop may make sense in the future, this proposal does not include that work.

tinynumbers commented 5 years ago

My thoughts:

The purpose of this change is to allow the modifiers prop to accept any of the following types of values:

Example:

<Button
  modifiers={{ 
    small: 'success', 
    large: ['highlighted', 'expanded'],
  }}
  size={getTheSizeFromSomewhere()}
>
  ...
</Button>

In this example, if the size is provided the value "small", the success modifier will be applied to the button. If the size is provided the value "large", the highlighted and expanded modifiers will be applied to the button instead.

QUESTIONS:

Maybe something like this:

<Button
  modifiers={{
    _: 'hidden',
    small: 'success',
    large: ['highlighted', 'expanded'],
  }}
  size={getTheSizeFromSomewhere()}
>
  ...
</Button>

...using the underscore to specify the default (since that's an unlikely size value; however some end-user might want to use e.g. "default" as a valid value for size).

andrewtpoe commented 5 years ago

I do like the idea of a "default" key in the responsive modifiers, especially with this added functionality. Previously, the modifiers value was applied, then any responsive modifiers were applied after.

tinynumbers commented 5 years ago

That raises another question, then: in which one of the following cases should the "default" modifiers be applied?

  1. always, and before any other responsive modifiers are subsequently applied
  2. only if no value is provided to the size prop (i.e. the size prop is "falsey")
  3. only if no matching value is provided to the size prop (either the size prop is not specified, or it is specified but the value matches none of the keys in the modifiers prop)
andrewtpoe commented 5 years ago

I think it makes the most sense to apply the default modifiers only if the size prop doesn't match a different value. That prevents any type of "multi-select" behavior that I think could cause more issues.

Basically what I have in mind:

  if (prop.size) {
    return modifiers[prop.size] || modifiers[DEFAULT_KEY]
  }
  return modifiers

If a user wants to apply defaults before any size option, I'd suggest something like this:

const baseModifiers = ['array', 'of', 'modifiers'];

// ...
  <MyComponent 
    modifiers={{
      small: [...baseModifiers, 'other', 'modifiers']
      // ...
    }}
  />