Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppBar: Use appBarText for text/icon colors #12937

Merged
merged 8 commits into from
Dec 16, 2024
+46 −13
Merged

AppBar: Use appBarText for text/icon colors #12937

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ae719c
AppBar: Use appBarText for text/icon colors
nucleogenesis Dec 11, 2024
4c0bee9
Include appBar in themeconfig; validate in themeSpec for appBar image…
nucleogenesis Dec 12, 2024
1851e10
use themeTokens in themeSpec for appbar
nucleogenesis Dec 12, 2024
ec00222
Update appbar & sidenav to use appbar themeConfig values
nucleogenesis Dec 12, 2024
ad78b7d
fix incorrect uses of themeConfig and themeTokens utilities
nucleogenesis Dec 13, 2024
9e7134e
Remove prop-drilling, just use theemConfig
nucleogenesis Dec 13, 2024
c8af4c1
missed default text updated
nucleogenesis Dec 13, 2024
d33c457
bump patch version to 0.17.5
nucleogenesis Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions kolibri/core/assets/src/styles/themeConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import Vue from 'vue';

const themeConfig = Vue.observable({
appBar: {
background: null,
textColor: null,
topLogo: {
src: null,
alt: null,
Expand Down
20 changes: 20 additions & 0 deletions kolibri/core/assets/src/styles/themeSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logger from 'kolibri.lib.logging';
import { themeTokens } from 'kolibri-design-system/lib/styles/theme';
import isObject from 'lodash/isObject';

const logging = logger.getLogger(__filename);
Expand Down Expand Up @@ -42,6 +43,25 @@ const _imageSpec = {
};

export default {
appBar: {
type: Object,
default: null,
spec: {
background: {
type: String,
default: themeTokens.appBar,
},
textColor: {
type: String,
default: themeTokens.text,
},
topLogo: {
type: Object,
default: null,
spec: _imageSpec,
},
},
},
brandColors: {
type: Object,
default: null,
Expand Down
14 changes: 9 additions & 5 deletions kolibri/core/assets/src/views/AppBar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@

<div
v-show="!$isPrint"
:style="{ backgroundColor: $themeTokens.appBar }"
:style="{ backgroundColor: $themeConfig.appBar.background }"
>
<header>
<SkipNavigationLink />

<UiToolbar
:removeNavIcon="showAppNavView"
type="clear"
textColor="black"
:textColor="$themeConfig.appBar.textColor"
class="app-bar"
:style="{ height: topBarHeight + 'px' }"
:style="{
height: topBarHeight + 'px',
color: $themeConfig.appBar.textColor,
}"
:raised="false"
:removeBrandDivider="true"
>
Expand All @@ -26,7 +29,7 @@
>
<KIconButton
icon="menu"
:color="$themeTokens.text"
:color="$themeConfig.appBar.textColor"
:ariaLabel="$tr('openNav')"
@click="$emit('toggleSideNav')"
/>
Expand All @@ -49,6 +52,7 @@
<slot name="sub-nav">
<Navbar
v-if="links.length > 0"
:textColor="$themeConfig.appBar.textColor"
:navigationLinks="links"
/>
</slot>
Expand Down Expand Up @@ -94,7 +98,7 @@
<KIcon
icon="person"
:style="{
fill: $themeTokens.text,
fill: $themeConfig.appBar.textColor,
height: '24px',
width: '24px',
margin: '4px',
Expand Down
8 changes: 6 additions & 2 deletions kolibri/core/assets/src/views/Navbar/NavbarLink.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,15 @@
required: true,
validator: validateLinkObject,
},
textColor: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we avoid prop drilling here, and just reference the value directly? NavbarLink is only used in the Navbar component, and we remove it from the public API completely in develop, so we can just set it directly to the value we want.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - just updated this

type: String,
default: null,
},
},
computed: {
tabStyles() {
return {
color: this.$themeTokens.text,
color: this.textColor || this.$themeTokens.text,
':hover': {
'background-color': this.$themeBrand.secondary.v_600,
},
Expand All @@ -81,7 +85,7 @@
activeClasses() {
// return both fixed and dynamic classes
return `router-link-active ${this.$computedClass({
color: this.$themeTokens.text,
color: this.textColor || this.$themeTokens.text,
})}`;
},
},
Expand Down
9 changes: 7 additions & 2 deletions kolibri/core/assets/src/views/Navbar/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
ref="navLinks"
:title="link.title"
:link="link.link"
:textColor="textColor || $themeTokens.text"
>
<KIcon
:icon="link.icon"
:color="$themeTokens.text"
:color="textColor || $themeTokens.text"
/>
</NavbarLink>
</ul>
Expand All @@ -27,7 +28,7 @@
:ariaLabel="coreString('moreOptions')"
icon="optionsHorizontal"
appearance="flat-button"
:color="$themeTokens.text"
:color="textColor || $themeTokens.text"
:primary="false"
class="kiconbutton-style"
>
Expand Down Expand Up @@ -81,6 +82,10 @@
return values.every(value => value.link.name);
},
},
textColor: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, Navbar is only used in AppBar, and is also removed from the public API in develop, so I think we can just reference theme variables directly.

type: String,
default: null,
},
},
data() {
return { mounted: false };
Expand Down
2 changes: 1 addition & 1 deletion kolibri/core/assets/src/views/SideNav.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
height: topBarHeight + 'px',
width: `${width}`,
paddingTop: windowIsSmall ? '4px' : '8px',
backgroundColor: $themeTokens.appBar,
backgroundColor: $themeConfig.appBar.background,
}"
>
<KIconButton
Expand Down
Loading