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

Review typing of CSS values #48

Closed
peduarte opened this issue Jul 8, 2020 · 3 comments · Fixed by #206
Closed

Review typing of CSS values #48

peduarte opened this issue Jul 8, 2020 · 3 comments · Fixed by #206

Comments

@peduarte
Copy link
Contributor

peduarte commented Jul 8, 2020

For example, gap can't be a number.

It may be worth referring to this https://github.com/system-ui/theme-ui/blob/f5dd5b3ee601909a52ceb14cbd5fdd1857364ef1/packages/css/src/types.ts#L14-L22

@peduarte
Copy link
Contributor Author

At the moment, numbers are not supported in most properties. This is because initially we didn't want to allow people to set a unitless number, and have it being converted to px value.

It seems that we've changed our minds about this, @colmtuite can you confirm, that now we do want to support unitless numbers? (with the exception of line-height)

@hadihallak maybe its worth doing this together when you implement the functionality?

@peduarte
Copy link
Contributor Author

Since we dont rely on css-types our implementation would probably be different

I've got it to work by changing the types to this:

export type GapProperty<TLength> =
  | Globals
  | TLength
  | "normal"
  | ((string | number) & {});

But will hold off until we can decide what to do

@peduarte
Copy link
Contributor Author

peduarte commented Sep 16, 2020

Done in #206

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 a pull request may close this issue.

1 participant