feat(theme-default): add css variables for transition #325
Conversation
| --t-duration: 0.3s; | ||
| --t-duration-slide: 0.3s; | ||
| --t-all: all var(--t-duration); | ||
| --t-opacity: opacity var(--t-duration); | ||
| --t-color: color var(--t-duration); | ||
| --t-border-color: border-color var(--t-duration); | ||
| --t-background-color: background-color var(--t-duration); | ||
| --t-color-all: var(--t-color), var(--t-border-color), | ||
| var(--t-background-color); | ||
| --t-transform: transform var(--t-duration); |
There was a problem hiding this comment.
Seems that transition-timing-function is not supported?
| transition: all 0.3s ease; | ||
| transition: all var(--t-duration-slide) ease; | ||
| } | ||
|
|
||
| .fade-slide-y-leave-active { | ||
| transition: all 0.3s cubic-bezier(1, 0.5, 0.8, 1); | ||
| transition: all var(--t-duration-slide) cubic-bezier(1, 0.5, 0.8, 1); |
There was a problem hiding this comment.
As the other parts of are not customizable, I think we could just leave the duration constant?
| --t-duration: 0.3s; | ||
| --t-duration-slide: 0.3s; | ||
| --t-timing-function: ease; |
There was a problem hiding this comment.
Maybe we could merge them into a single var, because "duration" and "timing function" always appear together in following usage.
There was a problem hiding this comment.
OK. Users can control transition-duration, transition-timing-function and transition-delay in a single variable in this way.
| --t-options: 0.3s ease; | ||
| --t-all: all var(--t-options); | ||
| --t-opacity: opacity var(--t-options); | ||
| --t-color: color var(--t-options); | ||
| --t-border-color: border-color var(--t-options); | ||
| --t-background-color: background-color var(--t-options); | ||
| --t-color-all: var(--t-color), var(--t-border-color), | ||
| var(--t-background-color); | ||
| --t-transform: transform var(--t-options); |
There was a problem hiding this comment.
One more thing: there are two different naming patterns
--t-${propertyName}:--t-all,--t-opacity,--t-color--t-${somethingElse}:--t-options,--t-color-all
Any good idea to normalize the naming pattern?
There was a problem hiding this comment.
Maybe an extra prefix --t-p- for property name? 🤔
| --t-p-all: all var(--t-options); | ||
| --t-p-opacity: opacity var(--t-options); | ||
| --t-p-transform: transform var(--t-options); | ||
| --t-p-color: color var(--t-options); | ||
| --t-p-border-color: border-color var(--t-options); | ||
| --t-p-background-color: background-color var(--t-options); | ||
| --t-color-all: var(--t-p-color), var(--t-p-border-color), | ||
| var(--t-p-background-color); |
There was a problem hiding this comment.
Well, after some discussion, both @shigma and me think we might not need so many vars for transitions.
IMO, only the --t-options is helpful for reusing the transition options. 🤔
There was a problem hiding this comment.
Although this PR is intended to provide scalability, I believe consistency is also important to a theme.
For theme-default, two transition variables are enough. One for color (color, border-color, background-color, etc) and the other for position (left, right, top, bottom, width, height, transform, etc).
There was a problem hiding this comment.
What naming convention is better to use in _variables.scss?
There was a problem hiding this comment.
I think using color var(--t-color-options) directly is enough 🤔 No need to add extra scss variables
| @@ -1,3 +1,5 @@ | |||
| @import '_variables'; | |||
meteorlxy
changed the title
3 participants
From this review