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.

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

fix(meter, progress-bar, progress-circle): use innerText when label is not provided #3483

Merged
merged 19 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 22 additions & 4 deletions packages/meter/src/Meter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ import {
SpectrumElement,
TemplateResult,
} from '@spectrum-web-components/base';
import { property } from '@spectrum-web-components/base/src/decorators.js';
import {
property,
query,
} from '@spectrum-web-components/base/src/decorators.js';

import { getLabelFromSlot } from '@spectrum-web-components/shared/src/get-label-from-slot.js';
import { ObserveSlotText } from '@spectrum-web-components/shared/src/observe-slot-text.js';
import { LanguageResolutionController } from '@spectrum-web-components/reactive-controllers/src/LanguageResolution.js';
import '@spectrum-web-components/field-label/sp-field-label.js';
Expand Down Expand Up @@ -53,6 +57,9 @@ export class Meter extends SizedMixin(ObserveSlotText(SpectrumElement, '')) {
@property({ type: String, reflect: true })
public label = '';

@query('slot')
private slotEl!: HTMLSlotElement;

private languageResolver = new LanguageResolutionController(this);

@property({ type: Boolean, reflect: true, attribute: 'side-label' })
Expand All @@ -66,7 +73,7 @@ export class Meter extends SizedMixin(ObserveSlotText(SpectrumElement, '')) {
return html`
<sp-field-label size=${this.size} class="label">
${this.slotHasContent ? html`` : this.label}
<slot>${this.label}</slot>
<slot @slotchange=${this.handleSlotchange}>${this.label}</slot>
</sp-field-label>
<sp-field-label size=${this.size} class="percentage">
${new Intl.NumberFormat(this.languageResolver.language, {
Expand All @@ -83,6 +90,13 @@ export class Meter extends SizedMixin(ObserveSlotText(SpectrumElement, '')) {
`;
}

protected handleSlotchange(): void {
const labelFromSlot = getLabelFromSlot(this.label, this.slotEl);
if (labelFromSlot) {
this.label = labelFromSlot;
}
}

protected override firstUpdated(changes: PropertyValues): void {
super.firstUpdated(changes);
this.setAttribute('role', 'meter progressbar');
Expand All @@ -93,8 +107,12 @@ export class Meter extends SizedMixin(ObserveSlotText(SpectrumElement, '')) {
if (changes.has('progress')) {
this.setAttribute('aria-valuenow', '' + this.progress);
}
if (this.label && changes.has('label')) {
this.setAttribute('aria-label', this.label);
if (changes.has('label')) {
if (this.label.length) {
this.setAttribute('aria-label', this.label);
} else {
this.removeAttribute('aria-label');
}
}
}
}
9 changes: 9 additions & 0 deletions packages/meter/test/meter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@ describe('Meter', () => {
expect(el.getAttribute('aria-valuenow')).to.equal('100');
});

it('accepts label from `slot`', async () => {
const el = await fixture<Meter>(html`
<sp-meter>Label From Slot</sp-meter>
`);

await elementUpdated(el);

expect(el.getAttribute('label')).to.equal('Label From Slot');
});
it('accepts a changing process', async () => {
const el = await fixture<Meter>(html`
<sp-meter>Changing Value</sp-meter>
Expand Down
51 changes: 40 additions & 11 deletions packages/progress-bar/src/ProgressBar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,31 @@ import {
SpectrumElement,
TemplateResult,
} from '@spectrum-web-components/base';
import { property } from '@spectrum-web-components/base/src/decorators.js';
import {
property,
query,
} from '@spectrum-web-components/base/src/decorators.js';

import { getLabelFromSlot } from '@spectrum-web-components/shared/src/get-label-from-slot.js';
import { ObserveSlotText } from '@spectrum-web-components/shared/src/observe-slot-text.js';
import { LanguageResolutionController } from '@spectrum-web-components/reactive-controllers/src/LanguageResolution.js';
import '@spectrum-web-components/field-label/sp-field-label.js';
import styles from './progress-bar.css.js';

/**
* @element sp-progress-bar
*/
export class ProgressBar extends SizedMixin(SpectrumElement) {
export class ProgressBar extends SizedMixin(
ObserveSlotText(SpectrumElement, '')
) {
public static override get styles(): CSSResultArray {
return [styles];
}

@property({ type: Boolean, reflect: true })
public indeterminate = false;

@property({ type: String })
@property({ type: String, reflect: true })
public label = '';

private languageResolver = new LanguageResolutionController(this);
Expand All @@ -52,13 +59,23 @@ export class ProgressBar extends SizedMixin(SpectrumElement) {
@property({ type: String, reflect: true })
public static: 'white' | undefined;

@query('slot')
private slotEl!: HTMLSlotElement;

protected override render(): TemplateResult {
return html`
${this.label
${this.slotHasContent || this.label
? html`
<sp-field-label size=${this.size} class="label">
${this.label}
${this.slotHasContent ? html`` : this.label}
<slot @slotchange=${this.handleSlotchange}>
${this.label}
</slot>
</sp-field-label>
`
: html``}
${this.label
? html`
${this.indeterminate
? html``
: html`
Expand Down Expand Up @@ -86,6 +103,13 @@ export class ProgressBar extends SizedMixin(SpectrumElement) {
`;
}

protected handleSlotchange(): void {
const labelFromSlot = getLabelFromSlot(this.label, this.slotEl);
if (labelFromSlot) {
this.label = labelFromSlot;
}
}

protected override firstUpdated(changes: PropertyValues): void {
super.firstUpdated(changes);
if (!this.hasAttribute('role')) {
Expand All @@ -99,34 +123,39 @@ export class ProgressBar extends SizedMixin(SpectrumElement) {
if (this.indeterminate) {
this.removeAttribute('aria-valuemin');
this.removeAttribute('aria-valuemax');
this.removeAttribute('aria-valuenow');
Rajdeepc marked this conversation as resolved.
Show resolved Hide resolved
} else {
this.setAttribute('aria-valuemin', '0');
this.setAttribute('aria-valuemax', '100');
}
}
if (!this.indeterminate && changes.has('progress')) {
this.setAttribute('aria-valuenow', '' + this.progress);
} else if (this.hasAttribute('aria-valuenow')) {
this.removeAttribute('aria-valuenow');
}
if (this.label && changes.has('label')) {
this.setAttribute('aria-label', this.label);
if (changes.has('label')) {
if (this.label.length) {
this.setAttribute('aria-label', this.label);
} else {
this.removeAttribute('aria-label');
}
}

if (window.__swc.DEBUG) {
if (
!this.label &&
!this.getAttribute('aria-label') &&
!this.getAttribute('aria-labelledby')
!this.getAttribute('aria-labelledby') &&
!this.slotHasContent
) {
Westbrook marked this conversation as resolved.
Show resolved Hide resolved
window.__swc.warn(
this,
'<sp-progress-bar> elements will not be accessible to screen readers without one of the following:',
'<sp-progress-bar> elements need one of the following to be accessible:',
'https://opensource.adobe.com/spectrum-web-components/components/progress-bar/#accessibility',
{
type: 'accessibility',
issues: [
'value supplied to the "label" attribute, which will be displayed visually as part of the element, or',
'text content supplied directly to the <sp-progress-circle> element, or',
'value supplied to the "aria-label" attribute, which will only be provided to screen readers, or',
'an element ID reference supplied to the "aria-labelledby" attribute, which will be provided by screen readers and will need to be managed manually by the parent application.',
],
Expand Down
11 changes: 11 additions & 0 deletions packages/progress-bar/test/progress-bar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ describe('ProgressBar', () => {
await expect(el).to.be.accessible();
});

it('accepts label from `slot`', async () => {
const el = await fixture<ProgressBar>(html`
<sp-progress-bar role="progressbar">
Label From Slot
</sp-progress-bar>
`);

await elementUpdated(el);

expect(el.getAttribute('label')).to.equal('Label From Slot');
});
it('accepts a changing progress', async () => {
const el = await fixture<ProgressBar>(html`
<sp-progress-bar label="Changing value"></sp-progress-bar>
Expand Down
31 changes: 26 additions & 5 deletions packages/progress-circle/src/ProgressCircle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ import {
SpectrumElement,
TemplateResult,
} from '@spectrum-web-components/base';
import { property } from '@spectrum-web-components/base/src/decorators.js';
import {
property,
query,
} from '@spectrum-web-components/base/src/decorators.js';
import { getLabelFromSlot } from '@spectrum-web-components/shared/src/get-label-from-slot.js';
import { ifDefined } from '@spectrum-web-components/base/src/directives.js';

import progressCircleStyles from './progress-circle.css.js';
Expand Down Expand Up @@ -48,6 +52,9 @@ export class ProgressCircle extends SizedMixin(SpectrumElement, {
@property({ type: Number })
public progress = 0;

@query('slot')
private slotEl!: HTMLSlotElement;

private makeRotation(rotation: number): string | undefined {
return this.indeterminate
? undefined
Expand Down Expand Up @@ -83,6 +90,7 @@ export class ProgressCircle extends SizedMixin(SpectrumElement, {
];
const masks = ['Mask1', 'Mask2'];
return html`
<slot @slotchange=${this.handleSlotchange}></slot>
<div class="track"></div>
<div class="fills">
${masks.map(
Expand All @@ -101,6 +109,13 @@ export class ProgressCircle extends SizedMixin(SpectrumElement, {
`;
}

protected handleSlotchange(): void {
const labelFromSlot = getLabelFromSlot(this.label, this.slotEl);
if (labelFromSlot) {
this.label = labelFromSlot;
}
}

Westbrook marked this conversation as resolved.
Show resolved Hide resolved
protected override firstUpdated(changes: PropertyValues): void {
super.firstUpdated(changes);
if (!this.hasAttribute('role')) {
Expand All @@ -115,24 +130,30 @@ export class ProgressCircle extends SizedMixin(SpectrumElement, {
} else if (this.hasAttribute('aria-valuenow')) {
this.removeAttribute('aria-valuenow');
}
if (this.label && changes.has('label')) {
this.setAttribute('aria-label', this.label);
if (changes.has('label')) {
if (this.label.length) {
this.setAttribute('aria-label', this.label);
} else {
this.removeAttribute('aria-label');
}
}

if (window.__swc.DEBUG) {
if (
!this.label &&
!this.getAttribute('aria-label') &&
!this.getAttribute('aria-labelledby')
!this.getAttribute('aria-labelledby') &&
!this.slotEl.assignedNodes().length
) {
window.__swc.warn(
this,
'<sp-progress-circle> elements will not be accessible to screen readers without one of the following:',
'<sp-progress-circle> elements need one of the following to be accessible:',
'https://opensource.adobe.com/spectrum-web-components/components/progress-circle/#accessibility',
{
type: 'accessibility',
issues: [
'value supplied to the "label" attribute, which will be displayed visually as part of the element, or',
'text content supplied directly to the <sp-progress-circle> element, or',
'value supplied to the "aria-label" attribute, which will only be provided to screen readers, or',
'an element ID reference supplied to the "aria-labelledby" attribute, which will be provided by screen readers and will need to be managed manually by the parent application.',
],
Expand Down
4 changes: 4 additions & 0 deletions packages/progress-circle/src/progress-circle.css
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@ governing permissions and limitations under the License.
--spectrum-alias-track-fill-color-overbackground
);
}

slot {
display: none;
}
12 changes: 6 additions & 6 deletions packages/progress-circle/stories/progress-circle.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,18 @@ export const Default = ({ indeterminate }: StoryArgs = {}): TemplateResult => {
progress="27"
size="s"
?indeterminate=${indeterminate}
label="Loading progess demo"
label="Loading progress demo"
></sp-progress-circle>
<sp-progress-circle
progress="27"
?indeterminate=${indeterminate}
label="Loading progess demo"
label="Loading progress demo"
></sp-progress-circle>
<sp-progress-circle
progress="27"
size="l"
?indeterminate=${indeterminate}
label="Loading progess demo"
label="Loading progress demo"
></sp-progress-circle>
</div>
`;
Expand All @@ -79,20 +79,20 @@ export const staticWhite = ({
static="white"
size="s"
?indeterminate=${indeterminate}
label="Loading progess demo"
label="Loading progress demo"
></sp-progress-circle>
<sp-progress-circle
progress="53"
static="white"
?indeterminate=${indeterminate}
label="Loading progess demo"
label="Loading progress demo"
></sp-progress-circle>
<sp-progress-circle
progress="53"
static="white"
size="l"
?indeterminate=${indeterminate}
label="Loading progess demo"
label="Loading progress demo"
Westbrook marked this conversation as resolved.
Show resolved Hide resolved
></sp-progress-circle>
</div>
`;
Expand Down
11 changes: 11 additions & 0 deletions packages/progress-circle/test/progress-circle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ describe('ProgressCircle', () => {

await expect(el).to.be.accessible();
});
it('accepts label from `slot`', async () => {
const el = await fixture<ProgressCircle>(html`
<sp-progress-circle role="progressbar">
Label From Slot
</sp-progress-circle>
`);

await elementUpdated(el);

expect(el.getAttribute('aria-label')).to.equal('Label From Slot');
});
it('accepts user `role`', async () => {
const el = await fixture<ProgressCircle>(html`
<sp-progress-circle
Expand Down
4 changes: 4 additions & 0 deletions tools/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
"development": "./src/get-deep-element-from-point.dev.js",
"default": "./src/get-deep-element-from-point.js"
},
"./src/get-label-from-slot.js": {
"development": "./src/get-label-from-slot.dev.js",
"default": "./src/get-label-from-slot.js"
},
"./src/index.js": {
"development": "./src/index.dev.js",
"default": "./src/index.js"
Expand Down