zetkin / call.zetk.in

Caller interface for Zetkin.
4 stars 4 forks source link

Tooltip close button has incorrect role #274

Open henrycatalinismith opened 2 years ago

henrycatalinismith commented 2 years ago

Steps to reproduce

  1. Open an assignment.
  2. Press ⌘ + F5 to start VoiceOver.
  3. Focus the tooltip's I understand button.

Expected outcome

As this element doesn't navigate to another page, but rather changes the state of the UI on the current page, it should be announced as a button rather than a link.

Actual outcome

The element is announced as a link.

Screenshot of the above screen reader scenario for the benefit of sighted people unfamiliar with screen readers

Severity

The success criterion covering this issue is either 1.3.1 or 4.1.2, which are both WCAG level A – the minimum level. Most organizations aim for a conformance level of AA, which includes all level A criteria. This one is probably not going to get in the way of a determined user but as it's conformance level A it's still moderately severe.

Recommendation

Use <button> rather than <a> for this one.

henrycatalinismith commented 2 years ago

Got as far as making the following change to <FormattedLink /> over in zetkin-common and then shelved this for a bit.

diff --git a/misc/FormattedLink.jsx b/misc/FormattedLink.jsx
index b15f4e8..3010041 100644
@@ -21,7 +21,15 @@ export default class FormattedLink extends React.Component {
         let formatMessage = this.props.intl.formatMessage;
         let msg = formatMessage({ id }, this.props.msgValues);

-        if (this.props.forceRefresh || !href || href.indexOf('//') === 0
+        if (!href) {
+            return (
+                <button className={ this.props.className }
+                    onClick={ this.props.onClick }>
+                    { msg }
+                </button>
+            )
+        }
+        if (this.props.forceRefresh || href.indexOf('//') === 0

The above makes sense for sure in the context of these tooltip close buttons. It might not make so much sense if there are other <FormattedLink /> call sites that are passing empty href props and onClick callbacks that trigger navigation. Rather than dig deep into that I'm going to come back to this another time and see if it's better to strip out this <FormattedLink /> usage entirely and render a bare <button> element directly in <TutorialNote /> instead. Just noting down this dead-end in case it needs revisiting.