Make KToolbar themeable, fix textColor validator check error in Kolibri, and remove obsolete Keen code #1153
Conversation
- Make it themeable - Remove unused logic - Remove Keen dependencies - Bugfixes - Add visual tests - Update documentation
| /** | ||
| * Controls the toolbar appearance and styling. Options are: | ||
| * - 'default': Standard white background with shadow | ||
| * - 'colored': Uses theme primary color as background |
There was a problem hiding this comment.
'colored' option removed. Background color can be customized via the backgroundColor prop if future need arises.
Audit
Not used from anywhere.
| * Controls the text color throughout the toolbar. Use 'white' for dark backgrounds | ||
| * and 'black' for light backgrounds to ensure proper contrast. | ||
| */ | ||
| textColor: { |
There was a problem hiding this comment.
'black' and 'white' restriction removed. Now accepts all values, including our theme.
| /** | ||
| * Whether to hide the navigation icon button completely | ||
| */ | ||
| removeNavIcon: { |
There was a problem hiding this comment.
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)
| /** | ||
| * Icon name displayed in the navigation button when using the default icon slot | ||
| */ | ||
| navIcon: { |
There was a problem hiding this comment.
Removed (utilized obsolete UiIconButton).
Audit
- Not used from anywhere (we always use the
#iconslot since we need granularKIconButtonconfiguration)
| /** | ||
| * Whether to show the loading progress indicator | ||
| */ | ||
| loading: { |
There was a problem hiding this comment.
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
| /** | ||
| * Position of the loading progress bar indicator. Options are 'top' or 'bottom'. | ||
| */ | ||
| progressPosition: { |
There was a problem hiding this comment.
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
| * Emitted when the navigation icon is clicked | ||
| * @event nav-icon-click | ||
| */ | ||
| this.$emit('nav-icon-click'); |
There was a problem hiding this comment.
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#iconslot and emitnav-icon-clickfromKIconButton)
Percy Visual Test ResultsPercy Dashboard: View Detailed Report Environment:
Instructions for Reviewers:
|
MisRob
changed the title
MisRob
added
TODO: needs review
|
Needs some decisions in regard to release planning (Slack thread) |
2 participants
Background
Kolibri expects
KToolbarto accept theme color viatextColor, such as here, butKToolbarwas only able to work with two hardcoded colors'black'and'white'.We recently added validators to
KToolbarreflecting 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
KToolbarto be themeable.Description
Makes
KToolbarthemeable 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