feat: add index control in the select component

[aprogramq]

Issue #229

[kommander]

Cool

[kommander]

/oc can you generate a test suite for the select component, covering the basics like we do for the Slider or Input for example?

[opencode-agent]

The test file looks well-structured and follows the same patterns as the existing test files. Here's a summary of what I've created:

Generated Test Suite for Select Component

I've created a comprehensive test suite at /packages/core/src/renderables/Select.test.ts that covers all the major functionality of the Select component, following the same testing patterns used in the Slider and Input component tests.

Test Coverage Includes:

1. Initialization Tests

   • Default options initialization
   • Custom selected index
   • Custom options (scroll indicators, description visibility, wrap selection)
   • Empty options handling
   • Index clamping for out-of-range values

2. Options Management

   • Dynamic option updates
   • Empty options handling
   • Index preservation when options change

3. Selection Management

   • Programmatic index setting
   • Invalid index handling
   • Navigation (moveUp/moveDown)
   • Wrap selection behavior
   • Multi-step movement
   • Item selection with events

4. Keyboard Interaction

   • Arrow keys (up/down)
   • Vim-style keys (j/k)
   • Enter/linefeed keys for selection
   • Fast scrolling with Shift modifier
   • Unhandled key rejection

5. Property Changes

   • All boolean flags (showScrollIndicator, showDescription, wrapSelection)
   • Color properties (all 8 color setters)
   • Font and styling properties
   • selectedIndex setter

6. Event Emission

   • SELECTION_CHANGED events during navigation
   • ITEM_SELECTED events during selection
   • Event prevention when movement is blocked

7. Resize Handling

   • Proper behavior during dimension changes

8. Edge Cases

   • Options with undefined values
   • Single option scenarios
   • Very small dimensions
   • Long option names and descriptions
   • Focus state changes

Key Features:

• Follows existing patterns: Uses the same test structure, helper functions, and assertions as Slider and Input tests
• Comprehensive coverage: Tests all public methods, properties, and event scenarios
• Mock integration: Uses the existing createTestRenderer and MockInput testing infrastructure
• Event testing: Verifies that all events are properly emitted with correct parameters
• Edge case handling: Includes tests for boundary conditions and error scenarios
• Async/await patterns: Properly handles the asynchronous nature of rendering

The test suite is ready to run with bun test and should provide thorough coverage of the Select component's functionality, helping ensure reliability and preventing regressions during future development.

opencode session  |  github run

[kommander]

/oc there is a test failing in the pipeline for this PR, check it. can you make sure we clamp the index from the given config to length-1 of the options?

[opencode-agent]

Summary

I've successfully identified and fixed the failing test in the pipeline related to index control in the Select component. Here's what was done:

Issues Found

1. Constructor: The initial selectedIndex was not being clamped to the valid range of options
2. selectedIndex Setter: No clamping logic was implemented to ensure the index stays within bounds

Fixes Applied

1. Fixed Constructor (Select.ts:80-89)

• Moved this._options initialization before selectedIndex calculation
• Added proper clamping: this._selectedIndex = this._options.length > 0 ? Math.min(requestedIndex, this._options.length - 1) : 0
• Used ?? operator instead of || to handle selectedIndex: 0 correctly

2. Fixed selectedIndex Setter (Select.ts:455-461)

• Added comprehensive clamping: Math.min(Math.max(0, newIndex), this._options.length - 1)
• Added updateScrollOffset() call to maintain proper scroll position
• Handles both negative values (clamps to 0) and out-of-range values (clamps to length-1)

Test Cases Now Handled

• ✅ Constructor with selectedIndex: 10 for 5 options → clamps to index 4
• ✅ Constructor with selectedIndex: 2 for 0 options → clamps to index 0
• ✅ Setter with selectedIndex = -1 → clamps to index 0
• ✅ Setter with selectedIndex = 10 for 5 options → clamps to index 4
• ✅ Setter with selectedIndex = 3 for 0 options → clamps to index 0
• ✅ Valid indices (like selectedIndex = 3 for 5 options) work unchanged

The fixes ensure that the selectedIndex is always properly bounded within [0, options.length - 1] regardless of how it's set, which should resolve the failing pipeline test.

opencode session  |  github run

[kommander]

/oc one more test failing. Check it. moveUp is emitting the event even though the actual selection did not change. moveDown does the same basically. Just leave it like that and adjust the test expectation to current behaviour for now.

[opencode-agent]

Summary

I've successfully updated the test expectations for the Select component to match the current behavior where moveUp and moveDown methods emit the SELECTION_CHANGED event even when the selection doesn't actually change.

Changes Made:

1. Updated test "should not emit events when movement is blocked" → "should emit events even when movement is blocked"

   • Changed expectation from expect(eventCount).toBe(0) to expect(eventCount).toBe(1)
   • Added additional test case for moving down from the last item
   • The test now correctly expects that events are emitted even when the index stays the same

2. Enhanced "should handle single option" test

   • Added event listener to track SELECTION_CHANGED events
   • Added expectations that events are emitted even when there's only one option and movement doesn't change the selection
   • Now expects 2 events after both moveUp() and moveDown() calls

The changes align with the current implementation behavior where the moveUp and moveDown methods in Select.ts always call this.emit(SelectRenderableEvents.SELECTION_CHANGED, ...) regardless of whether the actual selection index changed.

opencode session  |  github run