Audit and fix theme token updates against KDS
Summary
Updates Kolibri to KDS theming updates introduced by https://github.com/learningequality/kolibri-design-system/pull/551.
References
- Closes https://github.com/learningequality/kolibri/issues/11867
- KDS PR: https://github.com/learningequality/kolibri-design-system/pull/551
- Designs: https://www.figma.com/file/p7aOP2MBv0SED1uhaPwqEd/Product-rebrand?type=design&node-id=426-7545&mode=design
Reviewer guidance
- Please study the new theme first https://deploy-preview-551--kolibri-design-system.netlify.app/colors/#
Development
- [ ] Are code changes fine?
- [ ] Read the upgrade process below and preview regexes I used. Can you think of any important area I left out?
QA
- [ ] Can you see some glitches in regards to colors?
- [ ] Are contrast ratios good?
- [ ] Please test states that are often not obviously visible - errors, warning, and similar
- [ ] Mobile device
You may get a good sense on what to look from the "Upgrade process" notes below.
Upgrade process
Available colors and tokens can be found at https://deploy-preview-551--kolibri-design-system.netlify.app/
(1) Breaking KDS API changes
(a) KDS removed palette colors: purple, deeppurple, indigo, blue, brown, cyan, teal, lightgreen, lime, yellow, amber, deeporange, bluegrey
Search for
palette.purple|palette.deeppurple|palette.indigo|palette.brown|palette.cyan|palette.teal|palette.lightgreen|palette.lime|palette.amber|palette.deeporange|palette.bluegrey
and
palette\(\).purple|palette\(\).deeppurple|palette\(\).indigo|palette\(\).brown|palette\(\).cyan|palette\(\).teal|palette\(\).lightgreen|palette\(\).lime|palette\(\).amber|palette\(\).deeporange|palette\(\).bluegrey
and replace with closest available palette colors
(b) KDS removed grey scale values: palette.grey.v_300, palette.grey.v_500, palette.grey.v_700, palette.grey.v_900
Search for
palette.grey.v_300|palette.grey.v_500|palette.grey.v_700|palette.grey.v_900
and
palette\(\).grey.v_300|palette\(\).grey.v_500|palette\(\).grey.v_700|palette\(\).grey.v_900
and replace with the closest darker grey scale values that are available
(c) KDS removed scale values for brand and palette colors (except palette.grey): v_50,v_100, v_300, v_500, v_700, v_900
Search for
(?<!grey)\.v_(50|100|300|500|700|900)\b
and replace with the closest scale value that is available
(d) KDS removed content-related tokens: exercise, video, audio, document, html5, slideshow
Search for
tokens.exercise|tokens.video|tokens.audio|tokens.document|tokens.html5
and
tokens\(\).exercise|tokens\(\).video|tokens\(\).audio|tokens\(\).document|tokens\(\).html5
and replace with relevant tokens
(e) KDS removed other tokens: appBarFullscreen, appBarFullscreenDark, linkDark
Search for
tokens.appBarFullscreen|tokens.appBarFullscreenDark|tokens.linkDark
and
tokens\(\).appBarFullscreen|tokens\(\).appBarFullscreenDark|tokens\(\).linkDark
and replace with relevant tokens
(2) Hardcoded color values
Search the most commonly used CSS properties that can receive color
color|background|border|outline-color|box-shadow|text-shadow|fill|stroke|outline|text-decoration-color
and locate any hex, rgb(a), hsl(a),named color values to be replaced with relevant colors (themed ideally, but if blocked replace with new hardcoded color and open follow-up)
(3) Colors
Some colors in the new palette have the same name but look differently. Due to reduction in scales, they will now often look lighter. Preview colors and see if higher scale value is needed to ensure good contrast ratio or if another colors need to be used to work in the new theme. This applies particularly to non-grey colors.
(?<!grey)\.v_(200|400|600|800|1000|1100)\b
(4) Pages background color
Previously, we used palette.grey.v_100 to set page background color. It needs to be updated to lighter .v_50. There are more components where it's being set. Search for places that set some kind of background color with palette.grey.v_100 and see if it needs to be changed to palette.grey.v_50.
(5) Contrast
Check places that utilize tokens.text and tokens.textInverted together with setting background color via palette for sufficient contrast.
(6) Brand colors
Check places where brand.primary and brand.secondary is used and see if they need to be switched.
Testing checklist
- [ ] Contributor has fully tested the PR manually
- [ ] If there are any front-end changes, before/after screenshots are included
- [ ] Critical user journeys are covered by Gherkin stories
- [ ] Critical and brittle code paths are covered by unit tests
PR process
- [ ] PR has the correct target branch and milestone
- [ ] PR has 'needs review' or 'work-in-progress' label
- [ ] If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
- [ ] If this is an important user-facing change, PR or related issue has a 'changelog' label
- [ ] If this includes an internal dependency change, a link to the diff is provided
Reviewer checklist
- Automated test coverage is satisfactory
- PR is fully functional
- PR has been tested for accessibility regressions
- External dependency files were updated if necessary (
yarnandpip) - Documentation is updated
- Contributor is in AUTHORS.md
Build Artifacts
| Asset type | Download link |
|---|---|
| PEX file | kolibri-0.16.1a0.dev0_git.61.g582fe861.pex |
| Windows Installer (EXE) | kolibri-0.16.1a0.dev0+git.61.g582fe861-windows-setup-unsigned.exe |
| Debian Package | kolibri_0.16.1a0.dev0+git.61.g582fe861-0ubuntu1_all.deb |
| Mac Installer (DMG) | kolibri-0.16.1a0.dev0+git.61.g582fe861-0.4.0.dmg |
| Android Package (APK) | kolibri-0.16.1a0.dev0+git.61.g582fe861-0.1.1-debug.apk |
| TAR file | kolibri-0.16.1a0.dev0+git.61.g582fe861.tar.gz |
| WHL file | kolibri-0.16.1a0.dev0+git.61.g582fe861-py2.py3-none-any.whl |
HI @MisRob! I update the regex (?<!grey)\.v_(50|100|300|500|700|900) to (?<!grey)\.v_(50|100|300|500|700|900)\b as it was picking up any v_* including the mentioned values and not the actual values. As such, I was able to pickup v_1000. I also updated a another similar to this.
Makes sense, thanks @akolson
After the discussion about the text input fields on Slack, and subsequent changes by @LianaHarris360, that component now seems to have sufficient contrast, but there is a similar contrast issue with KSelect in the Library page:
@LianaHarris360 again dug into code and found out that KSelect component in the file kolibri/plugins/learn/assets/src/views/SearchFiltersPanel/SelectGroup.vue has the background manually set to $themePalette.grey.v_200 which is the HEX #CCCCCC, and that is way too dark for the labels to have sufficient contrast.
Can we make sure to apply the same front- and background color combinations as for the input text fields? Thank you!
I also want to flag that the original KSelect component has no background color at all, but after taking another look at the Figma specs, it should have the same background color as the other input components. I will update the KDS PR to reflect this change.
Alright, thank you. I will remove select background color override from Kolibri.
Selects styling is fixed
Since there's still time, I will do some more work here fixing issues that @jtamiace reported https://www.notion.so/learningequality/Kolibri-rebranded-first-preview-a8c0dc1a63a24071931dac150af0c014
We still need to replace the logo, but other than that, I think everything that has come up so far is resolved - let's review and QA so there's time to incorporate new feedback.
Hi @MisRob I have identified a number of issues mainly about the behavior and presentation of the modals, the search fields and the drop-down lists. As I have too many screenshots and videos I have added them to this shared folder so that you can take a look and identify the ones that are caused by changes made by this PR.
Also, it appears that the LanguageSwitcherList component is not being displayed on the login page.
Hrm, is it possible that we'll need to backport @AlexVelezLl's work from develop around this? There's this PR: https://github.com/learningequality/kolibri/pull/11977 but I also think some previous work as well.
Hmm Misha suggested not using the KListWithOverflow component in 0.16 because of IE support @rtibbles.
This seems to be a regression introduced in #11847, but I think we can just revert it if we can live with #7997 in 0.16.
And leave the more general solution from #11977 in develop.
Does that mean that this issue is currently extant in 0.16 then?
Nvm its not from #11847. Just tested it:
Well, that's a relief!
Seems like it is a regression from this PR specifically then.
Thank you @LianaHarris360 and @pcenov for feedback, that's really valuable. I believe that after it's all resolved, the PR will be robust. @pcenov I think some of the issues will definitely be related to rebrand and some probably not. There's still time so I will see if I can work through it all here since it may be faster than opening so many follow-ups. In case there are some left, I will take care of opening issues. Would you mind if I added 'resolved' subfolder to your folder and moved resolved issues in there?
Hi @MisRob - please feel free to create a 'resolved' subfolder or any other folders as needed. :)
Also, it appears that the LanguageSwitcherList component is not being displayed on the login page.
@AlexVelezLl fixed it in https://github.com/learningequality/kolibri-design-system/pull/612 so after I release the new KDS version next week, it will be resolved.
Summary of next steps on this PR:
- [x] @MisRob to release and install the new KDS with the fix for the language switcher list not being displayed on the login page, and select and modal issues
- [x] @MisRob to resolve https://drive.google.com/drive/u/1/folders/1C9jqrXs0h3ZYVCEpwyq5I7NAMdbBkEtO
- [x] @LianaHarris360 to resolve https://drive.google.com/drive/u/1/folders/13n0OWwzRR62eSI87Brtl2yQr_51Q2-jw
- [ ] @rtibbles to introduce the new logo
@LianaHarris360 fixed the modal and select bugs found by @pcenov here https://github.com/learningequality/kolibri-design-system/pull/616 and it will be available after I create a new release and install it here.
All feedback should now be resolved
Great work everybody :)!