Skip to content

Update meter to support semantic variables #6913

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

Open
GZolla wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −14
Open

Update meter to support semantic variables #6913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions components/meter/meter-linear.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class MeterLinear extends MeterMixin(LitElement) {
}

.d2l-meter-linear-full-bar {
background-color: var(--d2l-color-gypsum);
Copy link
Copy Markdown
Contributor

@dbatiste dbatiste May 5, 2026

Choose a reason for hiding this comment

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

If you haven't already, I think it would be worth discussing this mapping with Jeff since this isn't really an interactive element.

background-color: var(--d2l-theme-background-color-interactive-secondary-default);
position: relative;
}

Expand All @@ -57,7 +57,7 @@ class MeterLinear extends MeterMixin(LitElement) {
}

.d2l-meter-linear-inner-bar {
background-color: var(--d2l-color-celestine);
background-color: var(--d2l-theme-brand-color-primary-default);
Copy link
Copy Markdown
Contributor

@dbatiste dbatiste May 5, 2026

Choose a reason for hiding this comment

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

Would be worth confirming this mapping - we don't have a semantic variable for this type of thing (progress, level of a meter).

inset-inline-start: 0;
max-width: 100%;
position: absolute;
Expand All @@ -68,7 +68,7 @@ class MeterLinear extends MeterMixin(LitElement) {
}

.d2l-meter-linear-text {
color: var(--d2l-color-ferrite);
color: var(--d2l-theme-text-color-static-standard);
display: flex;
flex-direction: row;
gap: 0.45rem;
Expand Down
8 changes: 4 additions & 4 deletions components/meter/meter-styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@ export const meterStyles = css`
stroke-linecap: round;
}
.d2l-meter-full-bar {
stroke: var(--d2l-color-gypsum);
stroke: var(--d2l-theme-border-color-subtle);
Copy link
Copy Markdown
Contributor

@dbatiste dbatiste May 5, 2026

Choose a reason for hiding this comment

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

This is effectively being used as a background color, so this doesn't seem right to me. --d2l-theme-background-color-interactive-secondary-default maps to gypsum, but that doesn't seem right either, since this isn't really an interactive element.

}
:host([foreground-light]) .d2l-meter-full-bar {
stroke: rgba(255, 255, 255, 0.5);
}
.d2l-meter-progress-bar {
stroke: var(--d2l-color-celestine);
Copy link
Copy Markdown
Contributor

@dbatiste dbatiste May 5, 2026

Choose a reason for hiding this comment

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

Ditto with this one. Might be ok, but we should confirm that we want this to be the brand color.

stroke: var(--d2l-theme-brand-color-primary-default);
}
:host([foreground-light]) .d2l-meter-progress-bar {
stroke: white;
}
.d2l-meter-text {
color: var(--d2l-color-ferrite);
fill: var(--d2l-color-ferrite);
color: var(--d2l-theme-text-color-static-standard);
fill: var(--d2l-theme-text-color-static-standard);
line-height: 0.8rem;
text-align: center;
}
Expand Down
6 changes: 3 additions & 3 deletions components/meter/test/meter-linear.vdiff.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('meter-linear', () => {
});

[
{ name: 'normal-text', template: html`<d2l-meter-linear id="normal-text" value="4" max="10" text="You're doing great!"></d2l-meter-linear>` },
{ name: 'normal-text', template: html`<d2l-meter-linear id="normal-text" value="4" max="10" text="You're doing great!"></d2l-meter-linear>`, allColorModes: true },
{ name: 'normal-max-zero-value-zero', template: html`<d2l-meter-linear value="0" max="0" text="Visited: {x/y}" percent></d2l-meter-linear>` },
{ name: 'normal-round-to-zero', template: html`<d2l-meter-linear value="0.004" max="10" text="Visited: {x/y}" percent></d2l-meter-linear>` },
{ name: 'normal-over-100', template: html`<d2l-meter-linear value="15" max="10" text="Visited: {x/y}" percent></d2l-meter-linear>` },
Expand All @@ -61,10 +61,10 @@ describe('meter-linear', () => {
{ name: 'normal-text-fraction-text-hidden', template: html`<d2l-meter-linear value="4" max="10" text="Visited: {x/y}" percent text-hidden></d2l-meter-linear>` },
{ name: 'text-inline-text-percent-text-hidden', template: html`<d2l-meter-linear value="4" max="10" text-inline text="Visited" percent text-hidden></d2l-meter-linear>` },
{ name: 'no-text-text-hidden', template: html`<d2l-meter-linear value="4" max="10" text-hidden></d2l-meter-linear>` },
].forEach(({ name, template, dark }) => {
].forEach(({ name, template, dark, allColorModes }) => {
it(name, async() => {
const elem = await fixture(createTemplateWrapper(template, dark));
await expect(elem.querySelector('d2l-meter-linear')).to.be.golden();
await expect(elem.querySelector('d2l-meter-linear')).to.be.golden({ allColorModes });
});
});
});
8 changes: 4 additions & 4 deletions components/meter/test/meter-radial.vdiff.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import { expect, fixture, html } from '@brightspace-ui/testing';
describe('meter-radial', () => {
[true, false].forEach(rtl => {
[
{ name: 'progress', template: html`<d2l-meter-radial value="16" max="47"></d2l-meter-radial>` },
{ name: 'progress', template: html`<d2l-meter-radial value="16" max="47"></d2l-meter-radial>`, allColorModes: true },
{ name: 'percent', template: html`<d2l-meter-radial value="16" max="47" percent></d2l-meter-radial>` },
{ name: 'text', template: html`<d2l-meter-radial value="10" max="10" percent text="Completed"></d2l-meter-radial>` },
].forEach(({ name, template }) => {
{ name: 'text', template: html`<d2l-meter-radial value="10" max="10" percent text="Completed"></d2l-meter-radial>`, allColorModes: true },
].forEach(({ name, template, allColorModes }) => {
it(`${name}${ rtl ? '-rtl' : ''}`, async() => {
const elem = await fixture(template, { rtl });
await expect(elem).to.be.golden();
await expect(elem).to.be.golden({ allColorModes: allColorModes && !rtl });
});
});
});
Expand Down
Loading