Migrate all core theming to vuex driven dynamic styles #4587
Conversation
rtibbles
force-pushed
the
dynamic_styles
branch
from
dd6a820 to
7f44768
Compare
Codecov Report
@@ Coverage Diff @@
## develop #4587 +/- ##
===========================================
+ Coverage 52.57% 75.7% +23.12%
===========================================
Files 725 246 -479
Lines 23890 11496 -12394
Branches 3174 1371 -1803
===========================================
- Hits 12561 8703 -3858
+ Misses 10649 2481 -8168
+ Partials 680 312 -368Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## develop #4587 +/- ##
===========================================
+ Coverage 52.52% 52.79% +0.26%
===========================================
Files 726 736 +10
Lines 24128 24425 +297
Branches 3218 3257 +39
===========================================
+ Hits 12674 12895 +221
- Misses 10773 10847 +74
- Partials 681 683 +2
Continue to review full report at Codecov.
|
| if (process.env.NODE !== 'production') { | ||
| const colourPicker = require('../utils/colourPicker').default; | ||
| colourPicker.start(); | ||
| } |
There was a problem hiding this comment.
This is fun, but I don't think it should be turned on by default in the devserver.
Also, colourPicker.js is unnecessary. You can just drop ColourPicker.vue in CoreBase.vue.
Maybe add a prop on CoreBase like
showColorPicker: {
type: Boolean,
default: false,
}and then add to the template
...
<GlobalSnackbar />
<ColourPicker v-if="showColorPicker" />
</div>
</template>There was a problem hiding this comment.
Yeah, I did it like this so that it wouldn't even get bundled into the production build.
|
getting some travis failures too |
rtibbles
force-pushed
the
dynamic_styles
branch
from
69026b5 to
69856a1
Compare
Summary
#4524 did a lot of good things, but by separating each build into a separate, independent process, it broke the ability to do dynamic coreAPI injection at build time. Along with this, it also broke the ability to do custom styling of Kolibri at build time using a build time specified external plugin.
This PR rectifies some of this in a slightly different way by migrating all styling that hitherto has been done by variables specified in
core-theme.scssto be driven by a core object in Vuex state.As a bonus, it also adds a colour customization tool in dev mode to let people dynamically change styles.
…
Reviewer guidance
Check that styling is still applied correctly in the many components that have been refactored - I still have some more work to do manually verifying all this also, although I was mostly checking as I went, and did a post-pass of high visibility areas.
Also read the additions to the docs and see if they make sense!
References
No current issues this addresses, but will enable custom theming for accessibility and cosmetic purposes in future.
Contributor Checklist
PR process:
Testing:
Reviewer Checklist
yarnandpip)