Allow to add css classes to any components

[donnguyen]

Issue summary

Can't add my own css classes to Polaris's components.

Expected behavior

Should allow to add my own css classes to Polaris's components when using, for example:
<Card className='my-own-class' /> should include my-own-class also, not just Polaris's classes like the way it is doing now.

Thanks

[lemonmade]

Hi @donnguyen. We won't be adding the ability to set any custom class names/ styles on any of the Polaris components. It goes against how we think about components by making the markup part of the component's API, and means that we are much more likely to break things in consuming apps because we don't know what random variations people have made to our components. It also greatly complicates the UI, as we'd probably need to expose the ability to add class names to almost any node in the tree output by a component.

If there are specific changes/ variations you'd like to see to particular components, please feel free to leave issues here. We're happy to consider different options (or, even, different default behaviour), but we want to provide a clear, version-able API for triggering them.

[dhmacs]

Hi @lemonmade, you may have heard of Styled Components, a really nice way for styling react components which is rapidly becoming popular (they also had a talk at React Conf 2017). I do understand the reasoning behind not allowing className to be set, but unfortunately this prevent us from using really useful libraries like this, as they make use of className in order to work.. It would be nice to have this flexibility in Polaris to be able to opt in and allow using libraries like this.

[lemonmade]

Hi @Macs91, can you explain to me why our current approach breaks the ability to use this with styled components? Presumably, you can continue to render these normally, they just can't be extended using the mechanisms that library provides. Again, the restrictions against extension are by design: this is not meant to be a general-purpose library, so most of the visual styling should not be changed, and we want to be explicit about the variations we endorse by making these full-fledged variations (by adding dedicated props).

[dhmacs]

@lemonmade the explanation is on the above link, specifically the doc says

The styled method works perfectly on all of your own or any third-party components as well, as long as they're accepting the className prop.

I understand that Polaris is meant to create a unique experience across all apps and the platform itself, and like you say it makes explicit what is allowed and what's not.
But I'm not trying to extend Polaris components to change their visual appearance, rather I'm trying to reuse them to build other useful components that Polaris currently does not have (e.g. Toast).

For the most part, I try to develop every interface using exclusively Polaris components, but there are scenarios where this is not possible.

[lemonmade]

Can you give me an example of where this is breaking for you? I am having trouble visualizing why this shouldn’t be able to be used alongside a tool like styled components (it sounds like we are on the same page about not extending the visual appearance, so I’d like to understand your problem a bit better in order to help).

[dhmacs]

Yes sure, I've created a Toast component that works like this.
I need this component to display errors (even multiple errors) when they arise, so that the user gets a feedback when one or multiple operations have gone wrong. The errors might not be strictly related to the page the user is looking to, as they might refer to background operations that the user has started before.
To implement the UI for the toast, I wanted to reuse the Banner component (the critical type). To display it like a toast, I need to add a box shadow to it, and here the problem arises since style components uses the class props to extend the component.

I found a work-around anyway by wrapping Banner on a div, but still it's a work-around and makes you adding extra not necessary markup. Even though I can't think of an example right now, in my experience of working with css probably there are cases where introducing a wrapper does not solve the problem, since certain styles are tightly related to the markup.

[lemonmade]

Yeah, I see where you are coming from. Our decision has been to avoid this kind of extensibility, and instead make the path forward be requesting additional API for existing components/ entirely new components to address emerging/ better patterns. We want to avoid (internally and externally) having components be used in a context they weren't designed for (like a floating banner), as it can be confusing for merchants or break some important design assumptions.

From a code perspective, it would also not be possible to give you the hooks you actually need; on a banner, we have many different nested elements, so do we give you a prop for custom classnames on each one? Or only a "root" class name, which would mean people would have to use descendant selectors, locking in our current HTML structure? These are the kinds of issues we want to avoid.

If you have an interesting use case for this kind of thing (that isn't address by a regular banner or by the flash message method for embedded apps), we'd be happy to discuss ideas for new components/ additions, but we feel pretty strongly that the approach I've outlined above is the correct one given the nature of our design system.

[ptcampbell]

This decision strikes me as pretty weird, honestly. If you're using styled-components like I am, I should be able to make simple CSS changes such as…

const StyledButtonGroup = styled(ButtonGroup)`
    justify-content: flex-end;
`

But they don't work because… you want to avoid this type of extensibility?

What approach do you recommend to style Polaris components?

[lemonmade]

@ptcampbell in the case of layou modifications, there is almost always an alternative that gives us what we want (not making all of the CSS awe use be part of the public API), and you what you want. In a case like you have described above, it should be fairly trivial to accomplish the same layout with a wrapping element that does the work of pushing the entire button group to the right.

[ptcampbell]

It’s fair enough. Thanks. ☺️

…

On Sun, Aug 19, 2018 at 6:33 AM Chris Sauvé ***@***.***> wrote: @ptcampbell <https://github.com/ptcampbell> in the case of layou modifications, there is almost always an alternative that gives us what we want (not making all of the CSS awe use be part of the public API), and you what you want. In a case like you have described above, it should be fairly trivial to accomplish the same layout with a wrapping element that does the work of pushing the entire button group to the right. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#94 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1gMv9y3rQK47xFJ5kHrx6jBTClLAzgks5uSWk5gaJpZM4N675-> .

[ctrlaltdylan]

For others finding themselves frustrated with basic styling changes like reducing margin on the Polaris Icon component, my solution is to just wrap those components with styled divs:

const SideBySide = styled.div`
   display: flex;
   alignItems: center;

   /** identify the corresponding CSS classes that you'd like to change in the child Polaris component **/
   /** then apply the rule you'd like with the appropriate selector **/
   .Polaris-Icon {
      margin: 0;
   }
`

<SideBySide>
   <Icon source={CircleCancelMajorMonotone} />
   &nbsp;
   No
</SideBySide>

It's a hack but it helps with sizing/positioning on the fly: