fix(inline-edit)!: errors following a full review of the component by NikGurev · Pull Request #2024 · koobiq/angular-components

NikGurev · 2026-09-10T07:44:09Z

Summary

The inline edit shipped its keyboard and screen-reader contract on two aria-hidden="true" tabindex="0"
sentinels inside the edit overlay — an axe aria-hidden-focus failure and two dead-end tab stops — while
the view mode announced nothing at all. The review replaced the sentinels with a Tab boundary resolved
against the panel's own first and last tabbable control, moved the button role and its accessible name
onto the view content, and fixed the validation path, which committed a never-touched invalid value and
flipped a pristine required field into its error look on the first keystroke.

The second commit is a follow-up pass over the findings an independent verifier could not confirm outright;
the main one there is the focus ring, which a keyboard user lost on every field after the first when
Tab-chaining between editors.

The third commit folds the mode state into one model(). It was three members — a protected signal(),
the public modeAsReadonly computed() that existed only to expose it for reading, and a modeChange
output() fed by a hand-written toObservable(...).pipe(skip(1)) relay. The model generates an output of
the same name and payload, so (modeChange) bindings are untouched, and in exchange the mode becomes
writable: [(mode)] opens and closes the editor from the host, which nothing but toggleMode() could do
before.

ng update reporting ships as the inline-edit-a11y-and-types schematic. It reports everything except the
one change with a single mechanical translation — modeAsReadonly → mode — which it rewrites.

List of notable changes:

fixed the accessibility of the edit overlay: the two aria-hidden sentinels and the
KbqFocusRegionItem directive that marked them are gone, and the Tab boundary is resolved against the
panel's own first/last tabbable control
added the view-mode ARIA layer — role="button", aria-expanded, aria-disabled and a name from
the new edit key in KbqA11yLocaleConfiguration or from an aria-label input — on
.kbq-inline-edit__view-content rather than on the host
fixed save(), which now marks the projected controls touched itself and gates on the control's
validity instead of the cached ErrorStateMatcher verdict, so commit() no longer writes a
never-touched invalid value
fixed the Tab-chain neighbour resolution: closest() plus a registry and a real toggleMode() call
instead of a synthetic KeyboardEvent, and the chained field keeps its keyboard focus ring
fixed the focus restore when leaving edit mode — the target is resolved after the view is back,
through the template's own view children rather than an unscoped querySelector
added [(mode)]: mode is a model() now — public, writable and two-way bindable. The
modeAsReadonly alias is removed and the explicit modeChange output is generated by the model
fixed the overlay offset, measured in an effect keyed on the mode rather than ahead of the write in
toggleMode(), so an externally driven [(mode)] positions the panel correctly too
updated the public API: saved and canceled are public, KbqFocusRegionItem is no longer
exported, setValueHandler takes (value: unknown) => void and validationTooltip a
TemplateRef<unknown> — both anys are gone
fixed the focus ring, which outlined both the field and the kbqInlineEditMenu button when Tab
landed on the menu — the host watches its whole subtree, so it was marked focused for the button too
fixed Tab out of a select-style editor, which left the field open and dropped focus on <body>:
the select renders its options in an overlay of its own and swallows the key, so neither the panel's
handler nor the focus the chain follows survived
fixed the validation-tooltip lifecycle (scroll handle torn down from DestroyRef as well as from
detachments()) and the offsetHeight read that ran from a template binding
updated the styles: the panel finally reads --kbq-inline-edit-panel-shadow, and the private
.kbq-mask* classes became .kbq-inline-edit__menu-mask*
added axe, focus, keyboard and listener-teardown unit coverage, plus a DOM-asserted Playwright
keyboard scenario that needs no baseline of its own
updated the Validation section of both guides and added Keyboard and Accessibility sections

What should reviewers focus on?

packages/schematics/src/migrations/inline-edit-a11y-and-types/ and its registration in
collection.json / migrations.json. It writes now: the rename pass follows
filter-bar-rename-action, matches .modeAsReadonly as a property access only, and reports the index
and destructuring forms it cannot rewrite.
The overlay-offset move. It relies on a component effect() running before the template refresh, so the
measurement lands before the overlay attaches. Asserted directly — the spec mocks SharedResizeObserver
to disable the late correction and reads the offset from the CdkConnectedOverlay.attach output.
The selector break: a consumer stylesheet or test that matched the host by tabindex or :focus has to
be retargeted at .kbq-inline-edit__view-content. The focus ring still lands on the host through
cdk-keyboard-focused.
packages/components/core/locales/* and tools/public_api_guard/components/core.api.md are shared with
the other branches in this series — the edit key is the only addition here.
KbqFocusRegionItem leaving the entry point: it existed to mark the sentinels and has no other call site.

The sixth of the component-review series, rebased onto dec52b292. Deferred: IE-A11Y-03, whose root cause
is kbq-icon-button and belongs to fix/icon. Also deferred, raised during review: getValueHandler /
setValueHandler are fragile — controls are discovered through contentChildren(KbqFormField), so without
a <kbq-form-field> wrapper the host writes an unknown round-trip with a cast, and the automatic path
restores values by array index. The native fix is NgControl discovery scoped to the editor; separate task.

🤖 Generated with Claude Code

github-actions · 2026-09-10T07:48:55Z

Visit the preview URL for this PR (updated for commit dc8187a):

https://koobiq-next--prs-2024-hgkunws0.web.app

_{(expires Fri, 25 Sep 2026 15:17:26 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c}

Copilot

🔵 Needs a closer look

It is a large breaking change spanning accessibility, focus management, validation logic, public API, and a regex-based migration schematic, so it warrants final human review despite the extensive tests and no definitive defect found.

Pull request overview

This PR is the sixth in a component-review series and delivers a breaking accessibility, focus-management, and validation overhaul of KbqInlineEdit, plus an ng update schematic to help consumers migrate. It replaces the two aria-hidden="true" tabindex="0" overlay sentinels (an axe aria-hidden-focus failure) with a Tab boundary resolved against the panel's own first/last tabbable control, moves widget semantics (role="button", aria-expanded, aria-disabled, accessible name) onto .kbq-inline-edit__view-content/.kbq-inline-edit__focus-anchor, and fixes the validation path so a never-touched invalid value is no longer committed and a pristine required field no longer flips to its error look on the first keystroke. It also folds the mode state into a single model() (enabling [(mode)]), removes any from the public surface, and adds a new edit key to KbqA11yLocaleConfiguration.

Changes:

Accessibility/focus: sentinel + KbqFocusRegionItem removal, view-content ARIA layer, focusMonitor-based focus restore, Tab-chaining via a WeakMap registry.
API/state: mode becomes a model(), saved/canceled public, setValueHandler/validationTooltip typed unknown, overlay offset measured in a mode-keyed effect.
Tooling/docs: new inline-edit-a11y-and-types schematic (+ tests), edit locale key across all 5 locales, updated en/ru overview + migration guides, refreshed examples and unit/e2e coverage.

File summaries

File	Description
packages/components/inline-edit/inline-edit.ts	Core rework: `mode` model, offset effect, focus restore, tab chaining, control-based validity
packages/components/inline-edit/inline-edit.html	View-content ARIA, focus anchor, panel Tab handlers, offset binding
packages/components/inline-edit/inline-edit.scss	Mask classes renamed, panel shadow token, focus outline suppression
packages/components/inline-edit/inline-edit.spec.ts	New a11y/focus/tab-chaining/validation coverage; host refactor
packages/components/inline-edit/e2e.{ts,playwright-spec.ts}	`modeAsReadonly`→`mode`; DOM-asserted keyboard scenario
packages/components/inline-edit/inline-edit.{en,ru}.md	New Keyboard, Accessibility, Switching-modes sections
packages/components/core/locales/*.ts, types.ts	New required `edit` a11y key across all locales
packages/schematics/src/migrations/inline-edit-a11y-and-types/*	New migration (index/data/schema/README/spec)
packages/schematics/src/{collection,migrations}.json	Register the migration for `21.0.0-0`
docs/guides/migration.{en,ru}.md	Inline-edit migration entry
packages/docs-examples/components/inline-edit/*/.ts	Simplified validation example; `modeAsReadonly`→`mode`; `unknown` handler
tools/public_api_guard/components/{inline-edit,core}.api.md	Regenerated golden API files

Review details

Files reviewed: 31/31 changed files
Comments generated: 0
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

- IE-A11Y-01: drop the two `aria-hidden="true" tabindex="0"` sentinels from the edit overlay and resolve the Tab boundary against the panel's own first/last tabbable control; `KbqFocusRegionItem` goes with them - IE-A11Y-02: announce the view mode as a `button` with `aria-expanded`, `aria-disabled` and a name from the new `edit` a11y locale key or an `aria-label` input, on the view content rather than the host - IE-BUG-01: `save()` marks the projected controls touched itself and gates on the control's validity instead of the cached `ErrorStateMatcher` verdict - IE-DOC-01: correct the Validation section of both guides and add Keyboard and Accessibility sections - IE-A11Y-04: own the focus restore when leaving edit mode, resolving the target after the view is back instead of relying on the node CDK captured - IE-BUG-02: resolve the Tab-chain neighbour through `closest()` and a registry, and call its `toggleMode()` instead of dispatching a synthetic `KeyboardEvent` - IE-BUG-03: stop marking every control touched on every keystroke in edit mode, and delete the two workaround classes from the validation example - IE-LIFE-01: keep the validation-tooltip scroll handle installed past the fallback timer and tear it down from `DestroyRef` as well as from `detachments()` - IE-PERF-01: `overlayOrigin` is a `computed()` and the panel offset a signal written before the overlay opens, so no `offsetHeight` read runs from a template binding - IE-ARCH-01: drop the unreachable single-sentinel branch and build the scroll strategy lazily instead of holding it in a never-written `WritableSignal` - IE-API-01: make `saved`, `canceled` and `modeChange` public, stop exporting `KbqFocusRegionItem`, and replace both `any`s with `unknown` - IE-BUG-04: re-run interactive-content detection from a `ContentObserver` and scope the `closest()` walk to the component - IE-STYLE-01: read `--kbq-inline-edit-panel-shadow` in the panel and rename the private `.kbq-mask*` classes to `.kbq-inline-edit__menu-mask*` - IE-TEST-01: add axe, focus, keyboard and listener-teardown coverage, plus a DOM-asserted Playwright keyboard scenario - IE-TEST-02: give the dead tab-order host a real case, drop the `@Directive` base and stop binding optional inputs from `undefined` signals Deferred: IE-A11Y-03 (root cause is kbq-icon-button, owned by fix/icon) Stale: none

Follow-up to the component review: - `saveAndFocusNextInlineEdit` opened the neighbour through `toggleMode()`, which leaves `editModeOrigin` unset, so leaving that editor restored focus with a `program` origin. Confirmed in Chrome: Enter, Tab, Escape left the chained field with `cdk-program-focused` and no `box-shadow`, i.e. a keyboard user lost the focus indicator on every field after the first, and the menu mask stayed hidden. - `accessibleName` fell back with `??`, so an `aria-label` that interpolates to an empty string left the `role="button"` view content with no accessible name at all. Falls back on empty now. - `restoreFocus()` resolved its target with `querySelector` against the two class names the template already exposes as view children — a second source of truth, and unscoped, so a nested inline edit would hand back the wrong anchor. - The focus anchor's `[attr.aria-expanded]` sat inside an `@if` on view mode, so it could only ever render `false`. - The migration compiled its regexes once per visited file; they are module constants without the `g` flag, so they are compiled once now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`mode` was three members: a `protected signal()`, the public `modeAsReadonly` `computed()` that existed only to expose it for reading, and a `modeChange` `output()` fed by a hand-written `toObservable(...).pipe(skip(1))` relay. The model generates an output named `modeChange` carrying the same value, so `(modeChange)` bindings are untouched. It emits synchronously on the write rather than one microtask later, and is no longer a property of the class, so a programmatic subscription goes through `toObservable(inlineEdit.mode)`. In exchange the mode is writable: `[(mode)]` opens and closes the editor from the host, which nothing but `toggleMode()` could do before. The overlay offset is measured in an effect keyed on the mode rather than ahead of the write in `toggleMode()`, so an external `[(mode)]` write positions the panel correctly too. A component effect runs before the template refresh, so the measurement still lands before the overlay attaches. `inline-edit-a11y-and-types` gains the rewrite: `.modeAsReadonly` becomes `.mode` in `.ts` and `.html`, with an index read or a destructuring reported instead. BREAKING CHANGE: `KbqInlineEdit.modeAsReadonly` is removed — read `mode`, which is a `model()` now. `modeChange` keeps its name and payload for template bindings but is no longer a property of the class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Tab out of the panel's last control saves the field and opens its neighbour. Nothing exercised that end to end: the keyboard suite covered Enter and Escape only, and the jsdom unit tests stand in for the browser moving focus by calling `.focus()` on the neighbour themselves — which is the exact step the chain depends on, so they stay green whether or not it works. Three scenarios on a real Tab: forward, twice in a row, and backwards on Shift+Tab, which resolves the boundary against the panel's first tabbable control rather than its last. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The widget semantics moved onto the view content in this branch, so the host was given `cdkMonitorSubtreeFocus` to keep drawing the ring for it. That marks the host focused for *any* descendant, including the `kbqInlineEditMenu` button — which draws a ring of its own, so a Tab onto the menu outlined both the button and the whole field. Suppressed on the host while the menu holds keyboard focus, using the `:has()` form the file already uses for the menu's pressed and focused states. The class on the host is left alone: for a subtree monitor it is accurate, and it is the second ring that is wrong. Covered by an e2e test that asserts the computed shadow rather than the class, and that enters the view content with a real Tab — a programmatic focus() is reported as `program`, draws no ring at all, and would make the check vacuous. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A single-value select renders its options in an overlay of its own, and the editor opens that overlay itself, so focus leaves the inline-edit panel the moment the editor opens. Two things follow: the panel's `(keydown.tab)` can never fire, because the key is pressed in a different overlay, and the select `preventDefault()`s Tab and closes, which destroys focus rather than moving it. The field stayed open and focus landed on `<body>`. The key is caught on the document instead, for as long as the panel is attached. The neighbour is resolved by document order, because `document.activeElement` is `<body>` by then and carries no information — every other path still follows the focus the browser actually moved. A Tab the select used to walk its own footer is left alone: it keeps the panel open, which is what the deferred check reads. Covered by `E2eInlineEditSelectChain`, a new scenario with three adjacent select editors. Both directions are asserted end to end, since neither half of the mechanism survives outside a real browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The fix changes what a consumer sees without breaking a call site, so it belongs in the guides rather than in the schematic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ages Stylistic only, no change in meaning: drop the intensifiers and filler ("ровно", "действительно", "просто", "наконец"), replace the anthropomorphisms ("умеет", "обещает", "не умел", "живёт"), resolve the ambiguous pronouns, break up the run-on sentences, and format `touched` as code everywhere it appears. Also align the closing line of the migration subsection with the one every other component review uses ("Закрывается схематиком `X`: … остальное сообщается в отчёте"), write key combinations as `Ctrl+Enter` instead of `Ctrl`/`Cmd` + `Enter`, and use "нажатие" rather than "клик". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Six key behaviours across three modes read as a wall of clauses in one paragraph, and two of them are the same key (`Enter` saves, but inserts a line break inside a `textarea`), which prose can only disambiguate by repeating the context. A row per key makes the mode the reader is in the second column. What is not per-key stays prose around the table: the field being a single tab stop, a click outside behaving like `Tab`, and focus returning to the field on exit. Also restores `Ctrl`/`Cmd` + `Enter` in the Russian page, which the previous commit had split into `Ctrl+Enter` or `Cmd+Enter`. The slash form is what list, select and tree-select use for a platform-aware shortcut, in both languages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Adds the members PR #2068 landed — compareWith, saveHandler, saveErrorHandler, saveStatus, retrySave, rollback, saveError and the save types — alongside this branch's ariaLabel, the mode model() and the narrowed setValueHandler, and restores the CRLF the generated reports are committed with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…d a11y sections Two behaviours from the async save landed in code but not in the sections this branch rewrote. `canSaveOnEnter` no longer fires on a control that acts on Enter itself, so the keyboard table's `Enter` row was wrong for a projected button. And a row waiting on `saveHandler` keeps its tab stop while refusing to reopen — the distinction from `disabled` a keyboard user has to be told about. The save status is also announced from a visually hidden `role="status"` region rather than being left to colour, which belongs beside the rest of the locale-sourced names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…are focus ring The two save-state screenshots came from #2068, where leaving edit mode always left the row keyboard-focused. Focus now returns with the origin the editor was opened from, so a row opened by click comes back mouse-focused and draws no ring. The rows committed in these scenarios are opened by click, so the ring is gone from both. Regenerated in Docker; only 08 and 09 moved, the other nine are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The host carries `cdkMonitorSubtreeFocus`, so `cdk-keyboard-focused` lands on it for any descendant that takes focus. The ring rule excluded the menu button by name, which left every other focusable descendant drawing two rings at once: Tab onto a projected link outlined both the link and the whole row. The exclusion is now a descendant combinator instead of a list of selectors — `interactiveSelectors` is a runtime input, so what stays focusable in view mode cannot be enumerated in a stylesheet. The row ring stands for the component's own tab stops, the view content and the focus anchor, and is suppressed once focus moves past them into projected content or into the menu. Written against a focused *descendant* rather than against the view content being `:focus` itself, so the screenshot fixture that paints `cdk-keyboard-focused` on four rows at once — real focus being singular — still renders the ring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…not its container The previous exclusion named the two containers whose focused descendants must not raise the row ring, which still left a slot uncovered: a control projected into kbq-label sits beside the view container rather than inside it, so a Tab onto an icon button in the label outlined both the button and the whole row. Naming containers cannot be made complete — every slot a consumer can project a focusable control into has to be listed, and a missed one fails silently as a double ring. The condition is now about what holds focus: the row ring is drawn while one of the component's own two tab stops has it, and suppressed for anything else inside the host. Written so that the fixture painting `cdk-keyboard-focused` on four rows at once still renders the ring, real focus being singular: with nothing focused there is no element for `:has()` to match. The e2e scenario grows a label with an icon button, and its test now walks both slots a consumer can fill before landing on the edit anchor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions Bot added the bug Something isn't working label Sep 10, 2026

NikGurev changed the title ~~fix(inline-edit): errors following a full review of the component~~ fix(inline-edit)!: errors following a full review of the component Sep 10, 2026

github-actions Bot added the breaking changes label Sep 10, 2026

lskramarov assigned NikGurev Sep 10, 2026

lskramarov approved these changes Sep 10, 2026

View reviewed changes

NikGurev marked this pull request as ready for review September 11, 2026 07:25

artembelik requested a balanced review from Copilot September 11, 2026 11:03

Copilot started reviewing on behalf of artembelik September 11, 2026 11:04 View session

Copilot AI reviewed Sep 11, 2026

View reviewed changes

NikGurev marked this pull request as draft September 11, 2026 12:05

NikGurev force-pushed the fix/inline-edit branch 2 times, most recently from 07a32ea to 334c543 Compare September 17, 2026 11:18

NikGurev marked this pull request as ready for review September 17, 2026 11:18

NikGurev marked this pull request as draft September 21, 2026 13:57

NikGurev force-pushed the fix/inline-edit branch from 334c543 to f73a66b Compare September 21, 2026 15:26

NikGurev marked this pull request as ready for review September 21, 2026 15:32

NikGurev force-pushed the fix/inline-edit branch from 557b7e4 to 3b63ea7 Compare September 22, 2026 14:29

lskramarov and others added 12 commits September 22, 2026 17:29

docs(inline-edit): note the select editor's Tab behaviour in the guides

2f31899

The fix changes what a consumer sees without breaking a call site, so it belongs in the guides rather than in the schematic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

NikGurev force-pushed the fix/inline-edit branch from 3b63ea7 to 0bef680 Compare September 22, 2026 14:34

NikGurev and others added 2 commits September 22, 2026 17:55

NikGurev merged commit 3372529 into main Sep 22, 2026
14 checks passed

NikGurev deleted the fix/inline-edit branch September 22, 2026 15:23

Navigation Menu

fix(inline-edit)!: errors following a full review of the component - #2024

fix(inline-edit)!: errors following a full review of the component#2024
NikGurev merged 14 commits into
mainfrom
fix/inline-edit

NikGurev commented Sep 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

NikGurev commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

List of notable changes:

What should reviewers focus on?

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NikGurev commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading