Skip to content

chore: add select state to radio buttons and only show inputs on enter - #2190

Merged
nborges-aws merged 3 commits into
refactorfrom
polish/select-state
Sep 3, 2026
Merged

chore: add select state to radio buttons and only show inputs on enter#2190
nborges-aws merged 3 commits into
refactorfrom
polish/select-state

Conversation

@nborges-aws

@nborges-aws nborges-aws commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds polishing changes to the formRadioButton component and two of its call paths:

  • harness -> create -> model
  • memory -> record -> list

The two changes are as follows:

1. Input fields:

  • Previously appeared on hover, which added visual noise to the screen
  • It was unclear to the user that they must click ENTER in order to advance
  • Solution: hide inputs until user hits SELECT

2. Select state:

  • Previously, there was a hover state on the models, but no select state
  • This was another contributing factor to confusion on advancing from the user
  • Solution: add distinct select state, which appears after user hits ENTER on the selected option

PR also updates the tests for both surface areas to exercise this new behavior. Additionally, removed duplicate import which was causing typecheck to fail (fix unrelated to commits in this PR)

Screen.Recording.2026-09-03.at.3.09.46.PM.mov

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe): Polishing chore

Testing

How have you tested the change?

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@github-actions github-actions Bot added the size/m PR size: M label Sep 3, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Sep 3, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 3, 2026
@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.80952% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.16%. Comparing base (a665955) to head (5b1725a).

Files with missing lines Patch % Lines
src/handlers/project/create/screen.tsx 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           refactor    #2190   +/-   ##
=========================================
  Coverage     97.15%   97.16%           
=========================================
  Files           542      542           
  Lines         37728    37754   +26     
=========================================
+ Hits          36653    36682   +29     
+ Misses         1075     1072    -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AgentCore Harness Review

Verdict: Changes requested

The memory-record scope screen tests aren't updated for the new "enter to edit" flow

The refactor in src/handlers/memory/record/list/screen.tsx gates the FormTextInput (and key.return submission) behind an editing state that must be toggled on with enter before any text is accepted. The existing tests in src/handlers/memory/record/record.screen.test.tsx still assume the old flow (type, then press enter to submit), so they should now fail:

  • unwinds the record table… (lines 74–96): screen.write("/customers/acme") runs while editing=false, so the characters are dropped (the useInput handler for the non-editing branch only reacts to escape/up/down/return, and no FormTextInput is mounted to capture them). The subsequent press("return") just flips editing=true with an empty scope, so navigation to the record table never occurs and the waitForText("Customer prefers email notifications.") will time out.
  • uses namespace scope again after moving the selector up (lines 98–118): same problem — write before entering editing mode is a no-op, and the following return only enters editing.
  • submits a namespace prefix and calls listMemoryRecords with exact options (lines 120–154): same problem — listMemoryRecords is never invoked.
  • maps namespace-path scope to namespacePath (lines 175–201): same problem.
  • requires a non-empty namespace value (lines 316–323): the single press("return") only enters editing mode; submitted stays false, so A namespace value is required. never renders.

Fix options:

  1. Update each affected test to press("return") (to enter editing) before write(...), and add a second press("return") for the empty-namespace test.
  2. Rework the screen so typing on the radio step implicitly enters editing (i.e., forward text keys into the field), which would keep the existing tests valid.

Please pick one and get the suite green before merging. Everything else in the diff (the FormRadioGroup focused/selected split, the HarnessWizard field gating, and the project-create call-site renames) looks consistent with the rest of the codebase.

@agentcore-devx-automation agentcore-devx-automation Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Sep 3, 2026
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Sep 3, 2026
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Sep 3, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Sep 3, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 3, 2026

@notgitika notgitika left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nborges-aws
nborges-aws merged commit ef32ae3 into refactor Sep 3, 2026
22 checks passed
@nborges-aws
nborges-aws deleted the polish/select-state branch September 3, 2026 20:58
This was referenced Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants