alexanderwallin / react-player-controls

⏯ Dumb and (re)useful React components for media players.
http://alexanderwallin.github.io/react-player-controls/
ISC License
191 stars 35 forks source link

CSS strategy #5

Closed vkammerer closed 8 years ago

vkammerer commented 8 years ago

Hello,

The package does not include any default CSS, is it intentional? I understand that this means that the components are highly customizable, but it makes them practically unusable at first.

What could be an optimal CSS strategy that would:

Would it also be a good idea if some styling depended on props, like "height" and "color"?

alexanderwallin commented 8 years ago

Yes, this has indeed been intentional.

Regarding initial usability – which is poor since you basically don't see anything – I agree we could provide some bootstrap styles. Preferably we could do that in the README, on the demo page or in a css file named to hint about it not being meant for production use.

The only thing I feel would make sense to provide production-ready styles for is <RangeControlOverlay />, which is essential for sliders to work at all.

The tricky part is making all components customisable independently of which styling technique you use, be it CSS, CSS modules or style props. I don't have much experience with CSS modules etc, so some research on best practices would be good. Have you seen any good examples of this?

vkammerer commented 8 years ago

I have personally used css modules (https://github.com/css-modules/css-modules) with webpack for the past 8 months, and they are really nice and easy to use.

I am just throwing out ideas here, but what would you think about:

Regarding the documentation: what about displaying on your demo page, for each code section of each component, a CSS tab which, if selected, would display the default styles? That would make it easy for anyone to understand which properties to override. The README could simply indicate that the CSS structure can be found on the demo page.

vkammerer commented 8 years ago

After thinking again about it and doing some research, I realize that the proposal here above doesn't solve the issues you mention.

The problems with a class based approach to styling (as opposed to a styles based approach) are:

For these reasons (especially the second aspect), an approach to styling via inline styles might be best for your package, don't you think?

Exposing a way to override these styles via a prop could be a very satisfying solution

vkammerer commented 8 years ago

I have done a bit of research on the strategies that some other components use (like https://github.com/callemall/material-ui) and it seems that for now the way to go to distribute a component's style is to use inline styles.

Do you have time to talk about it on skype? my username is vincentkammerer

alexanderwallin commented 8 years ago

Thanks for all the good input!

As you say, providing overridable classNames and style props should be enough to be able to customise the components the way you want. That way this package stays unbiased on how to style your stuff.

For nested components we could expose [componentName]Style props (like playButtonStyle) to begin with.

On top of that, we'll provide some default styles both as CSS files and JavaScript objects, free for any consumer to implement.

What do you recon of that plan?

vkammerer commented 8 years ago

yes that sounds good! For example with the VolumeSlider component, do you plan to implement it this way?

<VolumeSlider
  defaultClassName={true} // Boolean, to add the class name used in the distributed stylesheet
  customClassName="personnalClassName" // String, to add more class names 
  defaultStyle={true} // Boolean, to add the default inline styles, same as those of the default class 
  customStyle={color: 'blue'} // Object, to be merged with the default styles if turned on
/>

If the component has sub tags (like for the VolumeSlider), how do you want to expose custom styling? Should props also enable fine control over these elements? like:

<VolumeSlider
  handleDefaultClassName={true}
  handleCustomClassName="handlePersonnalClassName"
  handleDefaultStyle={true}
  handleCustomStyle={color: 'blue'}
/>
alexanderwallin commented 8 years ago

Parent/root components

I think this can be simplified by simply providing className and style props together with a collection of Styles:

import { VolumeSlider, Styles } from 'react-player-controls'

<VolumeSlider
  className="MySuperVolumeSlider"
  style={{
    ...Styles.VolumeSlider,
    width: 36,
  }}
/>

Components have defaultProps like:

VolumeSlider.defaultProps = {
  className: 'VolumeSlider',
  style: {},
  ...
}
vkammerer commented 8 years ago

Yes that sounds more simple than what I had proposed. So, if I understand correctly:

.VolumeSlider {
  color: green;
}

and developers are free to include the stylesheet in their css stack, or to define this class as they want.

How do you want to generate the stylesheet? Ideally it should depend on where the { Styles } are defined. And do you know how the styling of sub tags could be exposed?

alexanderwallin commented 8 years ago

Precisely!

I have more faith in a JS -> CSS compilation than the other way around, and there's probably plenty of tools out there (such as this one).

Yes, customization of nested components needs to happen, probably through props on the wrapping component.

I'll make a milestone with issues for each task.

alexanderwallin commented 8 years ago

Milestone: https://github.com/reactify/react-player-controls/milestone/1

Feel free to grab an issue!

vkammerer commented 8 years ago

Hey thanks it's looking good! I just started a new job on monday and it's taking all my time so I'll be less available in the future.. (I'd have already sent you a PR otherwise). Maybe I'll have time on the weekend but I cant be sure for now..