fix: make theme variables and styles extensible #292
Conversation
levithomason
requested review from
dvdzkwsk,
kuzhelov,
miroslavstastny and
mnajdova
as code owners
levithomason
force-pushed
the
fix/theme-styles-typings
branch
from
7dbb692
to
fe64549
Compare
Codecov Report
@@ Coverage Diff @@
## master #292 +/- ##
=======================================
Coverage 89.54% 89.54%
=======================================
Files 62 62
Lines 1177 1177
Branches 175 152 -23
=======================================
Hits 1054 1054
Misses 121 121
Partials 2 2Continue to review full report at Codecov.
|
levithomason
force-pushed
the
fix/theme-styles-typings
branch
from
daa2cb2
to
bac6ea8
Compare
| @@ -60,6 +60,9 @@ export interface ICSSPseudoElementStyle extends ICSSInJSStyle { | |||
| } | |||
|
|
|||
| export interface ICSSInJSStyle extends React.CSSProperties { | |||
| // TODO Questionable: how else would users target their own children? | |||
| [key: string]: any | |||
There was a problem hiding this comment.
Is it wrong to make this [key: string]: CssInJssStyle? I am a bit worried with the any there..
There was a problem hiding this comment.
This is actually quite a bit trickier. Not every string property is an ICSSInJSStyle. Some have limited values, such as speak, so they cannot be typed as ICSSInJSStyle.
Since this is currently blocking and broken, let's continue with a more permissive object (as is) for now. We can always improve typings.
| @@ -163,72 +166,142 @@ export interface IThemePrepared { | |||
| } | |||
|
|
|||
| export interface IThemeComponentStylesInput { | |||
| [key: string]: any | |||
|
|
|||
There was a problem hiding this comment.
As in the previous comment, can we make this: [key: string]: IComponentPartStylesInput? What else can be the value of this?
There was a problem hiding this comment.
Yes, here we can since all keys have the same value type. Same with IThemeComponentStylesPrepared below which I have updated as well.
levithomason
force-pushed
the
fix/theme-styles-typings
branch
from
bac6ea8
to
cda5693
Compare
| Text?: IComponentPartStylesPrepared | ||
| } | ||
|
|
||
| export interface IThemeComponentVariablesInput { | ||
| [key: string]: any |
There was a problem hiding this comment.
I guesss the same can be applied here as well as in the IThemeComponentVariablesPrepared? [key: string]: ComponentVariablesInput & [key: string]: ComponentVariablesPrepared? Otherwise we have some typings errors already..
kuzhelov
added
postponed
Currently, consumers cannot add custom component display names to a theme's
componentVariablesandcomponentStyles. This is a problem as they will need to add their component's to the theme as well.I've also added the unlisted component display names the theme.