-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[theme] Fix types to pass array for spacing #20486
[theme] Fix types to pass array for spacing #20486
Conversation
Details of bundle changes.Comparing: 89f6208...dd609f7 Details of page changes
|
@@ -15,6 +15,6 @@ export interface Spacing { | |||
): string; | |||
} | |||
|
|||
export type SpacingOptions = number | ((factor: number) => string | number); | |||
export type SpacingOptions = number | ((factor: number) => string | number) | number[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand the implementation that this is just forwarded to @material-ui/system
, right? Seems like we should type createUnarySpacing
at some point and use that type in /core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do this change here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel strongly about it. Up to anybody who's interested in either method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I suggest one step at the time in this case :). @denys-pavlenko Feel free to send a follow-up. Maybe we should start by breaking down the index.d.ts to be closer to the source
https://github.com/mui-org/material-ui/blob/master/packages/material-ui-system/src/index.d.ts
?
@denys-pavlenko Thanks for the follow-up |
This PR complements #19900