reactackle icon indicating copy to clipboard operation
reactackle copied to clipboard

Add ability to override styles not only via themes

Open RIP21 opened this issue 8 years ago • 5 comments

Hi guys. Here is the complain with a bit of tips that may help :) From what I can see now, those components styles are impossible to override using Button.extend styled-components API. It's fine for something sophisticated like Checkboxes and other stuff, but for things that can be easily extend this way (Buttons, Inputs, Forms etc), I think it's very important. Also styled(Button)won't work too, since you don't pass down className to the core component (in this case Button itself). For things like styling of nested elements you can use something like labelStyles props where you can inject compiled CSS chunk via css function from styled-components. And then import them at the bottom of the Label this way.

export const Label =styled.label`
  ${props => props.css}
`;

So you can override its properties without any className hooks, like labelClass prop so you pass it to label and then inject it like

const NewForm = styled(Form).attrs({
  labelClass: "overriden-form-label"
})`
  & .overriden-form-label {
    font-size: 20px;
  }
`

This flexibility is VERY important :) Hope my suggestions will help with that.

RIP21 avatar Aug 03 '17 19:08 RIP21

Thanks for your suggestion, it's definitly a good one. 👍

I also think, maybe we can implement general extend api on each component, which make it easy to style inner nested elements, something like:

const extendStyles = ({ input, label, errorMessage }) => ({
  input: input.extend`
    color: blue;
  `,
  label: label.extend`
    color: red;
  `,
  errorMessage: errorMessage.extend`
    font-size: 10px;
  `
})

<Checkbox extend={extendStyles} />

@RIP21 What do you think about it? cc: @netmenya, @dmitriy-bizyaev

nickmaltsev avatar Aug 04 '17 05:08 nickmaltsev

Looks nice too. But it also looks like it locks up to styled-components. I mean, what is good about classNames is that if you make it smart enough you can then use almost any CSS - in - JS solution or just CSS. So, this is cool, but as an alternative some hooks to classNames will be nice too. Something like that :)

RIP21 avatar Aug 04 '17 06:08 RIP21

@RIP21 what about this?

const extendStyles = ({ input, label, errorMessage }) => ({
  input: input.extend`
    color: props => props.colorScheme.color
  `,
  label: label.addClassName('additionalClassName'),
  errorMessage: errorMessage.css(compiledCss),
})

<Checkbox extend={extendStyles} />

nickmaltsev avatar Aug 04 '17 07:08 nickmaltsev

And in addition to extend prop which is merge provided styles, classes with underlying, we can add override prop, to completely override styles of some elements

nickmaltsev avatar Aug 04 '17 07:08 nickmaltsev

@nickmaltsev in extend you already about to override stuff :) So I think the override is a bit redundant.

And small things to not create issues for them:

Remove ToggleButtonon hover animation :D It looks cheap and weird :D

Change API for nested stuff like in Material-UI and Semantic-UI, so it will be Card.Area, Card.Something. It's nice, cause you can destructure it and got all of them nice and short.

I'm not a developer here (dreaming to have more time for OSS dev, not only issues 😞 ), but few tips here too:

Few of them biased maybe, but maybe you would like them too :)

Move styled-components to styles.jsnot to a nested folder, and all of them in one file. It will drive me nuts if for 5-10 loc of styles I would need to open another tab.

__tests__ in the root, is annoying too. Co-located tests are nicer too. You see test file or folder (if you have a lot of them it makes sense then) in the folder of component means it tested. Don't need to go back and find out whether is covered or not.

Use Prettier over Standard. It formats not only JS, but template-literals with CSS too. So it's choice number 1 for styled-components users. Also, it just better supported and developing very very very fast. And has the plugin for eslint too, so you can fix all stuff with eslint --fix. If you don't like semi-colons etc. You can override those too. (Hint, perfect eslint presets instead is "react-app", "prettier", "prettier/react")

'use strict' makes no sense. create-react-app lint preset (which is amazing if you want to catch bad practices and causing bugs issues, and then to add some presets like prettier preset, and few others for your taste to catch formatting etc (JSX-no-bind!)), and prettier removes it, cause it's not needed and it's strict by default (is it ES6, or Webpack module system feature, not sure) http://eslint.org/docs/rules/strict

RIP21 avatar Aug 04 '17 10:08 RIP21