galacemiguel / fluid-system

Fluid System is a style props function transformer for generating fluid styles. 💦
MIT License
145 stars 3 forks source link

Error when applying non-single property values #8

Closed hazem3500 closed 4 years ago

hazem3500 commented 4 years ago

when applying a non-single property value it gives the error TypeError: Cannot read property '2' of null

non-single property values are things like "20px 20px"

codesandbox: https://codesandbox.io/s/fancy-snow-wjtsr?fontsize=14&hidenavigation=1&theme=dark

galacemiguel commented 4 years ago

Thank you for bringing this edge case to my attention!

In your example, the expected output is for Fluid System to do nothing with those styles (padding="20px 20px"), correct? Given that they weren't designed to be responsive and therefore couldn't be made fluid?

Was your intention with https://github.com/galacemiguel/fluid-system/pull/5 to address just that case?

Because I realize that this opens up the wider issue of if we should try to generate fluid styles from something like: padding={["10px 20px", "20px 30px"]}. 🤔

galacemiguel commented 4 years ago

Friendly ping! 😄 @hazem3500

galacemiguel commented 4 years ago

Because I realize that this opens up the wider issue of if we should try to generate fluid styles from something like: padding={["10px 20px", "20px 30px"]}. 🤔

I've decided that I won't try to have the library handle this (for now at least).

galacemiguel commented 4 years ago

Fixed by https://github.com/galacemiguel/fluid-system/pull/5. Thanks, @hazem3500!

hazem3500 commented 4 years ago

@galacemiguel sorry for being late.

I agree that we should not let the library handle fluid styles when the styling is a non-single value to avoid complexity. The user can split the shorthand prop if they want fluid styles

// this
padding={["10px 20px", "20px 30px"]}
// to
paddingY={['10px', '20px']}
paddingX={['20px', '30px']}