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

Update link, dropdown-menu-nav, and popover to use a11y-label #675

Merged
merged 12 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/thick-hornets-hammer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@ithaka/pharos': minor
---

Updating the use of a11y-label to replace label when needing to use aria-label
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { PharosElement } from '../base/pharos-element';
import { html } from 'lit';
import { property, queryAssignedElements } from 'lit/decorators.js';
import type { TemplateResult, CSSResultArray } from 'lit';
import type { TemplateResult, CSSResultArray, PropertyValues } from 'lit';
import { ifDefined } from 'lit/directives/if-defined.js';
import { dropdownMenuNavStyles } from './pharos-dropdown-menu-nav.css';
import type { PharosDropdownMenuNavLink } from './pharos-dropdown-menu-nav-link';
Expand All @@ -19,12 +19,20 @@ import FocusMixin from '../../utils/mixins/focus';
*/
export class PharosDropdownMenuNav extends FocusMixin(PharosElement) {
/**
* Indicates the aria label to apply to the nav.
* Indicates the aria-label to apply to the nav element.
* @attr label
* @deprecated
*/
@property({ type: String, reflect: true })
public label?: string;

/**
* Indicates the aria label to apply to the button.
* @attr a11y-label
*/
@property({ type: String, reflect: true, attribute: 'a11y-label' })
public a11yLabel?: string;

@queryAssignedElements({ selector: '[data-pharos-component="PharosDropdownMenuNavLink"]' })
private _allLinks!: NodeListOf<PharosDropdownMenuNavLink>;

Expand All @@ -39,6 +47,14 @@ export class PharosDropdownMenuNav extends FocusMixin(PharosElement) {
this.addEventListener('focus', () => this._closeAllMenus());
}

protected override update(changedProperties: PropertyValues): void {
super.update && super.update(changedProperties);

if (this.label) {
console.warn("The 'label' attribute is deprecated. Use 'a11y-label' instead.");
}
}

private _closeAllMenus(link: PharosDropdownMenuNavLink | undefined = undefined) {
const menu: PharosDropdownMenu | null = this.querySelector(
'[data-pharos-component="PharosDropdownMenu"][open]'
Expand Down Expand Up @@ -69,8 +85,10 @@ export class PharosDropdownMenuNav extends FocusMixin(PharosElement) {
}

protected override render(): TemplateResult {
// TODO: Remove in future release once sufficient time elapsed to update naming convention
const a11yLabel = this.a11yLabel ?? this.label;
return html`
<nav class="dropdown-menu-nav__container" aria-label=${ifDefined(this.label)}>
<nav class="dropdown-menu-nav__container" aria-label=${ifDefined(a11yLabel)}>
<slot @slotchange=${this._handleSlotChange}></slot>
</nav>
`;
Expand Down
12 changes: 6 additions & 6 deletions packages/pharos/src/components/footer/pharos-footer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ describe('pharos-footer', () => {
<test-pharos-link
href="https://twitter.com/JSTOR"
target="_blank"
label="twitter - This link opens in a new window"
a11y-label="twitter - This link opens in a new window"
>
<test-pharos-icon name="twitter" a11y-hidden="true"></test-pharos-icon>
</test-pharos-link>
Expand All @@ -187,7 +187,7 @@ describe('pharos-footer', () => {
<test-pharos-link
href="https://www.facebook.com/JSTOR.org"
target="_blank"
label="facebook - This link opens in a new window"
a11y-label="facebook - This link opens in a new window"
>
<test-pharos-icon name="facebook" a11y-hidden="true"></test-pharos-icon>
</test-pharos-link>
Expand All @@ -196,7 +196,7 @@ describe('pharos-footer', () => {
<test-pharos-link
href="https://www.instagram.com/jstor_org"
target="_blank"
label="instagram - This link opens in a new window"
a11y-label="instagram - This link opens in a new window"
>
<test-pharos-icon name="instagram" a11y-hidden="true"></test-pharos-icon>
</test-pharos-link>
Expand All @@ -205,7 +205,7 @@ describe('pharos-footer', () => {
<test-pharos-link
href="https://www.youtube.com/channel/UCQM-7sUBV6Z-PVas0S4k0lw"
target="_blank"
label="youtube - This link opens in a new window"
a11y-label="youtube - This link opens in a new window"
>
<test-pharos-icon name="youtube" a11y-hidden="true"></test-pharos-icon>
</test-pharos-link>
Expand All @@ -214,7 +214,7 @@ describe('pharos-footer', () => {
<test-pharos-link
href="https://www.linkedin.com/company/ithaka"
target="_blank"
label="linkedin - This link opens in a new window"
a11y-label="linkedin - This link opens in a new window"
>
<test-pharos-icon name="linkedin" a11y-hidden="true"></test-pharos-icon>
</test-pharos-link>
Expand All @@ -223,7 +223,7 @@ describe('pharos-footer', () => {
<test-pharos-link
href="https://jstor.tumblr.com"
target="_blank"
label="tumblr - This link opens in a new window"
a11y-label="tumblr - This link opens in a new window"
>
<test-pharos-icon name="tumblr" a11y-hidden="true"></test-pharos-icon>
</test-pharos-link>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ describe('pharos-image-card', () => {
const link = component.renderRoot.querySelector(
'[data-pharos-component="PharosLink"].card__link--image'
);
expect(link?.getAttribute('label')).to.equal('Label for card image link');
expect(link?.getAttribute('a11y-label')).to.equal('Label for card image link');
});

it('renders a label for the link around the image for the collection variant', async () => {
Expand All @@ -410,7 +410,7 @@ describe('pharos-image-card', () => {
const link = component.renderRoot.querySelector(
'[data-pharos-component="PharosLink"].card__link--collection'
);
expect(link?.getAttribute('label')).to.equal('Label for card image link');
expect(link?.getAttribute('a11y-label')).to.equal('Label for card image link');
});

it('renders a checkbox for the selectable variant', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ export class PharosImageCard extends ScopedRegistryMixin(FocusMixin(PharosElemen
[`card__link--selected`]: this._isSelected,
})}
href="${this.link}"
label="${ifDefined(this.imageLinkLabel)}"
a11y-label=${ifDefined(this.imageLinkLabel)}
subtle
flex
no-hover
Expand Down Expand Up @@ -311,7 +311,7 @@ export class PharosImageCard extends ScopedRegistryMixin(FocusMixin(PharosElemen
[`card__link--select-hover`]: this._isSelectableCardHover() && !this._isSelected,
})}
href="${this.link}"
label=${ifDefined(this.imageLinkLabel)}
a11y-label=${ifDefined(this.imageLinkLabel)}
subtle
no-hover
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const Base = {
href={args.href}
hreflang={args.hreflang}
indicateVisited={args.indicateVisited}
label={args.label}
a11yLabel={args.label}
noHover={args.noHover}
onBackground={args.onBackground}
ping={args.ping}
Expand Down
27 changes: 23 additions & 4 deletions packages/pharos/src/components/link/pharos-link.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { html, nothing } from 'lit';
import { property, state } from 'lit/decorators.js';
import type { TemplateResult, CSSResultArray } from 'lit';
import type { TemplateResult, CSSResultArray, PropertyValues } from 'lit';
import { ifDefined } from 'lit/directives/if-defined.js';
import { classMap } from 'lit/directives/class-map.js';
import { linkStyles } from './pharos-link.css';
Expand Down Expand Up @@ -43,12 +43,20 @@ export class PharosLink extends FocusMixin(AnchorElement) {
public onBackground = false;

/**
* Indicates the aria label to apply to the link.
* Indicates the aria-label to apply to the link.
* @attr label
* @deprecated
*/
@property({ type: String, reflect: true })
public label?: string;

/**
* Indicates the aria label to apply to the button.
* @attr a11y-label
*/
@property({ type: String, reflect: true, attribute: 'a11y-label' })
public a11yLabel?: string;

/**
* Indicates if the link should be bold.
* @attr bold
Expand Down Expand Up @@ -96,6 +104,14 @@ export class PharosLink extends FocusMixin(AnchorElement) {
@state()
private _hover = false;

protected override update(changedProperties: PropertyValues): void {
super.update && super.update(changedProperties);

if (this.label) {
console.warn("The 'label' attribute is deprecated. Use 'a11y-label' instead.");
}
}

public static override get styles(): CSSResultArray {
return [linkStyles];
}
Expand All @@ -113,6 +129,9 @@ export class PharosLink extends FocusMixin(AnchorElement) {
}

protected override render(): TemplateResult {
// TODO: Remove in future release once sufficient time elapsed to update naming convention
const a11yLabel = this.a11yLabel ?? this.label;

return this.href !== undefined
? html`<a
id="link-element"
Expand All @@ -127,7 +146,7 @@ export class PharosLink extends FocusMixin(AnchorElement) {
rel=${ifDefined(this.rel)}
target=${ifDefined(this.target)}
type=${ifDefined(this.type)}
aria-label=${ifDefined(this.label)}
aria-label=${ifDefined(a11yLabel)}
@click=${this._handleClick}
><slot></slot>${this.appendContent}</a
>`
Expand All @@ -137,7 +156,7 @@ export class PharosLink extends FocusMixin(AnchorElement) {
[`link--alert`]: this._alert,
[`link--hover`]: this._hover,
})}"
aria-label=${ifDefined(this.label)}
aria-label=${ifDefined(a11yLabel)}
>
<slot></slot>
${this.appendContent}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const Base = {
href=${ifDefined(args.href)}
hreflang=${ifDefined(args.hreflang)}
?indicate-visited=${args.indicateVisited}
label=${ifDefined(args.label)}
a11y-${ifDefined(args.label)}
?no-hover=${args.noHover}
?on-background=${args.onBackground}
ping=${ifDefined(args.ping)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const Base = {
<PharosButton id="my-button" data-popover-id="my-popover" icon-right="chevron-down">
Click Me
</PharosButton>
<PharosPopover id="my-popover" label="Pharos Popover">
<PharosPopover id="my-popover" a11yLabel="Pharos Popover">
<div style="padding: 1rem">Lorem ipsum dolor sit amet</div>
</PharosPopover>
</div>
Expand All @@ -39,7 +39,7 @@ export const Events = {
<PharosButton id="my-button" data-popover-id="my-popover" icon-right="chevron-down">
Click Me
</PharosButton>
<PharosPopover id="my-popover" label="Pharos Popover">
<PharosPopover id="my-popover" a11yLabel="Pharos Popover">
<div style="padding: 1rem; display: flex; flex-direction: column; gap: 1rem;">
<div style="padding: 1rem;">Lorem ipsum dolor sit amet</div>
<PharosButton
Expand All @@ -62,7 +62,7 @@ export const DarkPopover = {
<PharosButton id="my-button" data-popover-id="my-popover" icon-right="chevron-down">
Click Me
</PharosButton>
<PharosPopover id="my-popover" is-on-background label="Pharos Popover">
<PharosPopover id="my-popover" is-on-background a11yLabel="Pharos Popover">
<div style="background: #444444; color: white; padding: 1rem; display: flex; flex-direction: column; gap: 1rem;">
<div style="padding: 1rem">Lorem ipsum dolor sit amet</div>
<PharosButton
Expand All @@ -85,7 +85,7 @@ export const DarkPopoverOnBackground = {
<PharosButton id="my-button" data-popover-id="my-popover" icon-right="chevron-down">
Click Me
</PharosButton>
<PharosPopover id="my-popover" label="Pharos Popover">
<PharosPopover id="my-popover" a11yLabel="Pharos Popover">
<div style="background: #444444; color: white; padding: 1rem; display: flex; flex-direction: column; gap: 1rem;">
<div style="padding: 1rem">Lorem ipsum dolor sit amet</div>
<PharosButton
Expand All @@ -111,7 +111,7 @@ export const LargeContents = {
<PharosButton id="my-button" data-popover-id="my-popover" icon-right="chevron-down">
Click Me
</PharosButton>
<PharosPopover id="my-popover" label="Large Pharos Popover">
<PharosPopover id="my-popover" a11yLabel="Large Pharos Popover">
<div style="padding: 1rem; width: 300px; display: flex; flex-direction: column; gap: 1rem;">
<h2>Large Pharos Popover</h2>
<div style="height: 200px; overflow: auto; border: 1px solid black; padding: 1rem;">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('pharos-popover', () => {

beforeEach(async () => {
component = await fixture(html`
<test-pharos-popover id="my-popover" label="Test label for dialog">
<test-pharos-popover id="my-popover" a11y-label="Test label for dialog">
<div>I am popover contents</div>
</test-pharos-popover>
`);
Expand All @@ -25,7 +25,7 @@ describe('pharos-popover', () => {

const getSimplePopover = () => {
return html`
<test-pharos-popover id="my-popover" label="Test label for dialog">
<test-pharos-popover id="my-popover" a11y-label="Test label for dialog">
<div>I am popover contents</div>
</test-pharos-popover>
`;
Expand Down
17 changes: 16 additions & 1 deletion packages/pharos/src/components/popover/pharos-popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,18 @@ export class PharosPopover extends ScopedRegistryMixin(FocusMixin(OverlayElement
/**
* Indicates the aria label to apply to the dialog.
* @attr label
* @deprecated
*/
@property({ type: String, reflect: true })
public label?: string;

/**
* Indicates the aria-label to apply to the dialog.
* @attr a11y-label
*/
@property({ type: String, reflect: true, attribute: 'a11y-label' })
public a11yLabel?: string;

/**
* Indicates the aria label to apply to the dialog.
* @attr label
Expand Down Expand Up @@ -140,6 +148,10 @@ export class PharosPopover extends ScopedRegistryMixin(FocusMixin(OverlayElement
}

super.updated(changedProperties);

if (this.label) {
console.warn("The 'label' attribute is deprecated. Use 'a11y-label' instead.");
}
}

override connectedCallback(): void {
Expand Down Expand Up @@ -374,11 +386,14 @@ export class PharosPopover extends ScopedRegistryMixin(FocusMixin(OverlayElement
}

protected override render(): TemplateResult {
// TODO: Remove in future release once sufficient time elapsed to update naming convention
const a11yLabel = this.a11yLabel ?? this.label;

return html` <focus-trap>
<div
class="popover"
role="dialog"
aria-label=${ifDefined(this.label)}
aria-label=${ifDefined(a11yLabel)}
aria-labelledby="${ifDefined(this.labelledBy)}"
>
<slot></slot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const Base = {
>
Click Me
</storybook-pharos-button>
<storybook-pharos-popover id="my-popover" label="Pharos popover">
<storybook-pharos-popover id="my-popover" a11y-label="Pharos popover">
<div style="padding: 1rem;">Lorem ipsum dolor sit amet</div>
</storybook-pharos-popover>
</div>
Expand All @@ -42,7 +42,7 @@ export const Events = {
>
Click Me
</storybook-pharos-button>
<storybook-pharos-popover id="my-popover" label="Pharos popover">
<storybook-pharos-popover id="my-popover" a11y-label="Pharos popover">
<div style="padding: 1rem; display: flex; flex-direction: column; gap: 1rem;">
<span>Lorem ipsum dolor sit amet</span>
<storybook-pharos-button
Expand Down Expand Up @@ -70,7 +70,7 @@ export const DarkPopover = {
>
Click Me
</storybook-pharos-button>
<storybook-pharos-popover id="my-popover" is-on-background label="Pharos popover">
<storybook-pharos-popover id="my-popover" is-on-background a11y-label="Pharos popover">
<div
style="background: #444444; color: white; padding: 1rem; display: flex; flex-direction: column; gap: 1rem;"
>
Expand Down Expand Up @@ -102,7 +102,7 @@ export const DarkPopoverOnBackground = {
>
Click Me
</storybook-pharos-button>
<storybook-pharos-popover id="my-popover" label="Pharos popover">
<storybook-pharos-popover id="my-popover" a11y-label="Pharos popover">
<div
style="background: #444444; color: white; padding: 1rem; display: flex; flex-direction: column; gap: 1rem;"
>
Expand Down Expand Up @@ -136,7 +136,7 @@ export const LargeContents = {
>
Click Me
</storybook-pharos-button>
<storybook-pharos-popover id="my-popover" label="Large Pharos Popover">
<storybook-pharos-popover id="my-popover" a11y-label="Large Pharos Popover">
<div
style="padding: 1rem; width: 300px; display: flex; flex-direction: column; gap: 1rem;"
>
Expand Down
Loading