getAllByText is a code smell

TL;DR

Using getAllByText and getAllByLabelText can be a code smell, hinting at a lack of clarity or specificity in your UI.
Multiple elements with the same text can negatively impact the experience for screen reader users.
Leveraging ARIA labels effectively can provide better context for screen readers, boosting accessibility.

Introduction

In web development, code smells are patterns that may suggest there's an underlying problem. In this blog post, we'll look at how using getAllByText and getAllByLabelText can be seen as a code smell and how it can harm the experience of screen reader users. We'll also talk about using ARIA labels effectively to help fix this issue.

The Problem with getAllBy*

const changeLinks = screen.getAllByText('Change')
expect(changeLinks[0].href).toBe('/edit/name')
expect(changeLinks[1].href).toBe('/edit/email')
expect(changeLinks[2].href).toBe('/edit/address')

It's pretty common for multiple elements in a web app to share the same text. For example, you might have several links with the label "Change" that do different things, like changing a user's email, address, or password.

Screen reader users can find this tricky. If there are multiple elements with the same text, screen readers might not be able to tell them apart, causing confusion and potentially making your app less accessible. When using functions like getAllByText and getAllByLabelText in our testing suites, we might inadvertently enforce these bad habits.

This approach is not effective in ensuring a good experience for screen readers. It also requires the links to be ordered in a specific way.

The Solution: Leveraging ARIA Labels

One way to improve accessibility is by using ARIA labels effectively. ARIA labels provide additional context to screen reader users, helping them to understand what different elements on the page do. By adding ARIA labels, we can provide more clarity and specificity in our UI, reducing the likelihood of confusion for screen reader users.

<button aria-label="Change email address" onClick={() => handleEdit('email')}>
  Change
</button>
<button aria-label="Change address" onClick={() => handleEdit('address')}>
  Change
</button>

By using ARIA labels in this way, we provide screen reader users with the information they need to easily distinguish between elements with the same text. This makes for a better and more accessible user experience overall.

expect(screen.getByLabelText('Change name').href).toBe('/edit/name')
expect(screen.getByLabelText('Change email address').href).toBe('/edit/email')
expect(screen.getByLabelText('Change address').href).toBe('/edit/address')

When is `getAllBy*` ok?

Checking the length of the getAllBy* result is fine and often wise. Lets take this example:

<Table
    rows={[
        <>
            <div>{email}</div>
            <a aria-label="Change email address" href="/edit/email">
              Change
            </a>
        </>,
        <>
            <div>{address}</div>
            <a aria-label="Change address" href="/edit/address">
                Change
            </a>
        </>
    ]}
/>

We want a test to make sure each table row has a change link to go along with its value.

test('each row has a change link', () => {
    expect(screen.getByLabelText('Change name').href).toBe('/edit/name')
    expect(screen.getByLabelText('Change email address').href).toBe('/edit/email')
})

But a few weeks later, another dev comes along and adds a new row.

<Table
    rows={[
        <>
            <div>{email}</div>
            <a aria-label="Change email address" href="/edit/email">
              Change
            </a>
        </>,
        <>
            <div>{address}</div>
            <a aria-label="Change address" href="/edit/address">
                Change
            </a>
        </>,
        <>
            <div>{password}</div>
            <a aria-label="Change password" href="/edit/address">
                Change
            </a>
        </>
    ]}
/>

Notice we have a bug, the new Change link href for password is wrong. The developer was in a hurry that day so he didn’t update any tests, just saw that they passed and merged the changes.

To combat this, we can be stricter in our tests by ensuring a certain length of change links so that when any are added or removed, we have an assertion to fall back on.

test('each row has a change link', () => {
    expect(screen.getAllByText('Change').length).toBe(2)
    expect(screen.getByLabelText('Change name').href).toBe('/edit/name')
    expect(screen.getByLabelText('Change email address').href).toBe('/edit/email')
})

If the test had this additional check, it would have failed when the new row was added. The developer would be prompted to update that assertion, and hopefully see the other assertions they can add too giving them another chance to catch the bug.

test('each row has a change link', () => {
    expect(screen.getAllByText('Change').length).toBe(3)
    expect(screen.getByLabelText('Change name').href).toBe('/edit/name')
    expect(screen.getByLabelText('Change email address').href).toBe('/edit/email')
    expect(screen.getByLabelText('Change password').href).toBe('/edit/password')
})