Skip to content

Follow-ups to #104, #106 and #107: CI test order, dialog focus, TV keyboard - #109

Merged
keyldev merged 3 commits into
mainfrom
fix/post-merge-review
Nov 4, 2026
Merged

keyldev merged 3 commits into
mainfrom
fix/post-merge-review

Conversation

@compot-dev

Copy link
Copy Markdown
Collaborator

#104, #106 and #107 were merged without these follow-up commits, which were not pushed in time. They fix the Windows CI timeouts seen with six test projects and the five Qodo findings on #106 and #107.

CI (refs #100)

  • dotnet test now runs with -m:1, one test project at a time. With all six projects running side by side on windows-latest, five ONVIF tests in Devices.Tests timed out. The likely cause is a request reaching the other project's loopback listener. This cause is not proven, and the runs on main have been green so far, so the failure may be intermittent.

Dialogs (refs #102)

  • The recording player's hotkeys yield to an open mobile dialog: with the delete confirmation open, Left/Right no longer seek and Space no longer toggles playback.
  • The presenter's initial focus no longer replaces a focus the content has already set: the SSH terminal keeps its input surface focused.

Android TV keyboard (refs #103)

  • OK reopens the keyboard after Back closed it. It now depends on the input pane's state, not on a one-shot marker, so no stray space is typed.
  • The keyboard is hidden only when the focus arrived by D-pad or Tab, so the SSH terminal's keyboard button works.
  • The activity is refreshed on every OnCreate and dropped in OnDestroy.

Checked

  • Local Docker build matching CI, desktop linux-x64 and Android: 0 warnings, 610 tests passed, 9 skipped.
  • Android TV 16 emulator, before rebasing onto current main: into the search box by D-pad, no keyboard; OK opens it; Back closes it; OK opens it again; typed text has no stray space. Same after a font-scale change.
  • Not checked: the -m:1 effect on Windows (CI only) and the dialog changes on a device.

With all six test projects in OpenIPC.Viewer.Desktop.slnx, dotnet test ran
them side by side, and on windows-latest five ONVIF tests in Devices.Tests
timed out (TaskCanceledException after 17 s to 1 min 51 s); Linux and macOS
passed, and so did Windows on every earlier run with three projects.
Devices.Tests' stub cameras listen on HttpListener (http.sys on Windows)
and Infrastructure.Tests opens loopback TcpListeners, both on ports picked
by the OS; most likely an ONVIF request reached the other project's
listener and got no answer.

-m:1 runs one test project at a time; locally all six take ~9 s in a row.

Refs #100
…ocus

Qodo on #106:
- The recording player listens for its hotkeys on the TopLevel (tunnel), so
  with its delete confirmation open Left/Right still seeked the recording
  and Space toggled playback before the dialog saw them. The handler now
  returns while a mobile dialog is open (OverlayDialogPresenter.IsAnyOpen,
  never true on the desktop, whose dialogs are windows).
- The presenter's initial focus no longer replaces a focus the content has
  already set inside the dialog: the SSH terminal focuses its input surface
  itself, and the presenter moved it to the Close button.
…tivity

Qodo on #107:
- OK on a field whose keyboard was closed with Back (the field keeps the
  focus) went to the field instead of reopening the keyboard. Whether OK
  opens the keyboard now depends on the input pane's state
  (TopLevel.InputPane), not on a one-shot marker set on focus.
- The keyboard is hidden only when the focus reached the field by D-pad or
  Tab. A focus set from code is left alone, so the SSH terminal's keyboard
  button, which refocuses its input proxy to bring the keyboard back, works.
- The activity is refreshed on every MainActivity.OnCreate and dropped in
  OnDestroy; the class handlers are still registered once.

Checked on an Android TV 16 emulator: into the search box by D-pad, no
keyboard; OK opens it; Back closes it; OK opens it again; typed text lands
without a stray space; Down moves on. Same after a font-scale change.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Serialize CI tests and fix dialog focus and Android TV keyboard behavior

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Run CI test projects sequentially to reduce intermittent Windows ONVIF timeouts.
• Keep player hotkeys out of mobile dialogs and preserve focus set by dialog content.
• Let TV users reopen the keyboard with OK without disrupting programmatic focus.
Diagram

graph TD
  CI["CI test job"] --> Tests["Sequential test projects"]
  Activity["Android activity"] --> TV["TV keyboard handler"] --> Pane["Input pane"]
  Player["Player hotkeys"] -->|checks open state| Overlay["Overlay presenter"] -->|preserves focus| Content["Dialog content"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Isolate test listener ports instead of serializing projects
  • ➕ Could retain parallel execution if listener interference is confirmed.
  • ➖ Requires investigating an unproven cause and changing test infrastructure for a small runtime saving.

Recommendation: Keep serialization as a low-cost mitigation while the Windows failure remains intermittent and its cause unconfirmed. Revisit listener isolation if CI time becomes material or the timeouts persist.

Files changed (5) +42 / -17

Bug fix (4) +37 / -16
MainActivity.csRelease TV keyboard activity on destruction +6/-0

Release TV keyboard activity on destruction

• Calls the TV keyboard helper during 'OnDestroy' so it drops its reference to the destroyed activity. Installation continues on each 'OnCreate'.

src/OpenIPC.Viewer.Android/MainActivity.cs

TvSoftKeyboard.csUse navigation and input-pane state for TV keyboard handling +24/-16

Use navigation and input-pane state for TV keyboard handling

• Hides the keyboard only after directional or Tab focus, preserving programmatic requests such as the SSH terminal’s keyboard button. OK opens a closed keyboard, including after Back, without typing a stray character; the helper also refreshes its activity reference across recreation.

src/OpenIPC.Viewer.Android/Platform/TvSoftKeyboard.cs

OverlayDialogPresenter.csPreserve focus already set within a dialog +4/-0

Preserve focus already set within a dialog

• Skips initial presenter focus when a descendant of the dialog card is already focused. This keeps content such as the SSH terminal focused on its intended input surface.

src/OpenIPC.Viewer.App/Services/OverlayDialogPresenter.cs

RecordingPlayerPage.axaml.csDefer player hotkeys while a mobile dialog is open +3/-0

Defer player hotkeys while a mobile dialog is open

• Stops the TopLevel tunnel handler from processing player shortcuts when an overlay dialog is open. Dialog navigation keys can no longer seek or toggle playback underneath the delete confirmation.

src/OpenIPC.Viewer.App/Views/Pages/RecordingPlayerPage.axaml.cs

Other (1) +5 / -1
build.ymlRun solution test projects sequentially +5/-1

Run solution test projects sequentially

• Adds '-m:1' to the CI test command to avoid concurrent test-project execution. The comment documents the suspected, but unconfirmed, connection to Windows ONVIF timeouts.

.github/workflows/build.yml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Physical keys stop reaching TV terminals 🐞 Bug ≡ Correctness
Description
TvSoftKeyboard.OnKeyDown now handles every Space or Enter on a text box while the input pane is
closed, without excluding the SSH terminal’s input proxy. On Android TV, that proxy holds focus for
terminal input, so after the soft keyboard is dismissed, repeated physical Enter presses open the
keyboard instead of reaching the shell; the previous one-shot gate consumed only the first such
press.
Code

src/OpenIPC.Viewer.Android/Platform/TvSoftKeyboard.cs[R65-68]

+        if (TopLevel.GetTopLevel(box)?.InputPane?.State == InputPaneState.Open) return;
       SetKeyboard(show: true);
       // OK only opens the keyboard: no space typed, no Enter action.
       e.Handled = true;
Evidence
The TV handler is registered for all TextBoxes and now consumes Space/Enter whenever the pane is not
open. The terminal uses a TextBox proxy with its own tunneling key handler to forward physical keys;
unlike the prior one-shot gate, the new condition remains true on every press while the pane stays
closed.

src/OpenIPC.Viewer.Android/Platform/TvSoftKeyboard.cs[30-39]
src/OpenIPC.Viewer.Android/Platform/TvSoftKeyboard.cs[60-68]
src/OpenIPC.Viewer.App/Views/Dialogs/SshTerminalContent.axaml.cs[66-82]
src/OpenIPC.Viewer.App/Views/Dialogs/SshTerminalContent.axaml.cs[391-408]
src/OpenIPC.Viewer.App/Views/Dialogs/SshTerminalContent.axaml.cs[227-250]

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 TV-wide closed-keyboard handler consumes repeated physical Enter and Space presses intended for the SSH terminal. Its input proxy needs to retain its hardware-key forwarding path even when the soft keyboard is closed.
## Fix Focus Areas
- src/OpenIPC.Viewer.Android/Platform/TvSoftKeyboard.cs[60-68]
- src/OpenIPC.Viewer.App/Views/Dialogs/SshTerminalContent.axaml.cs[66-82]
## Recommended Fix
Mark the terminal input proxy as exempt from TV OK-to-open interception and let its existing key handler receive physical keys. Keep keyboard reopening available through the terminal’s explicit keyboard button, and preserve OK-to-open behavior for ordinary text fields.

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


Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@keyldev
keyldev merged commit c76ea65 into main Nov 4, 2026
6 checks passed
@compot-dev
compot-dev deleted the fix/post-merge-review branch November 7, 2026 20:36
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