Skip to content

A belard/refactor dialog refocus - #106

Open
a-belard wants to merge 6 commits into
CMU-17313Q:mainfrom
a-belard:a-belard/refactor-dialog-refocus
Open

a-belard wants to merge 6 commits into
CMU-17313Q:mainfrom
a-belard:a-belard/refactor-dialog-refocus

Conversation

@a-belard

@a-belard a-belard commented Sep 6, 2026

Copy link
Copy Markdown

1. Issue

Link to the associated GitHub issue:

Full path to the refactored file:
/workspaces/opencode/packages/tui/src/ui/dialog.tsx

What do you think this file does?
This file implements the TUI's dialog system, it is the shared modal coordinator for the terminal UI, with focus trapping/restoration handled manually.

What is the scope of your refactoring within that file?
I changed how refocus() schedules and validates restoring the previously focused renderable after a dialog closes.

Which Qlty‑reported issue did you address?
High cognitive complexity 18 in refocus()

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
The high cognitive complexity made the focus-restoration logic harder to read, reason about, and safely modify

What changes did you make to resolve the issue?
I introduced a new isDescendant helper to isolate render-tree validation from the scheduled focus restoration, reducing refocus()’s complexity.

How do your changes improve maintainability? Did you consider alternatives?
The new isDescendant helper makes refocus() easier to read and maintain by isolating tree-validation logic; I considered keeping the traversal inline, but extracting it reduced cognitive complexity without changing behavior.

3. Validation

How did you validate that the change is correct?
I validated the change by running the TUI tests and qlty smell; the tests passed, and Qlty no longer reports the cognitive-complexity issue for refocus().

Attach a screenshot of the test coverage showing the lines were executed by the tests.
image
image

Attach a screenshot showing the tests that cover the change passing during CI
image

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.ts> showing fewer reported issues after the changes.
image

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.

1 participant