Repository navigation
fix(workflow-form): keep the keyboard out of a read-only input card, not only the pointer - #9016
Conversation
Automated Reviewer SuggestionsBased on the
|
Backport auto-label reportThis
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9016 +/- ##
=========================================
Coverage 92.33% 92.33%
Complexity 5045 5045
=========================================
Files 1260 1261 +1
Lines 54067 54120 +53
Branches 6755 6768 +13
=========================================
+ Hits 49921 49972 +51
Misses 2486 2486
- Partials 1660 1662 +2
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
a963b0f to
ef10167
Compare
|
/request-review: @mengw15 |
There was a problem hiding this comment.
🟡 Changes recommended
The lock still permits activation outside Tab navigation, and widget redraws retain detached controls.
2 open issues
What changed in this PR
This PR strengthens Form View’s read-only input cards by blocking Tab navigation without hiding their contents from assistive technology.
Changes:
- Adds a directive that manages descendant tab order and accessibility attributes.
- Applies the directive to read-only input cards.
- Adds directive and rendered-template regression tests.
| File | Description |
|---|---|
| frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts | Tests directive integration in the real template. |
| frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts | Imports the directive. |
| frontend/src/app/workspace/component/workflow-form/workflow-form.component.html | Binds the lock to editing permissions. |
| frontend/src/app/workspace/component/workflow-form/read-only-card.directive.ts | Manages locking, restoration, and DOM observation. |
| frontend/src/app/workspace/component/workflow-form/read-only-card.directive.spec.ts | Tests locking, restoration, and observer lifecycle. |
Review effort: Balanced
…not only the pointer The card is locked with `pointer-events: none`, which is hit testing only. A keyboard user could Tab into the controls a custom widget draws of its own -- a file picker's Browse button, the HuggingFace file input -- and activate them with Enter or Space; those controls do not consult the form's disabled state the way the bound formly controls do, so the upload went out from a read-only card. A directive on the card takes every focusable descendant out of the tab order and marks it aria-disabled, re-applying when a widget renders or redraws its own controls. `inert`, which the step panel uses for its read-only mount, would also remove the card from the accessibility tree: a reader's card exists to be read, so that would trade this bug for a worse one.
ef10167 to
d5eb98e
Compare

What changes were proposed in this PR?
Closes #8943. Part of the Form View feature (#8011); from the #8439 refinements (item 5).
A read-only input card -- what a viewer without write access sees -- is locked with
pointer-events: none. That is hit testing only. The bound formly controls are disabled throughprops.disabled, but the controls a custom widget draws of its own are not: the HuggingFace audio and image uploaders each render a plain file input that nothing disables. A reader could Tab into one inside a locked card, open it with Enter and choose a file.For the audio input that is a real write. The widget POSTs the chosen file to
/api/huggingface/upload-audio, which authorises any signed-in user and never asks who may write this workflow, so the file lands in the server's temp store. Checked on a workflow shared read-only: two presses of Tab reach the uploader inside the locked card, Enter opens the file dialog, and the POST comes back 200 with the stored path. The image uploader is client-side, so there the reader only makes the card display a picture of their choosing -- a read-only card wires no write-back and a reader's save is refused, so the operator keeps the author's value and a run still runs it.The card now also keeps the keyboard out, through a small directive on it (
texeraReadOnlyCard, applied as[texeraReadOnlyCard]="!canEdit"beside the existingread-onlyclass):tabindex="-1", its own value remembered) and markedaria-disabled="true", so there is nothing for Tab to reach and assistive tech says why rather than going quiet.tabindex="-1"ends only sequential navigation andaria-disabledonly announces, so a reader's assistive tech could still focus a widget's file input and activate it, which for the audio one posts the file (Copilot).[disabled]="isUploading"). The observer runs outside Angular and sets only what is not already set, so one pass settles it.inertis what the step panel uses for its read-only mount, and the issue suggested following it; this does not, deliberately.inertalso removes the subtree from the accessibility tree, and a reader's card exists to be read -- the values are what a reader runs the workflow with -- so that would trade this bug for a worse one. Disabled controls stay in the accessibility tree and keep being announced. The directive's doc comment says so, next to the code.Nothing else changes: the pointer lock, the styles, the card's content and the author's editing path are untouched, and the directive does nothing at all while a card is editable.
Any related issues, documentation, discussions?
Closes #8943, which re-files #8758 (closed). Part of #8439 (item 5) and the Form View feature #8011. Raised by Copilot on #8437, #8438 and #8440.
Three neighbouring things are deliberately left alone. The widgets' own controls still do not consult the field's disabled state in their own handlers -- fixing that is per-widget work (the HuggingFace uploaders, the dataset picker, the resource value selector), and the card-level lock covers every widget at once, including ones added later. The upload endpoint itself authorises on being signed in and nothing else (
@RolesAllowed("REGULAR","ADMIN"), no workflow-access check), so it would still accept a file posted by hand; closing the UI route does not close that one, and it is worth its own issue. And the step panel'sinertmount has the same accessibility-tree problem described above; changing it is not this issue's scope, and it is worth its own look.How was this PR tested?
Unit tests (vitest). The directive's own spec, on a host shaped like a card (a bound input, a widget's Browse button, a link, a span, a control with its own
tabindex): every focusable control in a read-only card is out of the tab order; each is markedaria-disabledwhile the card is not inert and its text is still there; an editable card is left alone, keeping a control's owntabindex="3"; a card that becomes editable gets that3back, the untouched control gets notabindex, and thearia-disabledmarks go; a control a widget adds after the card was built is locked; a control whose widget writes itstabindexback, or enables again, is locked again; a click on a locked control dispatches no activation and the controls report disabled (jsdom does not model a disabled control refusing focus, so that half is asserted through the property the browser acts on); the disabled state is given back on unlock; five rounds of a control being added and removed leave the tracked set where it started, rather than holding the detached nodes; the observer is disconnected when the card goes away. The page's rendered spec wires it through the real template: a read-only viewer's card, with a button appended the way a widget renders one, has it out of the tab order and marked, and the card is not inert.By hand, on two dev builds of the same base commit, one with this change and one without, signed in as a second account the workflow is shared read-only with. Before: from the view toggle, two presses of Tab land on the uploader inside the locked card, Enter opens the file dialog, and choosing a file POSTs it (200). After: the same two presses land on Run, the card is skipped, and no request goes out. The mouse reaches nothing in either build, and the stored workflow is unchanged in both.
Each guard was deletion-checked and turned a named test red (the directive not applied; the tabindex write; the aria-disabled write; the native disable; the editable check; the unlock; the original-tabindex restore; the original-disabled restore; the release of what has left the card; the childList and both attribute halves of the observer; the disconnect). eslint, prettier and the production (AOT) build pass; every statement, function and branch this PR adds is covered, and the full frontend suite passes.
Video
Before
Screen.Recording.2026-10-18.at.4.10.47.PM.mov
After
Screen.Recording.2026-10-18.at.4.11.17.PM.mov
Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (Anthropic). Co-authored with Claude, reviewed line by line by the author before submission.
🤖 Generated with Claude Code