Skip to content

Focus a question card on appear without trapping Tab - #1144

Merged
alexeyzimarev merged 2 commits into
mainfrom
question-card-focus
Sep 24, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
question-card-focus

Conversation

@nortonandreev

Copy link
Copy Markdown
Contributor

No GitHub issue or Linear id — neither was opened for this change.

What & why

A question card in the desktop chat did not take the keyboard when it appeared, and a focused option had no ring of its own on this palette. The card now moves focus to the first option, or the text field when there are no options, when it shows in the chat you are looking at. Arrow keys and 1–9 move between options without selecting them; Enter or Space activates. Tab stays on the normal order and can leave the card for the composer, because the card is inline in the chat rather than a dialog.

Where to look

QuestionCardKeyboard records which step it already focused, so a recycled row does not take focus again, and it does not steal focus from outside that chat. Advancing a series moves focus to the next question only when focus was already inside the card.

Verification

dotnet run --project test/Capacitor.App.Tests.Unit/Capacitor.App.Tests.Unit.csproj -- --treenode-filter "/*/*/ChatTabViewSmokeTests/A_question_card_takes_focus*" — passed, twice, then again after the final edit.

dotnet run --project test/Capacitor.App.Tests.Unit/Capacitor.App.Tests.Unit.csproj -- --treenode-filter "/*/*/ChatTabViewSmokeTests/Advancing_a_series*" — passed (included in a 44-test run of that class's question-card filters; the one failure in that run was the focus test before a flaky assertion, and the rerun above passed).

The card sits in the chat, so Tab stays on Continue and can reach the composer. Arrow and number keys only move focus, so a fast-path option is not submitted while moving.
@nortonandreev nortonandreev self-assigned this Sep 24, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Improve question card keyboard focus and navigation

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Focuses newly visible question cards without stealing focus from another chat.
• Adds non-activating arrow and number-key navigation while preserving normal Tab flow.
• Adds visible focus borders and tests for focus lifecycle and step advancement.
Diagram

graph TD
  Chat["Visible chat"] -->|renders| Card["Question card"] -->|activates| Behavior["Keyboard behavior"] --> Same{"Same chat?"}
  Same -->|Yes| Target["First input"]
  Same -->|No| Preserve["Keep focus"]
  Keys["Arrow / 1-9"] --> Behavior
  Behavior -->|moves only| Target
  Card -->|Tab continues| Composer["Chat composer"]
Loading
High-Level Assessment

The attached behavior is the appropriate approach because focus management belongs to the Avalonia view layer and must be shared by both question-card templates. View-model-driven focus or per-template code-behind would couple presentation state to domain models or duplicate lifecycle handling, while dialog-style focus trapping would conflict with the card's inline placement.

Files changed (3) +338 / -2

Enhancement (1) +235 / -0
QuestionCardKeyboard.csAdd question-card focus and keyboard navigation behavior +235/-0

Add question-card focus and keyboard navigation behavior

• Introduces an attached Avalonia behavior that focuses the first relevant control when a visible card or question step appears, while avoiding repeated focus claims and focus theft across chats. It handles arrow and 1–9 navigation without activation, supports recycled visual rows, retries after layout, and cleans up subscriptions when detached.

src/Capacitor.App/Views/QuestionCardKeyboard.cs

Bug fix (1) +25 / -2
PendingCardTemplates.axamlEnable accessible focus styling and continuous Tab navigation +25/-2

Enable accessible focus styling and continuous Tab navigation

• Activates the shared keyboard behavior for standard and ACP question cards and explicitly keeps Tab navigation continuous. Replaces default focus adorners with palette-compatible purple borders for options, step controls, and review rows while preserving selected-state backgrounds.

src/Capacitor.App/Views/PendingCardTemplates.axaml

Tests (1) +78 / -0
ChatTabViewSmokeTests.csCover question-card focus, navigation, and step transitions +78/-0

Cover question-card focus, navigation, and step transitions

• Adds headless UI tests verifying initial focus and its visible border, non-activating arrow and numeric navigation, normal forward and reverse Tab flow, and focus transfer to the next question in a series.

test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs

@qodo-code-review

qodo-code-review Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Focus misses the next question ✓ Resolved 🐞 Bug ≡ Correctness
Description
BelongsToStep treats every focused control without a QuestionGroupViewModel ancestor as
belonging to the newly selected step. When a keyboard user activates a still-visible Next or Back
button, MoveFocus therefore leaves focus on that navigation button instead of moving it to the new
question's first option or text field.
Code

src/Capacitor.App/Views/QuestionCardKeyboard.cs[R90-91]

+        var group = GroupOf(element);
+        return group is null || group.Index == step;
Relevance

●●● Strong

Clear focus-navigation bug conflicts with the PR’s stated goal; keyboard accessibility fixes are
consistently accepted.

PR-#985

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
MoveFocus returns without focusing its target whenever BelongsToStep succeeds. The Next and Back
buttons are outside the current question's data template, while both commands change CurrentIndex;
because a missing group currently returns true, persistent navigation buttons are incorrectly
accepted as focus belonging to the new step.

src/Capacitor.App/Views/QuestionCardKeyboard.cs[64-67]
src/Capacitor.App/Views/QuestionCardKeyboard.cs[85-92]
src/Capacitor.App/Views/PendingCardTemplates.axaml[214-225]
src/Capacitor.App/ViewModels/QuestionCardViewModel.cs[220-241]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Question-series navigation retains focus on a persistent Next or Back button because `BelongsToStep` accepts controls with no question-group ancestor. This prevents automatic focus from reaching the newly displayed question.

## Fix Focus Areas
- src/Capacitor.App/Views/QuestionCardKeyboard.cs[64-67]
- src/Capacitor.App/Views/QuestionCardKeyboard.cs[85-92]
- src/Capacitor.App/Views/PendingCardTemplates.axaml[214-225]

## Recommended Fix
Change `BelongsToStep` so a non-review control belongs to the current step only when it has a `QuestionGroupViewModel` ancestor whose index matches the step. Add a keyboard test that selects a multi-select answer, activates Next, and verifies focus moves to the following question's first option.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. A focus test comment repeats behavior ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The XML comment on A_question_card_takes_focus_and_tabs_out only summarizes the focus, arrow-key,
number-key, and Tab assertions directly below it. Because it records no rationale or constraint
beyond the test body, later behavior changes require maintainers to update a redundant summary as
well.
Code

test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs[R1142-1143]

+    /// The card takes the keyboard when it appears, options move with arrows and number keys without
+    /// activating, and Tab leaves the card for the composer.
Relevance

●●● Strong

Recent precedents accept removing redundant test comments, including this exact test file.

PR-#834
PR-#703

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2762993 permits comments only when they document non-obvious, behavior-critical
constraints. The cited comment repeats the test's asserted keyboard behavior without adding
rationale or an invariant.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs[1142-1143]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The XML comment above `A_question_card_takes_focus_and_tabs_out` merely summarizes behavior already expressed by the test name and assertions.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs[1142-1143]

## Recommended Fix
Remove the XML comment. Retain a comment only if it documents a non-obvious constraint or rationale that cannot be inferred from the test body.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 11/18, lines 340/200; both must reach the floor). Router rationale: This introduces substantial, stateful keyboard/focus logic across multiple Avalonia paths, with many independent edge cases around recycled rows, chat scope, retries, navigation, and series advancement.

Grey Divider

Tip of the day
💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs Outdated
Comment thread src/Capacitor.App/Views/QuestionCardKeyboard.cs Outdated
Next and Back stay visible when the question changes, so a control with no question group cannot count as already on the new step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants