styled-components / styled-components

Visual primitives for the component age. Use the best bits of ES6 and CSS to style your apps without stress 💅
https://styled-components.com
MIT License
40.39k stars 2.49k forks source link

Feature Request: Support for class props other than className #3179

Open maclockard opened 4 years ago

maclockard commented 4 years ago

It is not an uncommon pattern for 3rd party component libraries to take multiple different css class props beyond just className. Currently styled-components only supports styling via the className prop requiring hacky workarounds to make use of the other props.

Take for example the Dialog component from the very popular React component library Blueprint. It has both a className prop for the dialog proper and a prop for the surrounding portal component portalClassName. If you are using styled components you are unable to make use of portalClassName, and instead have to rely on global styles which is not preferable.

In the past, a recommended workaround for the multiple class prop issue was to try to do something like the following:

const portalClassName = 'portal';

const MyStyledDialog = styled(Dialog).attrs({
    portalClassName,
})`
  & .${portalClassName} {
    color: blue;
  }
`

This has a critical issue in that it makes an assumption about the DOM structure of Dialog, specifically that the element portalClassName is applied to is a descendant of Dialog, which is not true in this case since the portal actually wraps Dialog. In general, this workaround does not work any component that takes advantage of React's Portal API since there is no telling where in the DOM a portal element will actually render.

Multiple class props is not an uncommon pattern for component libraries and pretty powerful css framework agnostic solution to styling multiple elements that works with vanilla css, emotion, css modules, etc. Adding support for this to styled-components would be helpful to those wishing to adopt styled-components but rely on 3rd party component libraries.

This issue had been previously discussed here: https://github.com/styled-components/styled-components/issues/1273, however, it did not adequately address some of the problems with assumptions of DOM structure.

quantizor commented 4 years ago

We don't get requests for this much, so going to leave it open and see if it reaches critical mass before proceeding.

Not a complicated feature obviously but we try not to add things directly to the library unless they're used often due to bundle size.

I'm sure you have a fix already, but my recommendation would be to make a HOC wrapper that does this prop translation for you.

maclockard commented 4 years ago

What would this HOC wrapper look like? I know what it would look like for just making use of portalClassName but unsure what it would look like for making use of both portalClassName and className on Dialog.

maclockard commented 4 years ago

I understand wanting to wait for critical mass, but one concern I would have is that folks won't really ask for this since the global style hack is 'good enough' for most people. However, the end result is that folks using styled components end up with more brittle styles that rely on global styles compared to if they used emotion or css modules instead.

RinatMullayanov commented 3 years ago

@probablyup I have same problem as @maclockard It will be great if you support this possibility.

teamf7 commented 3 years ago

@probablyup I am facing the same problem

Connor406 commented 3 years ago

@probablyup I've run in to the same issue when using Reactour's API. It would be awesome to have a fix!

n1313 commented 3 years ago

The same issue happens when using styled-components together with Webcomponents which expect class attribute: <my-awesome-button class="big red">Click me!</my-awesome-button>.

kitten commented 3 years ago

Just leaving a note here; I think the original issue suggests a solution, and personally, I don't really see a problem with it. I believe stylis does support :global and if we currently don't we could add it. That does open up other problems though, specifically that encapsulation is important, which is why previously, I believe, we've disabled it. Instead, what's much more desireable is to actually wrap a component like Dialog and pass className on as a different prop, hence enabling this using a small workaround.

However, what's much more important is to support the class prop, which is an issue I've marked as duplicate for this one, since it's the same general concern, and we may want to opt for an explicit API instead of an automatic class "switch".

RobinClowers commented 3 years ago

I'm running into a situation that I don't see discussed here. I've got a component that accepts a wrapperClassName, which gets applied to a wrapper element outside the input that gets the className prop. So I can't use the attrs work around, because my wrapper is not a child of the element that has className. Emotion has a really nice solution to this (https://emotion.sh/docs/class-names) that doesn't require global styles, is there a way to do something similar with Styled Components?

maclockard commented 3 years ago

@RobinClowers that's the same problem I described in the initial issue of the DOM tree not matching the React Component tree. Ideally this will be handled in the future by the more general API change @kitten is talking about.

For now, here is a hacky workaround I came up with for blueprint popover:

Click to expand code snippet ```tsx interface HexPortalPopoverProps extends IPopover2Props { realClassName?: string; } /** * This component is an intermediate component that allows us to use * styled components with a class prop that isn't `className`. * * We bascially just save off the consumer `className` to `realClassName`, applying the * styles added by styled components to `popoverClassName`. */ const HexPortalPopover: React.ComponentType = React.memo( function HexPortalPopover({ className, popoverClassName, realClassName, ...props }) { return ( ); }, ); const StyledHexPortalPopover = styled(HexPortalPopover)` ... styles for the wrapping element. `; export const HexPopover: React.ComponentType = React.memo( function HexPopover({ className, ...props }) { return ; }, ); ```

With some modification, should be able to be applied to your case as well.

ChrisKuBa commented 3 years ago

Another example from #3582 based on react: Use "as" polymorphic prop to rename the HTML tag and get a better overview while using devTools, also in production mode. I end up using boilerplate code to map classname to class. It works but synchronize both needs more code and also the classnames stays twice (class + classname) within the HTML code / tag. It could be easier and less error prone with a configurable attribute target.

class MyComponent extends HTMLElement {
  static get observedAttributes() {
    return ['classname'];
  }

  attributeChangedCallback(name, oldValue, newValue) {
    this.setAttribute('class', newValue);
  }
}

window.customElements.define('x-button-y', MyComponent);

const Component = styled.div`
  color: red;
`;

 <Component as="x-button-y">some text</Component>
maclockard commented 1 year ago

This issue was tagged with 6.0 but it looks like 6.0 was released without it. Curious if there are still any future plans to implement this (or if y'all are open to a PR for this)?

maclockard commented 9 months ago

Would y'all be open to external contribution for this issue? This is very painful to workaround with 3rd party libraries and a major motivator of mine to potentially migrate away from styled component right now

mmatuk commented 9 months ago

I also would like this feature. I have a couple components that use a container element where I can pass containerClassName and would love if I could remap the styled-component to target that prop instead of being the default className.

quantizor commented 9 months ago

You can do this easily via styled(p => <Component yourCustomClassname={p.className} {...p} />) fyi

mmatuk commented 9 months ago

You can do this easily via styled(p => <Component yourCustomClassname={p.className} {...p} />) fyi

While, yes it may be easy. It does add extra levels of complexity since now I need an extra wrapper component around anything that I want to do this too. A flag or some other way to change the target prop would save the extra wrapper component.