Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow theme.space to be array *or* object #417

Merged
merged 1 commit into from Mar 11, 2019

Conversation

jaridmargolin
Copy link
Contributor

This functionality appears to have been removed from v4 (hopefully not intentionally). Currently all other theme props support being defined as an array or an object. Spacing properties are the exception (or the only exception I have run into).

@jaridmargolin
Copy link
Contributor Author

I've also hit a few occasions where it would be helpful to declare negative margins/paddings. I was considering adding support for this by checking if object key startsWith -. but was concerned this would be unexpected behavior w/ objects. Any thoughts?

@aholachek
Copy link

Merging this pr would be helpful for my use case — designers have passed down a series of named spacing increments to integrate into our design system, and I'd prefer to stay close to their naming scheme rather than introducing a new mental model (array indices) just for spacing.

@jxnblk
Copy link
Member

jxnblk commented Mar 11, 2019

Thanks! Yeah, this was an unintentional regression so thanks for catching this!

@jxnblk jxnblk merged commit 573956d into styled-system:master Mar 11, 2019
@jaridmargolin
Copy link
Contributor Author

@jxnblk - Thank you for the quick merge/release. Do you have any opinions on if object definitions should support negative values? Ex: mx='-small'

I would have to sniff for - as the first character, which feels a little hacky. At the same time, it's annoying to NOT have access to negative theme values.

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

Successfully merging this pull request may close these issues.

None yet

3 participants