Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Support returning an array of styles to be merged from props callbacks #118

Closed
kentcdodds opened this issue May 19, 2017 · 4 comments · Fixed by #120
Closed

Support returning an array of styles to be merged from props callbacks #118

kentcdodds opened this issue May 19, 2017 · 4 comments · Fixed by #120

Comments

@kentcdodds
Copy link
Contributor

Problem description:

Sometimes I want to do this:

glamorous.div(({big, square}) => {
  const bigStyles = big ? {
    [desktopMediaQuery]: {
      fontSize: 20
    }
  } : {}

  const squareStyles = !square ? {
    [desktopMediaQuery]: {
      borderRadius: '50%',
    }
  } : {}

  return {...bigStyles, ...squareStyles}
})

But that wont work because Object.assign (or object spread in this case) is shallow, so the last desktopMediaQuery object will entirely override the first one. So I have to workaround this limitation myself with more imperative, less expressive code.

Suggested solution:

I think it'd be cool to be able to do this:

  glamorous.div(({big, square}) => {
    const bigStyles = big ? {
      [desktopMediaQuery]: {
        fontSize: 20
      }
    } : {}

    const squareStyles = !square ? {
      [desktopMediaQuery]: {
        borderRadius: '50%',
      }
    } : {}
-   return {...bigStyles, ...squareStyles}
+   return [bigStyles, squareStyles]
  })

And have glamorous forward both of those items to glamor and have glamor merge them as it does everything else so nicely for us.

I'm going to start working on this now... Because I want it now :)

@kentcdodds
Copy link
Contributor Author

Oh snap... This already works. ❤️ glamor! My PR will be to add a test and docs for this then!

@kentcdodds
Copy link
Contributor Author

I just want to thank @threepointone for being so awesome 👏

@ErikCupal
Copy link
Collaborator

Cool! Luckily very easy to change the Typescript typings to support it. I'll a make PR with the typings change and little typescript test 😉.

@threepointone
Copy link

🤗

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants