Open chasejarms opened 3 years ago
You can totally use arrays like this:
<img
src={props.src}
alt={props.alt}
css={[
!imageMetadata && emotionCSS.imgClass,
!!imageMetadata && emotionCSS.customImageClass
]}
/>
So there is no need for a dedicated helper for composition (nor in Emotion, not in the userland).
@Andarist what about for the emotion class name strings where we can't pass through an array? Seems like it would be easier to compose those with a helper function otherwise you end up needing to do something like this:
<img
src={props.src}
alt={props.alt}
className={[
!imageMetadata && emotionCSS.imgClass,
!!imageMetadata && emotionCSS.customImageClass
].filter((class) => !!class).join(" ")}
/>
@Andarist I'd argue the array syntax would become a lil' bit messy when working with complex conditions. It'd be very nice to have something like cx
for @emotion/react
to be able to do something like:
cx(classes.step, {
[classes.activeStep]: index + 1 === activeStep,
[classes.activeStepMobile]: index + 1 === activeStep,
[classes.prevStep]: index + 1 < activeStep,
})
Whereas the current implementation would have to be:
[
classes.step,
(index + 1 === activeStep) && classes.activeStep ,
(index + 1 === activeStep) && classes.activeStepMobile,
(index + 1 < activeStep) && classes.prevStep,
]
@ali-garajian I don't see any practical difference here. In fact - the latter reads better to me because you have the condition first so it reads like "if this is condition is met then this css should be applied" (reading from the left to the right).
The problem
For a few of my local projects, I've added in a utility function that makes emotion conditional composition a little cleaner. It's nice at the application level but it really shines in component libraries where we're conditionally applying user defined styles based on the internal state of the component.
Proposed solution
Here's the code I'm using locally for SerializedStyles.
Then in the actual places it's used it would look something like this:
Whereas it would typically look like this:
Pretty small difference but the first solution seems a little cleaner to me. If you decide this is something that would add value to the codebase, I can open up a PR for this scenario as well as the string class name scenario.
Alternative solutions
Alternative would be to just keep it as is:
Additional context