rusackas commented on pull request #12122:
URL:
https://github.com/apache/incubator-superset/pull/12122#issuecomment-754296259
> @rusackas Those were great observations. For me these specific properties
(margins) are related to positioning of the component in a screen so they are
not generic but layout based. With that in mind, Alert already have properties
for this scenario: `style` and `className`. What `styled` is actually doing is
passing a `className` for the Alert component but in a non direct manner so I
thought of replacing that with the `css` property.
>
> ```
> <Alert
> css={theme => ({
> marginTop: theme.gridUnit * 4,
> marginBottom: theme.gridUnit * 4,
> })}
> ...
> />
> ```
>
> This makes more clear the we are positioning the element and have the
benefit of autocompletion and type checking.
>
> You were totally wright about the mismatching grid units. I changed all to
4.
>
> Let me know if you agree with that 😉
If these are one-off, situational tweaks, then I prefer the way you have it
here, using the `css` prop. Somehow that looks more like a contextual tweak,
whereas `styled` makes it look (to me, at least) more like we're creating a new
variant of the component.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
