[GitHub] [superset] michael-s-molina commented on issue #15264: [Proposal][style-guide]Tips and best practices for Emotion `styled`/`css` implementation

[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