Make KToolbar themeable, fix textColor validator check error in Kolibri, and remove obsolete Keen code by MisRob · Pull Request #1153 · learningequality/kolibri-design-system

MisRob · 2025-11-04T19:27:24Z

Background

Kolibri expects KToolbar to accept theme color via textColor, such as here, but KToolbar was only able to work with two hardcoded colors 'black' and 'white'.

We recently added validators to KToolbar reflecting this restriction that now cause Kolibri console warnings:

Rather than reverting the validators, this PR addresses the issue by resolving the underlying problem of the unmet expectation for KToolbar to be themeable.

Description

Makes KToolbar themeable and cleans up unused or not functional code (this was easier than trying to update all intertwined logic related to colors, and is aligned with Keen tech debt removal).

First, I audited our codebases with this search, then update in such a way that should cause no breaking changes. Some removed APIs are used from Kolibri, but they are all obsolete code that don't seem to take effect - see review notes for details.

Also adds visual tests and has documentation fixes and further improvements.

Changelog

TBD

Steps to test

Did I remove/change something that shouldn't be?
Preview documentation
Preview visual tests

- Make it themeable - Remove unused logic - Remove Keen dependencies - Bugfixes - Add visual tests - Update documentation

MisRob · 2025-11-05T11:22:40Z

lib/KToolbar.vue

-      /**
-       * Controls the toolbar appearance and styling. Options are:
-       * - 'default': Standard white background with shadow
-       * - 'colored': Uses theme primary color as background


'colored' option removed. Background color can be customized via the backgroundColor prop if future need arises.

Audit

Not used from anywhere.

MisRob · 2025-11-05T11:22:57Z

lib/KToolbar.vue

-       * Controls the text color throughout the toolbar. Use 'white' for dark backgrounds
-       * and 'black' for light backgrounds to ensure proper contrast.
-       */
-      textColor: {


'black' and 'white' restriction removed. Now accepts all values, including our theme.

MisRob · 2025-11-05T11:23:23Z

lib/KToolbar.vue

-      /**
-       * Whether to hide the navigation icon button completely
-       */
-      removeNavIcon: {


Removed.

Audit

Used in Kolibri from AppBar but doesn't take effect (whether the navigation icon is displayed or not is configured in the #icon slot condition)

MisRob · 2025-11-05T11:23:09Z

lib/KToolbar.vue

-      /**
-       * Icon name displayed in the navigation button when using the default icon slot
-       */
-      navIcon: {


Removed (utilized obsolete UiIconButton).

Audit

Not used from anywhere (we always use the #icon slot since we need granular KIconButton configuration)

MisRob · 2025-11-05T11:23:47Z

lib/KToolbar.vue

-      /**
-       * Whether to show the loading progress indicator
-       */
-      loading: {


Removed. If we need KToolbar to have API for linear loading progress in the future, it'd be better to utilize KLinearLoader and expose APIs that are meaningful.

Audit

Not used from anywhere

MisRob · 2025-11-05T11:23:35Z

lib/KToolbar.vue

-      /**
-       * Position of the loading progress bar indicator. Options are 'top' or 'bottom'.
-       */
-      progressPosition: {


Removed. If we need KToolbar to have API for linear loading progress in the future, it'd be better to utilize KLinearLoader and expose APIs that are meaningful.

Audit

Not used from anywhere

MisRob · 2025-11-05T11:23:59Z

lib/KToolbar.vue

-         * Emitted when the navigation icon is clicked
-         * @event nav-icon-click
-         */
-        this.$emit('nav-icon-click');


Removed. Since navIcon and the default UiIconButton was removed, this will never be emitted.

Audit

Used in Kolibri from ImmersiveToolbar but doesn't take effect (since we don't use navIcon, but instead use the #icon slot and emit nav-icon-click from KIconButton)

github-actions · 2025-11-05T11:30:37Z

Percy Visual Test Results

Percy Dashboard: View Detailed Report

Environment:

Node.js Version: 18.x
OS: Ubuntu-latest

Instructions for Reviewers:

Click on the Percy Dashboard link to view detailed visual diffs.
Review the visual changes highlighted in the report.
Approve or request changes based on the visual differences.

MisRob · 2025-11-05T12:39:20Z

Needs some decisions in regard to release planning (Slack thread)

AlexVelezLl

Overall, this looks really nice; it feels like a lot of tech debt clean-up. In general, just left more reflective comments on how the design system concepts relate to how we have actually used this component in Kolibri, and if the efforts to standardize them would help in anything. Perhaps we will have a better idea when migrating the VToolbar component in Studio.

AlexVelezLl · 2025-12-08T22:26:46Z

docs/pages/ktoolbar.vue

    <DocsPageSection
      title="Usage"
      anchor="#usage"
    >
-      <!-- Example 1: Light theme with close icon -->
-      <h3>Light theme toolbar</h3>
-      <p>
-        This example demonstrates a toolbar with light theme colors using
-        <code>$themeTokens.appBar</code> background and <code>$themeTokens.text</code>
-        color, with a close icon in the icon slot.
-      </p>
+      <h3>Light theme</h3>
+
+      <p>Uses the light app bar theme token.</p>

      <DocsExample
        loadExample="KToolbar/LightTheme.vue"
        exampleId="ktoolbar-light-theme"
        block
      />

-      <!-- Example 2: Dark theme with back icon in router-link -->
-      <h3>Dark theme toolbar with router navigation</h3>
-      <p>
-        This example shows a dark-themed toolbar using
-        <code>$themeTokens.appBarDark</code> background and
-        <code>$themeTokens.textInverted</code> color, with a back navigation button wrapped in
-        router-link.
-      </p>
+      <h3>Dark theme</h3>
+
+      <p>Uses the dark app bar theme token.</p>


(Non-blocking) In general, it feels weird to call these examples "Light theme" and "Dark theme" because it feels like they are somehow related to a general light/dark app theme, but they are not. They are both used in the same theming but with different purposes as described in the appbars specs. It seems the difference resides in "primary bars/Full-screen modal bars".

Just looked at how these different styles are used, and in Kolibri, this concept only affects the ImmersiveToolbar components, and for ImmersiveToolbar, the differentiator factor is the isFullscreen prop, while on the ImmersivePage component, the differentiator factor is the primary prop.

This is definitely out of scope for this, but it may be good (or it may not be worth it, who knows! 😅) considering standardizing these concepts as we use this KToolbar component more in Studio. And this may be more of a conceptual discussion rather than changing anything on the KToolbar component, as right now it is implemented to be super flexible to match any layout.

Yes, I think it'd be meaningful for documentation to reflect how we think about toolbars. Alternatively, we may just simplify it and not use 'theme' language at all.

AlexVelezLl · 2025-12-09T12:37:30Z

lib/KToolbar/index.vue

+       * - `'default'`: `$themeTokens.surface` background with shadow
+       * - `'clear'`: Transparent background with shadow
+       */
+      type: {


This type prop is a bit "tricky", because in essence the "primary" purpose of this prop could be achieved by just setting a backgroundColor="transparent", and the result will be the same. e.g.:

<KToolbar title="Toolbar title" backgroundColor="transparent" />

<KToolbar type="clear" title="Toolbar title" />

But the only difference occurs when raised=false: if type="default", a bottom border is shown; if type="clear", it is not. Coincidentally (or not), the latter configuration is the one used in the main AppBar component in Kolibri here, so better be careful with that, we could potentially just remove the type and manage both the bottom border and the box shadow just with the raised prop, but again, not sure if this will be worth the effort.

Yes I agree that it'd be much clearer interface, and it'd be a change that would go well with other changes introduce here. I only didn't touch type in these areas to not cause a breaking change.

AlexVelezLl · 2025-12-09T12:43:59Z

lib/KToolbar/index.vue

+      textColor: {
+        type: String,
+        default: null,
+      },
+      /**
+       * Background color
+       */
+      backgroundColor: {
+        type: String,
+        default: null,
+      },


Just noting that we can have the same behavior if we use the style prop to set the color and background color. In some places in Kolibri, it seems this is the preferred way to do this.

Just reflecting on the tradeoff and whether we want to prefer these more general "props" like style to override these styles, or prefer the specific props so that we can have them better documented.

I am not sure why I added it as a prop - it's possible that I didn't reflect on it, or there might be some dependencies to other props in connection with the limited scope of changes.

I agree that whenever we can use native

:style= { backgroundColor: ..., color: ... }

that'd be ideal.

So yes in long-term, I would prefer we drop backgroundColor and possibly textColor too.

MisRob · 2025-12-17T14:13:16Z

Thank you @AlexVelezLl, those are good notes.

Based on what we discussed around releases, I will close this in favor of

Fix documentation examples not loaded

c9398b4

MisRob had a problem deploying to percy_tests November 4, 2025 19:27 — with GitHub Actions Failure

KToolbar maintanence

2188ef5

- Make it themeable - Remove unused logic - Remove Keen dependencies - Bugfixes - Add visual tests - Update documentation

MisRob force-pushed the k-toolbar branch from c8b2de7 to 2188ef5 Compare November 5, 2025 11:21

MisRob temporarily deployed to percy_tests November 5, 2025 11:22 — with GitHub Actions Inactive

MisRob commented Nov 5, 2025

View reviewed changes

MisRob changed the title ~~KToolbar maintenance~~ Make KToolbar themeable, fix textColor validator check error in Kolibri, and remove obsolete Keen code Nov 5, 2025

MisRob requested review from AlexVelezLl and marcellamaki November 5, 2025 12:26

MisRob added TODO: needs review Waiting for review TODO: needs decisions Further discussion and planning necessary labels Nov 5, 2025

AlexVelezLl self-assigned this Nov 5, 2025

MisRob mentioned this pull request Nov 18, 2025

[Remove Vuetify from Studio] Channel details in Channels - page layout learningequality/studio#5510

Merged

5 tasks

AlexVelezLl reviewed Dec 9, 2025

View reviewed changes

This was referenced Dec 15, 2025

Fix textColor console warning learningequality/kolibri#14007

Merged

Fix KToolbar documentation examples not loaded #1182

Merged

Complete KToolbar public interface, make it fully themeable, and cleanup obsolete Keen code #1183

Open

MisRob closed this Dec 17, 2025

MisRob had a problem deploying to percy_tests December 17, 2025 14:13 — with GitHub Actions Failure

Conversation

MisRob commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Description

Changelog

Steps to test

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 5, 2025

Percy Visual Test Results

Uh oh!

MisRob commented Nov 5, 2025

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MisRob commented Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MisRob commented Nov 4, 2025 •

edited

Loading