You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/06/18 21:08:19 UTC
[GitHub] [superset] rusackas opened a new issue #15264: [WIP][proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
rusackas opened a new issue #15264:
URL: https://github.com/apache/superset/issues/15264
The following is a proposal to provide guidance on how best to use Emotion, some of the tips/tricks/patterns that exist in the repo, and the pros/cons of using `styled` vs `css` approaches, with example use cases. This proposal, after a little clean-up here and feedback from this thread, will be posted to the `dev@` email list for lazy consensus before being migrated to the Superset Wiki.
***********************
# Emotion Tips and Strategies
The first thing to consider when adding styles to an element is how much you think a style might be reusable in other areas of Superset. Always err on the side of reusability here. Nobody wants to chase styling inconsistencies, or try to debug little endless overrides scattered around the codebase. The more we can consolidate, the less will have to be figured out by those who follow. Reduce, reuse, recycle.
## When to use `css` or `Styled`
In short, either works for just about any use case! And you’ll see them used somewhat interchangeably in the existing codebase. But we need a way to weigh it when we encounter the choice, so here’s one way to think about it:
A good use of `styled` syntax if you want to re-use a styled component. In other words, if you wanted to export flavors of a component for use, like so:
```js
const StatusThing = styled.div`
padding: 10px;
border-radius: 10px;
`;
export const InfoThing = styled(StatusThing)`
background: blue;
&::before {
content: "ℹ️";
}
`;
export const WarningThing = styled(StatusThing)`
background: orange;
&::before {
content: "⚠️";
}
`;
export const TerribleThing = styled(StatusThing)`
background: red;
&::before {
content: "🔥";
}
`;
```
You can also use `styled` when you’re building a bigger component, and just want to have some custom bits for internal use in your JSX. For example:
```js
const SeparatorOnlyUsedInThisComponent = styled.hr`
height: 12px;
border: 0;
box-shadow: inset 0 12px 12px -12px rgba(0, 0, 0, 0.5);
`
function SuperComplicatedComponent(props) {
return (
<>
Daily standup for {user.name}!
<SeparatorOnlyUsedInThisComponent />
<h2>Yesterday:</h2>
// spit out a list of accomplisments
<SeparatorOnlyUsedInThisComponent />
<h2>Today:</h2>
// spit out a list of plans
<SeparatorOnlyUsedInThisComponent />
<h2>Tomorrow:</h2>
// spit out a list of goals
<>
);
}
```
### The `css` prop, in reality, shares all the same styling capabilities as `styled` but it does have some particular use cases that jump out as sensible. For example if you just want to style one element in your component, you could add the styles inline like so:
```js
function SomeFanciness(props) {
return (
<>
Here's an awesome report card for {user.name}!
<div
css={css`
box-shadow: 5px 5px 10px #ccc;
border-radius: 10px;
`
}>
<h2>Yesterday:</h2>
// ...some stuff
<h2>Today:</h2>
// ...some stuff
<h2>Tomorrow:</h2>
// ...some stuff
</div>
<>
);
}
```
You can also define the styles as a variable, external to your JSX. This is handy if the styles get long and you just want it out of the way. This is also handy if you want to apply the same styles to disparate element types, kind of like you might use a CSS class on varied elements. Here’s a trumped up example:
```js
function FakeGlobalNav(props) {
const menuItemStyles = css`
display: block;
border-bottom: 1px solid cadetblue;
font-family: "Comic Sans", cursive;
`;
return (
<Nav>
<a css={menuItemStyles} href="#">One link</a>
<Link css={menuItemStyles} to={url} >Another link</Link>
<div css={menuItemStyles} onclick={alert('this is not a great example`)} >Another link</Link>
</Nav>
);
}
```
## `css` tips and tricks
Using the `theme`
### `css` lets you write actual CSS
By default the `css`prop uses the object syntax with JS style definitions, like so:
```js
<div css={{
borderRadius: 10;
marginTop: 10;
backgroundColor: `#00FF00`
}}>Howdy</div>
```
But you can use the `css` interpolator as well to get away from icky JS styling syntax. Doesn’t this look cleaner?
```js
<div css={css`
border-radius: 10px;
margin-top: 10px;
background-color: `#00FF00`;
`}>Howdy</div>
```
You might say “whatever… I can read and write JS syntax just fine.” Well, that’s great. But… let’s say you’re migrating in some of our legacy LESS styles… now it’s copy/paste! Or if you want to migrate to or from `styled` syntax… also copy/paste!
### You can combine `css` definitions with array syntax
You can use multiple groupings of styles with the `css` interpolator, and combine/override them in array syntax, like so:
```js
function AnotherSillyExampe(props) {
const shadowedCard = css`
box-shadow: 2px 2px 4px #999;
padding: 4px;
`;
const infoCard = css`
background-color: #33f;
border-radius: 4px;
`;
const overrideInfoCard = css`
background-color: #f33;
`;
return (
<div className="App">
Combining two classes:
<div css={[shadowedCard, infoCard]}>Hello</div>
Combining again, but now with overrides:
<div css={[shadowedCard, infoCard, overrideInfoCard]}>Hello</div>
</div>
);
```
### Style variations with props
You can give any component a custom prop, and reference that prop in your component styles, effectively using the prop to turn on a “flavor” of that component
For example, let’s make a styled component that acts as a card. Of course, this could be done with any AntD component, or any component at all. But we’ll do this with a humble `div` to illustrate the point
```js
const SuperCard = styled.div`
${({ theme, cutout }) => `
padding: ${theme.gridUnit * 2}px;
border-radius: ${theme.borderRadius}px;
box-shadow: 10px 5px 10px #ccc ${cutout && `inset`};
border: 1px solid ${cutout ? transparent : theme.colors.secondary.light3 };
`
`
```
Then just use the component as `<SuperCard>Some content</SuperCard>` or with the (potentially dynamic) prop: `<SuperCard cutout>Some content</SuperCard>`
## Styled component tips
### No need to use `theme` the hard way:
It’s very tempting (and commonly done) to use the `theme` prop inline in the template literal like so:
```js
const SomeStyledThing=styled.div`
padding: ${({ theme }) => theme.gridUnit * 2}px;
border-radius: ${({ theme }) => theme.borderRadius}px;
border: 1px solid ${({ theme }) => theme.colors.secondary.light3};
`
```
Instead, you can make things a little easier to read/type by writing it like so:
```js
const SomeStyledThing=styled.div`
${({ theme }) => `
padding: ${theme.gridUnit * 2}px;
border-radius: ${theme.borderRadius}px;
border: 1px solid ${theme.colors.secondary.light3};
`
`
```
## Extend an AntD component with custom styling
As mentioned, you want to keep your styling as close to the root of your component system as possible, to minimize repetitive styling/overrides, and err on the side of reusability. In some cases, that means you’ll want to globally tweak one of our core components to match our design system. In Superset, that’s Ant Design (AntD).
AntD uses a cool trick called compound components. For example, the `Menu` component also lets you use `Menu.Item`, `Menu.SubMenu`, `Menu.ItemGroup`, and `Menu.Divider`
### The `Obeject.assign` trick
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #15264:
URL: https://github.com/apache/superset/issues/15264#issuecomment-872304244
This example needs a closing bracket:
```
const SomeStyledThing=styled.div`
${({ theme }) => `
padding: ${theme.gridUnit * 2}px;
border-radius: ${theme.borderRadius}px;
border: 1px solid ${theme.colors.secondary.light3};
`} <-- closing bracket
`;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #15264:
URL: https://github.com/apache/superset/issues/15264#issuecomment-871717018
Great guide @rusackas! Thanks for the hard work.
I would like to add some points extracted from [CSS prop vs styled](https://benmcmahen.com/why-i-choose-emotion/) to help with the discussion.
> I generally prefer the css style api for the following reasons:
>
> **You’re still writing regular React components.**
> Especially when working with typescript, I consider this beneficial. You type a Button component just as you would a regular React component, allowing you to clearly specify which props are accepted. As a result, you’re less likely to pollute your dom with odd attributes—a problem I found common when passing custom attributes to styled components.
>
> **Object styles are easier to work with.**
> When working with typescript, I love that all of my css is typechecked and provides robust autocompletion. And I generally find it easier to insert theme variables into objects instead of using the ${theme.color.red} style notation. The small downside to objects is that they are slightly more cumbersome to write and aren’t easily copied from browsers.
>
> **You colocate styles with elements.**
> With the css prop, what you see is what you get. It’s a small point, but not having to scroll away from an element to find your style definition really improves my workflow. It feels more efficient and keeps me in the flow when writing my components. Need to delete an element? There’s no need to hunt down the orphaned style definition.
For me, one thing that is really important is the typescript checking, and this can be achieved either by using `styled` or the `css` property because both accept a CSS-in-JS object. I also found that handling conditional styling is way easier when using the CSS-in-JS approach.
> you’re less likely to pollute your dom with odd attributes
This really happens in our codebase. We have a lot of warnings of properties that are being passed to styled components without the `shouldForwardProp`. By the way, this is only required because the `styled` mixes the component's properties with properties used to conditionally style.
We also can achieve reusability with the `css` prop.
```
const StatusThing = props => (
<div
css={{ padding: 10, borderRadius: 10 }}
{...props}
/>
);
const InfoThing = props => (
<StatusThing
css={{ background: 'blue', '&::before': { content: 'i' } }}
{...props}
/>
);
```
```
const SeparatorOnlyUsedInThisComponent = props => (
<hr
css={{
height: 12,
border: 0,
boxShadow: 'inset 0 12px 12px -12px rgba(0, 0, 0, 0.5)',
}}
{...props}
/>
);
```
Since we're using CSS-in-JSS all style props are being checked by typescript.
For me, the difference is bigger when we have conditional styling:
```
const StyledSelect = styled(AntdSelect, {
shouldForwardProp: prop => prop !== 'hasHeader',
})<{ hasHeader: boolean }>`
${({ theme, hasHeader }) => `
width: 100%;
margin-top: ${hasHeader ? theme.gridUnit : 0}px;
`}
`;
```
```
<Select css={{ width: '100%', marginTop: hasHeader ? theme.gridUnit : 0 }} />
```
Preferences aside, the real gain is defining the standard, so thanks again for this awesome guide, it will be great content for our wiki 👏🏼 🚀
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas edited a comment on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
rusackas edited a comment on issue #15264:
URL: https://github.com/apache/superset/issues/15264#issuecomment-878402092
Thoughts on some "rules" to make this more of a style guide. Please feel free to suggest additional ones or contest anything controversial here.
**DO** use `styled` when you want to include additional (nested) class selectors in your styles
**DO** use `styled` components when you intend to export a styled component for re-use elsewhere
**DO** use `css` when you want to amend/merge sets of styles compositionally
**DO** use `css` when you're making a small, or single-use set of styles for a component
**DO** move your style definitions for the `css` prop to an external variable when they get long
**DO** prefer tagged template literals (`css={css...`) over style objects wherever possible for maximum style portability/consistency (note: typescript support may be diminished, but IDE plugins like [this](https://marketplace.visualstudio.com/items?itemName=jpoissonnier.vscode-styled-components) make life easy)
**DO NOT** use `styled` for small, single-use style tweaks that would be easier to read/review if they were inline
**DO NOT** export incomplete AntD components (make sure all their compound components are exported)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina removed a comment on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
michael-s-molina removed a comment on issue #15264:
URL: https://github.com/apache/superset/issues/15264#issuecomment-872304244
This example needs a closing bracket:
```
const SomeStyledThing=styled.div`
${({ theme }) => `
padding: ${theme.gridUnit * 2}px;
border-radius: ${theme.borderRadius}px;
border: 1px solid ${theme.colors.secondary.light3};
`} <-- closing bracket
`;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #15264:
URL: https://github.com/apache/superset/issues/15264#issuecomment-878402092
Thoughts on some "rules" to make this more of a style guide. Please feel free to suggest additional ones or contest anything controversial here.
**DO** use `styled` when you want to include additional (nested) class selectors in your styles
**DO** use `styled` components when you intend to export a styled component for re-use elsewhere
**DO** use `css` when you want to amend/merge sets of styles compositionally
**DO** use `css` when you're making a small, or single-use set of styles for a component
**DO** move your style definitions for the `css` prop to an external variable when they get long
**DO** prefer tagged template literals (`css={css...`) over style objects wherever possible for maximum style portability/consistency (note: typescript support may be diminished, but IDE plugins like [this](https://marketplace.visualstudio.com/items?itemName=jpoissonnier.vscode-styled-components) make life easy)
**DO NOT** use `styled` for small, single-use style tweaks that would be easier to read/review if they were inline
**DO NOT** export impartial AntD components (make sure all their compound components are exported)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina edited a comment on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on issue #15264:
URL: https://github.com/apache/superset/issues/15264#issuecomment-871717018
Great guide @rusackas! Thanks for the hard work.
I would like to add some points extracted from [CSS prop vs styled](https://benmcmahen.com/why-i-choose-emotion/) to help with the discussion.
> I generally prefer the css style api for the following reasons:
>
> **You’re still writing regular React components.**
> Especially when working with typescript, I consider this beneficial. You type a Button component just as you would a regular React component, allowing you to clearly specify which props are accepted. As a result, you’re less likely to pollute your dom with odd attributes—a problem I found common when passing custom attributes to styled components.
>
> **Object styles are easier to work with.**
> When working with typescript, I love that all of my css is typechecked and provides robust autocompletion. And I generally find it easier to insert theme variables into objects instead of using the ${theme.color.red} style notation. The small downside to objects is that they are slightly more cumbersome to write and aren’t easily copied from browsers.
>
> **You colocate styles with elements.**
> With the css prop, what you see is what you get. It’s a small point, but not having to scroll away from an element to find your style definition really improves my workflow. It feels more efficient and keeps me in the flow when writing my components. Need to delete an element? There’s no need to hunt down the orphaned style definition.
For me, one thing that is really important is the typescript checking, and this can be achieved either by using `styled` or the `css` property because both accept a CSS-in-JS object. I also found that handling conditional styling is way easier when using the CSS-in-JS approach.
> you’re less likely to pollute your dom with odd attributes
This really happens in our codebase. We have a lot of warnings of properties that are being passed to styled components without the `shouldForwardProp`. By the way, this is only required because the `styled` mixes the component's properties with properties used to conditionally style.
We also can achieve reusability with the `css` prop.
```
const StatusThing = props => (
<div
css={{ padding: 10, borderRadius: 10 }}
{...props}
/>
);
const InfoThing = props => (
<StatusThing
css={{ background: 'blue', '&::before': { content: 'i' } }}
{...props}
/>
);
```
```
const SeparatorOnlyUsedInThisComponent = props => (
<hr
css={{
height: 12,
border: 0,
boxShadow: 'inset 0 12px 12px -12px rgba(0, 0, 0, 0.5)',
}}
{...props}
/>
);
```
Since we're using CSS-in-JS all style props are being checked by typescript and we get autocompletion while coding.
For me, the difference is bigger when we have conditional styling:
```
const StyledSelect = styled(AntdSelect, {
shouldForwardProp: prop => prop !== 'hasHeader',
})<{ hasHeader: boolean }>`
${({ theme, hasHeader }) => `
width: 100%;
margin-top: ${hasHeader ? theme.gridUnit : 0}px;
`}
`;
```
```
<Select css={{ width: '100%', marginTop: hasHeader ? theme.gridUnit : 0 }} />
```
Preferences aside, the real gain is defining the standard, so thanks again for this awesome guide, it will be great content for our wiki 👏🏼 🚀
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas closed issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
rusackas closed issue #15264:
URL: https://github.com/apache/superset/issues/15264
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #15264:
URL: https://github.com/apache/superset/issues/15264#issuecomment-916493307
Added to wiki [here](https://github.com/apache/superset/wiki/Emotion-Styling-Guidelines-and-Best-Practices)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #15264:
URL: https://github.com/apache/superset/issues/15264#issuecomment-866127123
I think this doc needs some "DOs" and "DON'Ts" before we can call it a "coding guideline" - any thoughts on the matter, anyone?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org