[withSwitchLabel] labelClassName vs className (LabelSwitch, LabelRadio, LabelCheckbox)

[Alxandr]

I'm writing an app using the next release of material-ui on npm, and for the most part it's really great. However I ran into one thing that puzzled me greatly today with the LabelSwitch. I wanted to move it up to the upper right corner, so I created a class witch had position: absolute; top: 5; right: 5 and used <LabelSwitch className='topRight' />, however only the switch was moved to the top right. I looked at the source and quickly discovered why (and how to fix it) using labelClassName, but I would argue that the outermost component should probably be styled using className, and that you could rather make a different switchClassName which targets the switch itself. All in all it's not the biggest of problems, I just feel that if you try to move a component you expect the entire component to move, not just a subcomponent.

[rosskevin]

Agreed, we have been discussing perhaps a different approach to complex components (perhaps under classes prop) and refining our use of them.

@nathanmarks @oliviertassinari - thoughts?

related: #6711

[oliviertassinari]

@Alxandr Thanks for trying out and sharing your experience with the next branch.
@rosskevin Sounds like you are experiencing the very same issue.

My thoughts on this. I see two issues.

Predictibility

• This is quite an important API decision to make. Every single person using the library is going to face that issue.
• We have an inconstancy between what's documented: the className always apply on the root element and what's implemented.
• The inconstancy is privileging ease of use over predictability.
• I would rather keep predictability by:
  • applying the className and the other properties on the root element.
  • providing a xxProps property for changing the properties of a nested xx element when that makes sense.

A very specific variation for a one-time problem/situation

We don't provide a theme property like react-toolbox is doing to customize every class names.
Instead, we expose class names, like the inputClassName className with the Input component.

After thinking about it, I think that exposing a classes property object, automatically documented would be better. Users would be able to customize everything at the class name level. That's deeply linked to the overrides key in the theme where users can override everything at the class level, not the instance level.
Actually, we have an issue for that #6137 :).

[Alxandr]

Slightly related: nathanmarks/jss-theme-reactor#42

That's another problematic control I found btw. Using Tabs inside an AppBar required this mess:

const styleSheet = createStyleSheet('RootAppBar', () => ({
  tabs: {
    height: '100%',

    '& > div': {
      height: '100%',

      '& > div': {
        height: '100%',

        '& > div:first-child': {
          height: '100%',
          alignItems: 'center',
        },
      },
    },
  },
}));

I can make another issue about the Tabs inside AppBar if wanted.

[oliviertassinari]

@Alxandr The Tabs inside an AppBar is an interesting issue! I have faced it yesterday. I have been simply applying the height of the AppBar to the Tabs. Not the best solution. The height: 100% trick wasn't working for me on Safari. Noticed that you can now use the classes property to access nested elements.

[oliviertassinari]

Back to this issue, I think that the best other alternative is exposing a SelectionLabel component on user-land instead of using a Higher-order component internally.

[oliviertassinari]

I'm fine with both solutions (the current one and the proposed one). Let me know what you think :).

[oliviertassinari]

Any objection to expose a FormSwitchLabel component instead of a wrapping the base component with an HOC?

[rosskevin]

No objections here

[oliviertassinari]

I'm on it, let's follow rebass API, it's going to be a breaking change through.