bpmn-io / dmn-js

View and edit DMN diagrams in the browser.
https://bpmn.io/toolkit/dmn-js/
Other
289 stars 137 forks source link

Add tab focus & keyboard enter event to DrillDown & ViewDrd #802

Closed BrianJVarley closed 9 months ago

BrianJVarley commented 9 months ago

🔎 a11y changes to improve accessibility of the expand and minimise Decision Table diagram

Closes #778 (💡 Discussion in progress & tests needed)

CLAassistant commented 9 months ago

CLA assistant check
All committers have signed the CLA.

barmac commented 9 months ago

Hi, thanks for opening this PR and sorry that you had to wait for so long. I will look into this today.

barmac commented 9 months ago

I just noticed that we set the icon DOM element to be a button even if it's not clickable. I will fix that.

barmac commented 9 months ago

OK I played a bit with CSS, and also made sure that we display span and not button if the drilldown icon is not interactive. Before we merge this, I'd love to gather feedback from the bpmn.io team.

I built a demo website which can be accessed at https://dmn-js-a11y-demo.netlify.app/

https://github.com/bpmn-io/dmn-js/assets/28307541/f1fa3ea9-a40f-45c4-83b8-8d6e3fad52bf

marstamm commented 9 months ago

[X-Posting from Slack for transparency]

The Focus has little contrast on the DRD, I think. For the first iteration, this is absolutely fine (still better than what we had before), but we could consider changing it in a follow up

I had a look how Github does it for dark buttons, and they add a white border around the blue outline. Not sure if this is something we want to do image (5) image (6)

barmac commented 9 months ago

I tested this additionally on Firefox, and the focus there is even harder to notice.

What we need to remember is that this PR is just a first step. We can do much more for the accessibility of the tools. For example, the a11y tools don't announce the element which is focused. However, I think it's still better to have some improvement than no-solution as it is without this contribution.

barmac commented 9 months ago

Increase outline offset looks good on Firefox:

https://github.com/bpmn-io/dmn-js/assets/28307541/b0ffa88b-2c52-4bfd-8c37-77df705b1fba

and in Chrome:

https://github.com/bpmn-io/dmn-js/assets/28307541/557d4ec2-0fba-4a75-be44-298a0cd93428

lmbateman commented 9 months ago

Thank you for raising the accessibility issue, @marstamm . I think there are three ways we could address it:

  1. If we have control over the focus color, we could darken it. Then it would contrast well with both the white background and the button color.
  2. We could also try lightening the button color instead, just on focus. But if we take this route, I think we need to consider it more holistically.
  3. Add an outline-offset to the focus as Martin suggested. This might be difficult because of the lack of space and the small size of the button, but I think reducing the button background area by a pixel on each side would work from a visual perspective (not sure if it's feasible from a technical perspective).

Otherwise, this looks good to me.

barmac commented 9 months ago

Thanks @BrianJVarley for initiating this and everybody else for feedback! :)

BrianJVarley commented 9 months ago

[X-Posting from Slack for transparency]

The Focus has little contrast on the DRD, I think. For the first iteration, this is absolutely fine (still better than what we had before), but we could consider changing it in a follow up

I had a look how Github does it for dark buttons, and they add a white border around the blue outline. Not sure if this is something we want to do image (5) image (6)

@marstamm @barmac I've noticed also using screen reader that the view / hide DRD buttons don't have an aria-label. I think it would improve the accesibility to label these buttons also for example "Show DRD" / "Hide Diagram"

Screenshot 2023-12-11 at 10 57 53
barmac commented 9 months ago

Thanks for the feedback. Indeed, right now the user does not hear what the button action is. This could be something like View decision table/literal expression of {name}.