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

BrianJVarley · 2023-11-16T12:09:42Z

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

ViewDrd: Remove button outline style override, allow native browser focus style to be applied on focus
DrillDown: Change element type to - button to allow keyboard focus and capture ENTER key event to trigger drilldown.

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

CLAassistant · 2023-11-16T12:09:48Z

All committers have signed the CLA.

BrianJVarley · 2023-11-16T12:55:16Z

packages/dmn-js-drd/src/features/drill-down/DrillDown.js

@@ -71,7 +72,7 @@ export default class DrillDown {
  addOverlay(element, className) {
    const html = domify(`
      <div class="drill-down-overlay">
-        <span class="${className}"></span>
+        <button class="${className}"></button>


Since the span element doesn't capture ENTER key, I've changed this element to a Button which does capture it. Also removed the outline style override to allow native button outline to be displayed on button focus.

We still need to fix styling here. Also, if the drilldown is not enabled (e.g. for DRD-only viewer), this should remain to be a span as it's not interactive.

I added type=button because otherwise it's type=submit which is not correct.

We still need to fix styling here.

Yeah good point, about the styling added a comment here and a commit -> #802 (comment)

Also, if the drilldown is not enabled (e.g. for DRD-only viewer), this should remain to be a span as it's not interactive.

Is there some state or property we can check for "DRD-only viewer" mode? Then we could just add a conditional to render either a span or button within the addOverlay() function:

let html; if (isDrdOnlyMode) { html = domify(` <div class="drill-down-overlay"> <span type="button" class="${className}"></span> </div> `); } else { html = domify(` <div class="drill-down-overlay"> <button type="button" class="${className}"></button> </div> `); }

BrianJVarley · 2023-11-16T15:06:39Z

packages/dmn-js-drd/test/spec/features/drill-down/DrillDownSpec.js

+        // then
+        expect(drillSpy).to.have.been.calledOnce;
+
+        expect(drillSpy).to.have.been.calledWith(overlayEl);


Working on fixing this test, at the moment overlayEl does return the button element, but the drillSpy is not called in test. After the ENTER key event has been triggered.

I wasn't able to fix this test too. It looks like in Chrome the synthetic keyboard event for enter doesn't trigger "click" handler, but a trusted keyboard event does. So this won't work:

const event = new KeyboardEvent('keydown', { key: 'Enter', code: 'Enter', which: 13, keyCode: 13, }); mybutton.dispatchEvent(event);

We could add an event handler for Enter (and also Space), or just trust in browser behavior, and leave it untested.

Yeah seems fine to skip this test, as the browser will trigger the click event on a button element by default on ENTER key event.

barmac · 2023-11-28T08:44:22Z

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

barmac · 2023-11-28T10:37:49Z

packages/dmn-js-drd/src/features/drill-down/DrillDown.js

@@ -71,7 +71,7 @@ export default class DrillDown {
  addOverlay(element, className) {
    const html = domify(`
      <div class="drill-down-overlay">
-        <span class="${className}"></span>
+        <button type="button" class="${className}"></button>


The styling is not quite there yet. I'd rather keep the old look for now.

@barmac can you explain a bit more about the styling? I guess you mean that the previous blue background is missing on the new button element.

Before:

After:

I've pushed a new commit to match the styling that was there previously on the span.
What do you think? __34041ad

Without focus

With focus

What I meant is that if we switch to standard button look, we change the visuals significantly. On the other hand, we need some way to indicate focus.

@barmac I've pushed another commit so the focus style is more apparent on the drill down button.

Do you see some specific style difference between original and new style as described below?
If so I can try to adjust it further to match the expectation.

For comparison this is the original span style on focus, i.e, no focus focus style applied just span with blue background:

===

Whereas below, this is the latest commit blue button which IMO looks the same as original span style, but with the browser focus style being applied:

Before Focus (default style)

Focused (Dark blue focus style applied by browser)

Thanks, it looks better now. I will tweak it a bit and then merge this PR :)

Great, looking forward to the next release then :)

barmac · 2023-12-06T10:43:16Z

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 · 2023-12-06T15:58:04Z

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/

Screen.Recording.2023-12-06.at.16.57.36.mov

marstamm · 2023-12-07T08:44:20Z

[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

barmac · 2023-12-07T09:11:13Z

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 · 2023-12-07T12:50:40Z

Increase outline offset looks good on Firefox:

Screen.Recording.2023-12-07.at.13.49.20.mov

and in Chrome:

Screen.Recording.2023-12-07.at.13.50.16.mov

…wnComp: Apply tabindex=o attribute to catch keyboard focus & change element from span -> button to capture enter enter key event and drill down

…us, .view-drd-button: Remove outline: none style to allow browser native focus style to be applied to button

…ement

…@barmac

…@barmac

…r tab focus style is visible (.djs-overlay .drill-down-overlay)

…r tab focus style is visible (.djs-overlay .drill-down-overlay)..remove style prop

lmbateman · 2023-12-07T12:52:37Z

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

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.
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.
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 · 2023-12-07T13:20:18Z

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

BrianJVarley · 2023-12-11T10:00:44Z

[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

@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"

barmac · 2023-12-11T13:04:37Z

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}.

BrianJVarley mentioned this pull request Nov 16, 2023

Support DMN diagram accessibility #778

Closed

BrianJVarley commented Nov 16, 2023

View reviewed changes

barmac self-requested a review November 16, 2023 13:47

BrianJVarley commented Nov 16, 2023

View reviewed changes

BrianJVarley changed the title ~~Add tab focus and keyboard enter event to DrillDown and ViewDrd Buttons~~ Add tab focus & keyboard enter event to DrillDown & ViewDrd Nov 23, 2023

barmac reviewed Nov 28, 2023

View reviewed changes

BrianJVarley force-pushed the feat/view-drd-and-drilldown-focus branch from ac556c6 to e3d7e14 Compare December 4, 2023 10:25

BrianJVarley and others added 16 commits December 7, 2023 13:52

ViewDrdComp: Add tabIndex and include focus outline styling, DrillDo…

693530c

…wnComp: Apply tabindex=o attribute to catch keyboard focus & change element from span -> button to capture enter enter key event and drill down

DrillDown: remove tabIndex as overlay button will receive focus natively

fa70258

ViewDrdComp: remove tabindex attr. since button natively receives foc…

f349730

…us, .view-drd-button: Remove outline: none style to allow browser native focus style to be applied to button

view-drd-button: remove custom focus style, now provided by button el…

05e9dea

…ement

clean up unused code in PR

5ea5a0d

fix: add type=button

3d772d3

DrillDown: Add blue background & white color styling to button element …

146a9c9

…@barmac

drilldown: remove unused css class

53af083

dmn-js-drd.css: remove background color on drilldown button so Browse…

c41fde6

…r tab focus style is visible (.djs-overlay .drill-down-overlay)

dmn-js-drd.css: remove background color on drilldown button so Browse…

7ee24ff

…r tab focus style is visible (.djs-overlay .drill-down-overlay)..remove style prop

chore: improve colors

77c13cc

fix: show outline for view-drd button

d7522bb

feat: display span for non-interactive

990a8d2

test: remove exclusive test

8937133

test: revert unnecessary change

ecc4f26

increase outline offset

35100ba

barmac force-pushed the feat/view-drd-and-drilldown-focus branch from 16ad122 to 35100ba Compare December 7, 2023 12:52

lol

539ad76

barmac merged commit ab43a42 into bpmn-io:develop Dec 7, 2023
6 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

BrianJVarley commented Nov 16, 2023 •

edited

Loading

CLAassistant commented Nov 16, 2023 •

edited

Loading

BrianJVarley Nov 16, 2023 •

edited

Loading

barmac Nov 28, 2023

barmac Nov 28, 2023

BrianJVarley Nov 28, 2023 •

edited

Loading

BrianJVarley Nov 28, 2023 •

edited

Loading

BrianJVarley Nov 16, 2023 •

edited

Loading

barmac Nov 28, 2023

BrianJVarley Nov 28, 2023

barmac commented Nov 28, 2023

barmac Nov 28, 2023

BrianJVarley Nov 28, 2023

BrianJVarley Nov 28, 2023 •

edited

Loading

barmac Dec 1, 2023

BrianJVarley Dec 4, 2023 •

edited

Loading

barmac Dec 5, 2023

BrianJVarley Dec 5, 2023

barmac commented Dec 6, 2023

barmac commented Dec 6, 2023

marstamm commented Dec 7, 2023

barmac commented Dec 7, 2023

barmac commented Dec 7, 2023

lmbateman commented Dec 7, 2023

barmac commented Dec 7, 2023

BrianJVarley commented Dec 11, 2023

barmac commented Dec 11, 2023

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

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

Conversation

BrianJVarley commented Nov 16, 2023 • edited Loading

CLAassistant commented Nov 16, 2023 • edited Loading

BrianJVarley Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrianJVarley Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

BrianJVarley Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

BrianJVarley Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barmac commented Nov 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrianJVarley Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrianJVarley Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barmac commented Dec 6, 2023

barmac commented Dec 6, 2023

marstamm commented Dec 7, 2023

barmac commented Dec 7, 2023

barmac commented Dec 7, 2023

lmbateman commented Dec 7, 2023

barmac commented Dec 7, 2023

BrianJVarley commented Dec 11, 2023

barmac commented Dec 11, 2023

BrianJVarley commented Nov 16, 2023 •

edited

Loading

CLAassistant commented Nov 16, 2023 •

edited

Loading

BrianJVarley Nov 16, 2023 •

edited

Loading

BrianJVarley Nov 28, 2023 •

edited

Loading

BrianJVarley Nov 28, 2023 •

edited

Loading

BrianJVarley Nov 16, 2023 •

edited

Loading

BrianJVarley Nov 28, 2023 •

edited

Loading

BrianJVarley Dec 4, 2023 •

edited

Loading