Focus indicators consistent #4728

abdul99ahad · 2024-11-26T09:21:58Z

Proposed Changes

Tab action buttons

Earlier: no focus
Now:

Checkbox

Earlier:

Now:

Buttons

Changes:

Bottom panel tab bar

Earlier: no focus
Changes:
(changes required)

Bottom panel icons

Earlier: no focus
Changes:

Bottom panel focus alignment issue

Earlier:
Changes:

Overlay buttons

Earlier:

Changes:

Footer icons

Earlier: grey background

Changes:

Variable outline tab

Earlier: no focus
Changes:

Closes #4632

Depends on bpmn-io/variable-outline#6
Depends on bpmn-io/properties-panel#390

Checklist

To ensure you provided everything we need to look at your PR:

Brief textual description of the changes present
Visual demo attached
Steps to try out present, i.e. using the @bpmn-io/sr tool
Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

github-actions · 2024-11-26T09:26:26Z

This Pull Request targets develop branch, but contains fix commits.

Consider targeting main instead.

nikku · 2024-11-26T10:52:18Z

client/src/app/panel/tabs/variable-outline/VariableOutlineTab.less

@@ -24,6 +24,18 @@
      font-size: 14px !important;
    }
  }
+
+  .bio-vo-text-button {


Is this about the specific variable outline or a general Carbon customization?

I don't think this is general carbon customization. Nevertheless, we have to see in variable-outline project.

nikku · 2024-11-26T10:52:42Z

client/src/app/resizable-container/PropertiesPanelContainer.less

@@ -21,4 +21,15 @@
    --font-family-monospace: inherit;
    background: var(--color-white);
  }
+
+  .bio-properties-panel-input[type="checkbox"]:focus {


Should this be a general properties panel fix?

The fix is implemented in bpmn-io/properties-panel#390

nikku

Good progress, some early comments:

Cannot be focused yet:
I see two versions of the outline (thin, and thick), do we plan to align this?
I see two versions of outline styles now for buttons, do we plan to align this?
Collapsed properties panel is still part of focusable (tabbable) application; we should detach it once collapsed:

Generally I'd suggest we fix the outlines in the relevant libraries, where they are broken, and don't monkey patch in modeler. Applies to properties panel and variable outline.

abdul99ahad · 2024-11-26T11:45:15Z

Improved the tab bar icon focus. The styles are consistent and I think the difference is very minor if you look closely. Do we invest more time to align this more perfectly or is it good to have?

cc: @nikku @lmbateman

abdul99ahad · 2024-11-26T15:55:56Z

Improved the tab bar icon focus. The styles are consistent and I think the difference is very minor if you look closely. Do we invest more time to align this more perfectly or is it good to have?
cc: @nikku @lmbateman

I think it's much better now.

abdul99ahad · 2024-11-27T09:55:39Z

Summary of the changes:

Tabs and tab action buttons have focus
Collapsed panel elements won't get focus
Bottom bar have same consistent focus states
Adjusted focus states offset for certain elements (buttons, containers, etc.,)
Properties panel focus states are fixed here
Variable outline focus states are fixed here

Figma designs can be found here

lmbateman · 2024-11-29T13:10:30Z

Collapsed properties panel is still part of focusable (tabbable) application; we should detach it once collapsed (Nico, above)
resolved collapsed panel to be not part of focus anymore (Abdul, in Slack)

I'm a little confused. Does this mean that

when the Properties panel is collapsed, it completely drops out of the tab sequence, or
when the Properties panel is collapsed, focus lands once on the panel as a whole, but not the items inside the panel?

ETA: The second one is great. I think the first one would make it impossible to open the Properties panel with the Tab and Return keys.

nikku · 2024-11-29T14:53:29Z

when the Properties panel is collapsed, focus lands once on the panel as a whole, but not the items inside the panel?

This, "the second one", is what I'd like to see:

You can focus the "expand trigger", to expand the properties panel (using keyboard tabbing)
But, if the panel is closed, then you're not able to focus items within it.

abdul99ahad · 2024-12-03T15:52:34Z

You can focus the "expand trigger", to expand the properties panel (using keyboard tabbing)

Currently, the "expand trigger" doesn't support any keyboard event but only mouse event. The control to open and close the panel is supported by its parent element (resizer).

<div
      className={ classNames('resizer', `resizer-${ direction }`) }
      onMouseDown={ onMouseDown }
    >
      {
        isHorizontal(direction)
          ? <HandleBarX tabIndex="0" className="handlebar" />
          : <HandleBarY tabIndex="0" className="handlebar" />
      }

Even if we modify it to support keyboard events, we would still need to delegate control to the HandleBar. Do you suggest addressing this within the scope of this issue, or do you have an alternative approach in mind?

This issue is also arising in switching modeler tabs using Keyboard (since they are divs and requires separate keydown event)

But, if the panel is closed, then you're not able to focus items within it.

Yes you won't be able to focus the elements within the panel if it's collapsed.

nikku · 2024-12-06T08:34:20Z

@abdul99ahad reviewed + merged both upstream PRs:

Please go ahead and incorporate them.

nikku · 2024-12-06T08:34:54Z

resources/plugins/test/style.css

@@ -1,7 +1,3 @@
-.bjs-powered-by {


Why this change?

The bpmn.io watermark was removed in this commit. However, this block forces the watermark to remain visible in the modeler and allows it to gain focus when navigating via keyboard tabbing.

Yea. I assume this is a test plug-in specific override, isn't it? Happy to remove it, just flagging that one of the purposes of this plug-in is to override the core styles, bringing back the logo.

lmbateman · 2024-12-11T22:38:32Z

In general, this is a huge improvement! Thanks for all your hard work.

I turned up a handful of additional issues, including a couple I missed on the first round (sorry). I'm not sure if it makes sense to fix these as part of this issue, or to open another issue. What's the standard practice? Also, I think a couple of these are judgment calls, so I'd be interested in hearing any thoughts from the team.

What does the focus indicator on the tab do? Maybe we should only have focus indicators on the tab Close button (unless this has to do with keyboard binding).
I think we missed the focus indicators on the + and ... dropdown menus. The menu items still only have gray backgrounds.
There's an extra tab stop between ... and the General section's accordion button. Is this focusing the Properties panel?
I'm willing to discuss this one, but I was surprised when I pressed Return on a row in the Problems panel and the focus shifted to the erroneous field in the Properties panel. I guess this boils down to two questions:
- How easy is it to learn this behavior? (I would say fairly easy; after I've done it once or twice, I'll probably remember.)
- How beneficial or annoying is this behavior?
The tooltip icon, Elements header, and Variables header in the Variables panel still doesn't get focus. I'm OK either way -- the user will probably only want to read them once -- but we should be consistent with the Properties panel, where headers with tooltips do currently get focus.
I noticed the table and the tree in the Variables panel behave differently; when focus lands on the tree, it lands on the root node, and then you can use the arrow keys to navigate through the tree. In the table, focus lands on every cell. This might get annoying when the table is large. https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/data-grids/ has examples of tables that are initially accessed through the Tab key but then use arrow keys for internal navigation. (To be clear, I think it's OK for focus to move through the table headers, land on the first cell, and then land on the XML button.)
When I pressed Return on the XML button, it showed me the XML, with focus remaining on the button; and then when I pressed Return again, it showed the diagram, with the focus on the first interactive item in the Properties panel of the selected element. I think focus should remain on the XML button in both cases.
It might be my eyes, but could you please check the colors on the three elements in the Camunda version dialog? It seems like the focus indicator on the dropdown field has a lighter blue than the ones on the link and the Apply button.
I missed the Remember credentials toggle in the Deploy dialog; sorry! It doesn't have a visible indicator.
When the Deploy dialog opens, focus is on the Cluster endpoint field. Does this make sense from a developer's perspective?
When I switch the target to Camunda 8 SaaS, focus remains in the radio button group. However, when I switch it to Camunda 8 self-managed, focus moves to the first text field. We should be consistent. (I would leave it in the radio button field.)
Pressing Return on the Problems list or the Variables button in the status bar toggles the bottom panel. I think this is OK.

nikku · 2024-12-12T06:47:07Z

What's the standard practice?

To follow-up on things we found as part of this initiative, or clearly capture them in issues , or agree the are won't fix.

What does the focus indicator on the tab do? Maybe we should only have focus indicators on the tab Close button (unless this has to do with keyboard binding).

Tab can potentially be moved, it is a selectable thing, and hence I'd keep it focusable.

I think we missed the focus indicators on the + and ... dropdown menus. The menu items still only have gray backgrounds.

Agreed.

There's an extra tab stop between ... and the General section's accordion button. Is this focusing the Properties panel?

It is focusing the canvas, cf. bpmn-io/diagram-js#662, but we don't indicate the focus to the user (we should).

I'm willing to discuss this one, but I was surprised when I pressed Return on a row in the Problems panel and the focus shifted to the erroneous field in the Properties panel. I guess this boils down to two questions:

What do you expect instead? Return or SPACE does always (unless you are in a form) trigger the link action (or click behavior). Which is to navigate to the thing. What we could do instead:

When focusing via keyboard, show a special "navigate to problem in problems panel" link. This would only be shown when keyboard focus is on the element.

I noticed the table and the tree in the Variables panel behave differently; when focus lands on the tree, it lands on the root node, and then you can use the arrow keys to navigate through the tree. In the table, focus lands on every cell. This might get annoying when the table is large. https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/data-grids/ has examples of tables that are initially accessed through the Tab key but then use arrow keys for internal navigation. (To be clear, I think it's OK for focus to move through the table headers, land on the first cell, and then land on the XML button.)

This is mandated by Carbon, unless I'm mistaken. I'd not invest in working around the Carbon behaviors.

When I pressed Return on the XML button, it showed me the XML, with focus remaining on the button; and then when I pressed Return again, it showed the diagram, with the focus on the first interactive item in the Properties panel of the selected element. I think focus should remain on the XML button in both cases.

Likely not an easy fix, let's create a clearly reproducible a11y tagged follow-up issue.

When the Deploy dialog opens, focus is on the Cluster endpoint field. Does this make sense from a developer's perspective?

Only if it is empty, I think, otherwise the canoncal action is to "just press enter" to confirm config and deploy.

However I'd not invest there right now. We're planning to rebuild this anyway shortly.

Pressing Return on the Problems list or the Variables button in the status bar toggles the bottom panel. I think this is OK.

Could you share a recording? I'm not sure I can reproduce it.

nikku

This works well, and while there is still things to tackle as a potential follow-up I see the current state as worth releasing.

lmbateman · 2024-12-13T20:55:24Z

This works well, and while there #4728 (comment) I see the current state as worth releasing.

Agreed; it's a HUGE improvement already and we shouldn't wait to release.

bpmn-io-tasks bot added the needs review Review pending label Nov 26, 2024

bpmn-io-tasks bot assigned abdul99ahad Nov 26, 2024

abdul99ahad marked this pull request as draft November 26, 2024 09:23

bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Nov 26, 2024

abdul99ahad requested review from nikku, lmbateman and barmac November 26, 2024 10:52

nikku reviewed Nov 26, 2024

View reviewed changes

abdul99ahad force-pushed the focus-indicators branch from e18c2a3 to 30bf720 Compare November 26, 2024 11:33

abdul99ahad force-pushed the focus-indicators branch 3 times, most recently from 8b69b40 to 48a989b Compare November 26, 2024 15:48

abdul99ahad force-pushed the focus-indicators branch 2 times, most recently from d925bdb to 462ad4e Compare November 26, 2024 16:30

abdul99ahad marked this pull request as ready for review November 27, 2024 09:49

bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Nov 27, 2024

abdul99ahad requested a review from nikku November 27, 2024 10:58

abdul99ahad force-pushed the focus-indicators branch 2 times, most recently from 40d88f3 to d7ee414 Compare December 3, 2024 15:11

abdul99ahad force-pushed the focus-indicators branch from 41c44b5 to d3a3def Compare December 3, 2024 15:58

nikku reviewed Dec 6, 2024

View reviewed changes

abdul99ahad mentioned this pull request Dec 9, 2024

V5.2.2 camunda/camunda-bpmn-js#395

Merged

4 tasks

abdul99ahad force-pushed the focus-indicators branch from fd9099e to 281c49a Compare December 9, 2024 15:54

nikku approved these changes Dec 13, 2024

View reviewed changes

abdul99ahad added 13 commits December 13, 2024 10:24

fix: show focus indicators only when containers are expanded

625d2f9

fix: tab action btn

bef2f5d

fix: bottom panel focus

2111d31

fix: execution-info

fb484eb

fix: version-info-hyperlinks

e0d85e3

fix: bottom tab

8fdb2d4

fix: modeler-tabs

2d72dc6

fix: focus-visible default

770d66f

fix: panel expand trigger focusable

691a781

fix: watermark display

993040b

deps: update to @bpmn-io/[email protected]

3b74af3

deps: update to @bpmn-io/[email protected]

9ae389e

deps: update to [email protected]

21853a0

nikku force-pushed the focus-indicators branch from 281c49a to 21853a0 Compare December 13, 2024 09:28

nikku mentioned this pull request Dec 13, 2024

Make focus indicators in Desktop Modeler more obvious (a11y) #4632

Closed

nikku merged commit 887eda1 into develop Dec 13, 2024
12 checks passed

nikku deleted the focus-indicators branch December 13, 2024 09:48

bpmn-io-tasks bot removed the needs review Review pending label Dec 13, 2024

github-actions bot added this to the M84 milestone Dec 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Focus indicators consistent #4728

Focus indicators consistent #4728

abdul99ahad commented Nov 26, 2024 •

edited by nikku

Loading

github-actions bot commented Nov 26, 2024

nikku Nov 26, 2024

abdul99ahad Nov 26, 2024

nikku Nov 26, 2024

abdul99ahad Nov 26, 2024

nikku left a comment

abdul99ahad commented Nov 26, 2024

abdul99ahad commented Nov 26, 2024

abdul99ahad commented Nov 27, 2024 •

edited

Loading

lmbateman commented Nov 29, 2024 •

edited

Loading

nikku commented Nov 29, 2024

abdul99ahad commented Dec 3, 2024 •

edited

Loading

nikku commented Dec 6, 2024 •

edited

Loading

nikku Dec 6, 2024

abdul99ahad Dec 9, 2024

nikku Dec 9, 2024

lmbateman commented Dec 11, 2024

nikku commented Dec 12, 2024

nikku left a comment

lmbateman commented Dec 13, 2024 •

edited

Loading

Focus indicators consistent #4728

Focus indicators consistent #4728

Conversation

abdul99ahad commented Nov 26, 2024 • edited by nikku Loading

Proposed Changes

Tab action buttons

Checkbox

Buttons

Bottom panel tab bar

Bottom panel icons

Bottom panel focus alignment issue

Overlay buttons

Footer icons

Variable outline tab

Checklist

github-actions bot commented Nov 26, 2024

nikku Nov 26, 2024

Choose a reason for hiding this comment

abdul99ahad Nov 26, 2024

Choose a reason for hiding this comment

nikku Nov 26, 2024

Choose a reason for hiding this comment

abdul99ahad Nov 26, 2024

Choose a reason for hiding this comment

nikku left a comment

Choose a reason for hiding this comment

abdul99ahad commented Nov 26, 2024

abdul99ahad commented Nov 26, 2024

abdul99ahad commented Nov 27, 2024 • edited Loading

Summary of the changes:

lmbateman commented Nov 29, 2024 • edited Loading

nikku commented Nov 29, 2024

abdul99ahad commented Dec 3, 2024 • edited Loading

nikku commented Dec 6, 2024 • edited Loading

nikku Dec 6, 2024

Choose a reason for hiding this comment

abdul99ahad Dec 9, 2024

Choose a reason for hiding this comment

nikku Dec 9, 2024

Choose a reason for hiding this comment

lmbateman commented Dec 11, 2024

nikku commented Dec 12, 2024

nikku left a comment

Choose a reason for hiding this comment

lmbateman commented Dec 13, 2024 • edited Loading

abdul99ahad commented Nov 26, 2024 •

edited by nikku

Loading

abdul99ahad commented Nov 27, 2024 •

edited

Loading

lmbateman commented Nov 29, 2024 •

edited

Loading

abdul99ahad commented Dec 3, 2024 •

edited

Loading

nikku commented Dec 6, 2024 •

edited

Loading

lmbateman commented Dec 13, 2024 •

edited

Loading