Polaris-react: Pass through `className` and `style` props

Created on 5 Dec 2018  路  6Comments  路  Source: Shopify/polaris-react

Feature request summary

I feel that most components, at a minimum should provide two props for modifying styling: className, and style. This is probably something that has come up before, and if so I would certainly appreciate the rationale on not doing this and maybe a discussion on the merits of making components more extensible.

Rationale

The reason this is useful is because you frequently run into situations where you need to modify the style of a component, and end up wrapping it in an element to achieve this, which is a terrible approach.


馃専 Feature requests that are not yet planned will be closed. We then use the issue鈥檚 :+1: upvotes to track and set priorities. See the contribution guidelines for more information.

All 6 comments

馃憢 Thanks for opening your first issue. A contributor should give feedback soon. If you haven鈥檛 already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack.

Also, I would certainly be willing to create a PR for this, assuming there is interest from collaborators.

@Introvertuous, as with all feature requests, I'm going to close this issue and allow interested folks to express their interest by upvoting with a 馃憤 on the original comment.

For a bit of background, this is a deliberate decision to not accept style or className props in our components. Historically, we have incurred a lot of support debt by allowing internal developers to style Polaris components directly. The consequence was typically lots of style regressions any time we changed something about the components folks were styling. This is less problematic for third party apps, but would still likely occur.

I'd be curious to hear what your specific need is. Can you tell me a bit more about what you're trying to style and how?

@danrosenthal It is not any one particular use case, it is just a very common need to style the root element of a component. Without passing through those props the user is forced to wrap your component and apply styling to that wrapper. In my opinion this makes for a worse user experience than potential breaking of styles on upgrades. I would certainly incur the risk of style breaking over compromising on code.

Agree with @Introvertuous. It is a common need to customize components.

Yeah a lot of libraries allow this so call sites can add layout properties, e.g. position / margin etc. but agree it means there's more risk for Polaris changes to break call sites -- if for example the call site changed the display to block and later Polaris decided to layout its children with display flex instead of floats.

For now, we're just wrapping Polaris components with extra components to handle layout, e.g.

<Select className={styles.select} />
<div className={styles.select}>
   <Select />
</div>
.select {
   margin: 0 0 20px;
}
Was this page helpful?
0 / 5 - 0 ratings