cleanup: Remove detour-route-selection and route-ladder-header-update test groups

[joshlarson]

Asana Ticket: https://app.asana.com/0/1205385723132845/1207483512351237/f

[github-actions]

Coverage of commit 5f54a0a

Summary coverage rate:
  lines......: 93.6% (3308 of 3536 lines)
  functions..: 73.6% (1364 of 1854 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

[github-actions]

Coverage of commit 5f54a0a

Summary coverage rate:
  lines......: 93.6% (3309 of 3536 lines)
  functions..: 73.6% (1365 of 1854 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

[hannahpurcell]

What do you think about renaming NewHeader to Header in this PR? I think it should be renamed with this work, but understand if you'd want it in a new PR

[hannahpurcell]

Oooh additionally, beyond just changing the component name, the "new" label on the classnames will need changing. In hindsight, this should have been done in my work! I should have used the term "oldHeader" instead of going with "newHeader"

[joshlarson]

I created a follow-up subtask for this in order to keep the diff for this smaller and easier to review. I could lump them together if you'd prefer.

[hannahpurcell]

Could you teach me why the aria-label needed moving?

[joshlarson]

This test uses a selector that tries to find the route alert icon using byRole("generic", { name: "Route alert" }). For reasons I don't fully understand, having the aria-label on the bootstrap icon doesn't actually put the aria-label onto the rendered component, but putting it on the wrapping div does.

I think this means that the new route ladder header route alert icon isn't doing accessibility quite right.

(FWIW - on main right now, the test is testing the old route ladder header, because the test group defaults to not being enabled, so the new route ladder header wasn't being tested for accessibility or an alert icon)

[firestack]

For reasons I don't fully understand, having the aria-label on the bootstrap icon doesn't actually put the aria-label onto the rendered component, but putting it on the wrapping div does.

You could try setting aria-hidden={false}

[firestack]

But also, as I understand it, aria-label should be used as a last resort. This wasn't known when this was originally implemented, but a better way to do this would be to add the label text as a visually hidden DOM node and linking it to the icon with aria-labelledby

[firestack]

We could attempt to make things smarter, where if aria-label or aria-labelledby is provided, we could not set the default aria-hidden value, but I'm not sure it's worth it.

[joshlarson]

but a better way to do this would be to add the label text as a visually hidden DOM node and linking it to the icon with aria-labelledby

Want to either:

• pair on this?
• ...address as a follow-up?

[firestack]

Want to either:

* pair on this?

* ...address as a follow-up?

I'm good either way! Since it was old code, I'm also fine with how it is for now.

[joshlarson]

I tried aria-hidden={false} and it still said Unable to find an accessible element with the role "generic" and name "Route Alert"

I also tried aria-labelledby, but that gave this error:

Expected element to have accessible description:
  /Active Detour/i
Received:

PR (into this branch for now) below

• fix: Use aria-labelledby for route alert icon on route ladder headers #2703

[hannahpurcell]

Great, thanks for writing that follow-up sub-task for renaming the header. As for the aria-label question, I feel the same as Kayla, that it can be left as-is for now, and addressed in the future if/when needed. But it's above and beyond what you need to cover here!