654: Announce Number of Filters & 627: Keep Keyboard Focus on Filter Button

[jgrimes86]

This PR addresses issues #654 and #627

For issue 654:
Two screen-reader-only spans have been added to the filter button label so that a screen reader can announce the filter button with the number of filters currently applied. When no filters are applied it announces 'Filter button'. When 2 filters are applied, it announces 'Filter button 2 filters applied'. To allow the screen reader to read the label in that order, the aria-label property was removed and the default 'Button' announcement is overridden by adding aria-roledescription=' '

For issue 627:
The 'if' statement has been removed from the filtuer button's onPress function so that the focus stays on the filter button when filter view is toggled off.

To test:

• go to the Find Properties page
• using a screen reader, TAB to the Filter button, which will be announced as 'filter button'
• hit SPACE or ENTER to enter the filter view
• select one or more filter categories
• SHIFT+TAB back to the filter button, which will be announced as 'filter button [#] filters applied current' (Note: the word 'current' included in the announcement is the subject of issue 625)
• test focus with [screen reader key]+TAB, which will announce 'filter button [#] filters applied focused current'
• hit SPACE or ENTER to exit filter view
• test focus again with [screen reader key]+TAB, which will again announce 'filter button [#] filters applied focused current

Note: If one of the active filters is a panel in the 'Get Access' section and you tab back into the filter view, the panel will not appear selected, although though the filter will still be active. This is a bug being addressed by issue 685

[vercel]

@jgrimes86 is attempting to deploy a commit to the Clean and Green Philly Team on Vercel.

A member of the Team first needs to authorize it.

[jgrimes86]

@bacitracin or @marvieqa I think you are the appropriate code approvers to review this PR?

[jgrimes86]

I think this PR also addresses issue #626, which appears to be a duplicate if issue #654 (or vice versa)

Issue 626 suggests using <aria-label=" Filter, 3 Filters applied">. I abandoned the existing aria-label before I realized that I could mute the default 'button' announcement by including aria-roledescription=' '.

I could try to revise this PR to use a variable for the aria-label instead of using the screen-reader-only elements that I've added. In hindsight, that seems like better practice, but I'll wait for input from the PR review before I re-work things.

[bacitracin]

@bacitracin or @marvieqa I think you are the appropriate code approvers to review this PR?

@jgrimes86 Thanks for the heads up. In the future you can feel free to tag me under Reviewers for a11y-related PRs.

[jgrimes86]

@bacitracin I made a commit to this PR to re-add the aria-label and get rid of the screen-reader only components. Looking through previous PRs, I saw that one of them (number 608) had added the aria-label so that the screen reader would correctly announce the button on small screens, so me deleting it re-introduced an issue that was previously fixed.

I figured whatever other changes or corrections are needed, that issue was yet another reason to put the aria-label back so I might as well take care of it now.

[bacitracin]

Keeping the semantic meaning of the button is more important than matching the suggested text order, so you can remove this line 'aria-roledescription=' '. Also aria-roledescription should never be an empty string, it should be a semantic role that you apply to divs or other HTML elements that have no existing role.

For the aria-label you usually don't want to include words like "button" or "image." Ideally that will come from the component being coded semantically (a real button element instead of a div for ex). So once you remove that aria-roledescription line, you can also remove "button" from both of the labels.

[bacitracin]

Here is what it sounds like on VoiceOver/Chrome
no filters selected

2 filters selected

[bacitracin]

@jgrimes86 Thanks for adding the aria-label back in. I had a small cleanup comment. You don't need the "button" text or the "aria-roledescription." Other than that, looks good! You can tag me again when it's updated and I'll approve/merge.

[jgrimes86]

@bacitracin I made those changes. Thank you for the explanations in the code comments.

[bacitracin]

👍 Looks good!