Skip to content

Commit

Permalink
Update link, dropdown-menu-nav, and popover to use a11y-label (#675)
Browse files Browse the repository at this point in the history
* chore: merge upstream

Squashed commit of the following:

* feat: update label to a11y-label

Added the a11y-label attribute to replace
the label attribute when needing to
update a components aria-label

* feat: update references from label to a11y-label

For components and tests that referenced the
label attribute this updates those to now use
the a11y-label attribute

* chore: add changeset

* fix: update a11y-label descriptions for each component
  • Loading branch information
sirrah-tam committed Jan 23, 2024
1 parent 5b8510e commit 88c580a
Show file tree
Hide file tree
Showing 12 changed files with 89 additions and 32 deletions.
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
4 changes: 2 additions & 2 deletions packages/pharos/src/components/popover/pharos-popover.test.ts
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

0 comments on commit 88c580a

Please sign in to comment.