Skip to content

Commit

Permalink
Update link, dropdown-menu-nav, and popover to use a11y-label - Break…
Browse files Browse the repository at this point in the history
…ing (#676)

* feat: add & update a11y-label, remove label

* feat: update remaining components to use a11y-label

* chore: add changeset

* fix: update a11yLabel references

* fix: update a11y-label descriptions for each component

* fix(popover): clean up the aria-labelledby attribute
  • Loading branch information
sirrah-tam committed Jan 23, 2024
1 parent 145da90 commit 536a598
Show file tree
Hide file tree
Showing 16 changed files with 62 additions and 57 deletions.
5 changes: 5 additions & 0 deletions .changeset/long-hotels-allow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@ithaka/pharos': major
---

Update all use of label attribute to a11y-label
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ import FocusMixin from '../../utils/mixins/focus';
*/
export class PharosDropdownMenuNav extends FocusMixin(PharosElement) {
/**
* Indicates the aria label to apply to the nav.
* @attr label
* Indicates the aria-label to apply to the nav element.
* @attr a11y-label
*/
@property({ type: String, reflect: true })
public label?: string;
@property({ type: String, reflect: true, attribute: 'a11y-label' })
public a11yLabel?: string;

@queryAssignedElements({ selector: '[data-pharos-component="PharosDropdownMenuNavLink"]' })
private _allLinks!: NodeListOf<PharosDropdownMenuNavLink>;
Expand Down Expand Up @@ -70,7 +70,7 @@ export class PharosDropdownMenuNav extends FocusMixin(PharosElement) {

protected override render(): TemplateResult {
return html`
<nav class="dropdown-menu-nav__container" aria-label=${ifDefined(this.label)}>
<nav class="dropdown-menu-nav__container" aria-label=${ifDefined(this.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 @@ -194,7 +194,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 @@ -203,7 +203,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 @@ -212,7 +212,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 @@ -221,7 +221,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 @@ -230,7 +230,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 @@ -239,7 +239,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}
isOnBackground={args.isOnBackground}
ping={args.ping}
Expand Down
12 changes: 6 additions & 6 deletions packages/pharos/src/components/link/pharos-link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ export class PharosLink extends FocusMixin(AnchorElement) {
public isOnBackground = false;

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

/**
* Indicates if the link should be bold.
Expand Down Expand Up @@ -127,7 +127,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(this.a11yLabel)}
@click=${this._handleClick}
><slot></slot>${this.appendContent}</a
>`
Expand All @@ -137,7 +137,7 @@ export class PharosLink extends FocusMixin(AnchorElement) {
[`link--alert`]: this._alert,
[`link--hover`]: this._hover,
})}"
aria-label=${ifDefined(this.label)}
aria-label=${ifDefined(this.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-label=${ifDefined(args.label)}
?no-hover=${args.noHover}
?is-on-background=${args.isOnBackground}
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
14 changes: 7 additions & 7 deletions packages/pharos/src/components/popover/pharos-popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ export class PharosPopover extends ScopedRegistryMixin(FocusMixin(OverlayElement
public isOnBackground = false;

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

/**
* Indicates the aria label to apply to the dialog.
* @attr label
* Indicates the aria-labelledby to apply to the dialog.
* @attr labelledBy
*/
@property({ type: String, reflect: true, attribute: 'labelled-by' })
public labelledBy?: string;
Expand Down Expand Up @@ -378,7 +378,7 @@ export class PharosPopover extends ScopedRegistryMixin(FocusMixin(OverlayElement
<div
class="popover"
role="dialog"
aria-label=${ifDefined(this.label)}
aria-label=${ifDefined(this.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
12 changes: 6 additions & 6 deletions packages/pharos/src/pages/shared/react/Footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export const Footer: FC = () => {
id="twitter-link"
href="https://twitter.com/JSTOR"
target="_blank"
label="twitter - This link opens in a new window"
a11yLabel="twitter - This link opens in a new window"
>
<PharosIcon name="twitter" a11yHidden="true"></PharosIcon>
</PharosLink>
Expand All @@ -252,7 +252,7 @@ export const Footer: FC = () => {
id="facebook-link"
href="https://www.facebook.com/JSTOR.org"
target="_blank"
label="facebook - This link opens in a new window"
a11yLabel="facebook - This link opens in a new window"
>
<PharosIcon name="facebook" a11yHidden="true"></PharosIcon>
</PharosLink>
Expand All @@ -262,7 +262,7 @@ export const Footer: FC = () => {
id="instagram-link"
href="https://www.instagram.com/jstor_org"
target="_blank"
label="instagram - This link opens in a new window"
a11yLabel="instagram - This link opens in a new window"
>
<PharosIcon name="instagram" a11yHidden="true"></PharosIcon>
</PharosLink>
Expand All @@ -272,7 +272,7 @@ export const Footer: FC = () => {
id="youtube-link"
href="https://www.youtube.com/channel/UCQM-7sUBV6Z-PVas0S4k0lw"
target="_blank"
label="youtube - This link opens in a new window"
a11yLabel="youtube - This link opens in a new window"
>
<PharosIcon name="youtube" a11yHidden="true"></PharosIcon>
</PharosLink>
Expand All @@ -282,7 +282,7 @@ export const Footer: FC = () => {
id="linkedin-link"
href="https://www.linkedin.com/company/ithaka"
target="_blank"
label="linkedin - This link opens in a new window"
a11yLabel="linkedin - This link opens in a new window"
>
<PharosIcon name="linkedin" a11yHidden="true"></PharosIcon>
</PharosLink>
Expand All @@ -292,7 +292,7 @@ export const Footer: FC = () => {
id="tumblr-link"
href="https://jstor.tumblr.com"
target="_blank"
label="tumblr - This link opens in a new window"
a11yLabel="tumblr - This link opens in a new window"
>
<PharosIcon name="tumblr" a11yHidden="true"></PharosIcon>
</PharosLink>
Expand Down
4 changes: 2 additions & 2 deletions packages/pharos/src/pages/shared/react/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { PharosIcon } from '../../../react-components/icon/pharos-icon';
import logo from '@config/assets/images/jstor-logo.svg';

const accountNav = (section: string) => (
<PharosDropdownMenuNav label="profile">
<PharosDropdownMenuNav a11yLabel="profile">
<PharosDropdownMenuNavLink
href="/account/profile"
id={`profile-link-${section}`}
Expand Down Expand Up @@ -107,7 +107,7 @@ export const Header: FC = () => (
display: 'flex',
}}
>
<PharosDropdownMenuNav label="main navigation">
<PharosDropdownMenuNav a11yLabel="main navigation">
<PharosDropdownMenuNavLink
href="/action/showAdvancedSearch"
id="adv-search-menu-link"
Expand Down
2 changes: 1 addition & 1 deletion packages/pharos/src/pages/shared/react/HeaderRevised.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const HeaderRevised: FC<Header> = ({ showSearch = false }) => (
a11yLabel="search"
></PharosButton>
</PharosInputGroup>
<PharosDropdownMenuNav label="main navigation">
<PharosDropdownMenuNav a11yLabel="main navigation">
<PharosDropdownMenuNavLink
href="/action/showAdvancedSearch"
id="adv-search-menu-link"
Expand Down

0 comments on commit 536a598

Please sign in to comment.