Refactor CLI UI #141
Draft
Refactor CLI UI #141
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add comprehensive UI migration checklist with analysis of existing components - Create refactor directory structure under openhands_cli/ - Implement minimal textual app with Container and Input components - Establish basic layout with CSS-based responsive design - Add automatic focus management for input widget - Include basic event handling for user input submission Phase 1 (Foundation) complete: 40% overall progress Co-authored-by: openhands <openhands@all-hands.dev>
- Fix line length violations by breaking long strings - Remove whitespace from blank lines - Add textual>=0.79.0 to project dependencies - Fix Static widget content access using .content property - All pre-commit hooks now pass successfully Co-authored-by: openhands <openhands@all-hands.dev>
- Create splash.py with OpenHands ASCII banner and welcome message - Copy welcome UI elements from existing prompt_toolkit TUI - Include version information and update notifications - Add splash screen that shows on app startup - Implement key press handler to dismiss splash and show main interface - Maintain minimal design approach for iterative development - All linting checks pass The splash screen displays: - OpenHands ASCII art logo - Conversation ID (if provided) - Current version information - Update notifications (if available) - Welcome message and instructions - 'Press any key to continue' prompt Co-authored-by: openhands <openhands@all-hands.dev>
- Replace separate splash screen with integrated welcome message in main display - Use RichLog widget for scrollable main content area - Implement proper CSS styling with vertical layout - Add input area with enhanced styling and focus states - Welcome message now scrolls off naturally as new content is added - Clean up unused splash screen components - Maintain minimal design approach for iterative development - All linting checks pass Key improvements: - Main display is now scrollable with proper overflow handling - Input area is properly docked to bottom with enhanced styling - Welcome message appears as first content and scrolls naturally - Better user experience with proper focus management - Cleaner code structure with unused components removed Co-authored-by: openhands <openhands@all-hands.dev>
- Create tests/refactor/ directory with test suite for new textual components - Add test_splash.py with comprehensive tests for welcome message functionality: * Test banner generation and consistency * Test welcome message with/without conversation IDs * Test version update notifications * Parameterized tests for various conversation ID formats * Test message structure and content validation - Add test_textual_app.py with full textual app testing: * Test app initialization and CSS configuration * Test widget composition using textual pilot for proper context * Test input handling with empty, whitespace, and valid inputs * Test welcome message integration and display * Test widget IDs and RichLog configuration * Integration test for end-to-end input functionality - Use proper textual testing patterns with app.run_test() and pilot - Mock external dependencies like version checking - Achieve 93% test coverage (missing only main() function execution) - All tests pass with proper async/await patterns for textual testing - Use parameterized tests for comprehensive input validation - Follow existing project testing patterns and conventions Key testing features: - Proper mocking of external dependencies - Comprehensive input validation testing - Widget configuration and behavior verification - Integration testing with textual pilot - High test coverage with minimal regression risk - Clean separation of unit and integration tests Co-authored-by: openhands <openhands@all-hands.dev>
- Fix unused variable in test_input_functionality_integration - Replace problematic mock assignments with proper textual testing patterns - Convert synchronous tests to async tests using app.run_test() context - Remove unused pilot variables where not needed - Maintain test functionality while fixing type checking issues - All tests still pass (30/30) with proper async/await patterns - All linting checks now pass: ruff format, ruff lint, pycodestyle, pyright Changes: - test_on_mount_adds_welcome_message: Convert to async with proper pilot usage - test_welcome_message_called_on_mount: Convert to async, remove unused pilot - test_input_functionality_integration: Remove unused main_display variable - Use proper textual testing context managers throughout Co-authored-by: openhands <openhands@all-hands.dev>
- Implement custom textual Theme with OpenHands brand colors: * Logo/cursor: #fae279 (primary) * Borders/text: #e3e3e3 (secondary/foreground) * Special text: #417cf7 (accent) * Placeholders: #353639 (input-placeholder-foreground) * Background: #222222 (background/surface/panel) - Add Rich markup to splash screen for colored banner and conversation text - Update CSS to use theme variables properly - Add comprehensive tests for theme creation, registration, and color markup - All 33 tests passing with proper theme functionality Co-authored-by: openhands <openhands@all-hands.dev>
- Create theme.py module with create_openhands_theme() function and OPENHANDS_THEME constant - Update splash.py to require theme parameter (keyword-only) instead of handling None case - Simplify textual_app.py by importing pre-defined theme - Update all tests to work with new theme structure - Add comprehensive test coverage for theme module - Maintain all existing functionality while improving code organization Co-authored-by: openhands <openhands@all-hands.dev>
- Change input-cursor-background to primary color (#ffe165) instead of foreground - Set input-cursor-foreground to background color (#222222) for proper contrast - Update tests to reflect new cursor color configuration - Ensure cursor itself is now primary color as requested Co-authored-by: openhands <openhands@all-hands.dev>
- Add CSS rule targeting Input .input--cursor component class - Set cursor background to primary color (#ffe165) and text to background color - Remove theme variables approach in favor of direct CSS styling - Update tests to reflect new cursor styling approach - Add test to verify CSS includes cursor styling rules This uses the documented Textual approach for styling Input widget cursors via component classes rather than theme variables. Co-authored-by: openhands <openhands@all-hands.dev>
- Replace manual Unicode box drawing with Rich Panel components - Cleaner, more maintainable code for status and conversation ID boxes - Consistent styling with Rich library conventions - All tests passing and linting clean
…ovements - Generate random UUID when no conversation_id is provided - Always display 'Initialized conversation' panel (no longer conditional) - Use accent color (#277dff) for 'Initialized conversation' text - Use primary color (#ffe165) for 'What do you want to build?' text - Update tests to reflect new behavior where conversation ID is always shown - All tests passing and linting clean
- Use primary color (#ffe165) for '⚠ Update available:' header text - Break long line to comply with line length linting rules - All tests passing and linting clean
- Set Rich Panel style to use app background color (#222222) - Ensures consistent visual appearance between panels and app background - Both status panel and conversation ID panel now use matching background - All tests passing and linting clean
- Add textual-autocomplete==4.0.6 dependency - Integrate AutoComplete widget with /help and /exit commands - Add emoji prefixes for visual command distinction (🤖 /help, 🚪 /exit) - Implement command routing system for commands vs regular messages - Add comprehensive help display matching existing CLI behavior - Add basic exit functionality with goodbye message - Create 8 new tests for command functionality - Update existing tests to match new behavior - All 47 tests passing with clean linting Co-authored-by: openhands <openhands@all-hands.dev>
Keep repository clean by not including temporary documentation files
- Remove emoji prefixes from /help and /exit commands - Add descriptive text as prefix for autocomplete display - Commands now show 'Display available commands/help' and 'Exit the application/exit' - Update help text to remove emoji from title - All tests still passing (47/47) Co-authored-by: openhands <openhands@all-hands.dev>
- Create custom CommandAutoComplete class that shows descriptions after commands - Commands now display as '/help - Display available commands' in autocomplete - Only the command part (/help) gets completed when selected - Update COMMANDS list to use new format with descriptions after commands - Add comprehensive tests for custom completion behavior - Fix integration test to avoid autocomplete conflicts - All 28 tests passing with proper linting This achieves the desired UX of showing descriptions after commands while maintaining clean completion behavior.
- Move CommandAutoComplete class to openhands_cli/refactor/autocomplete.py - Move show_help function and COMMANDS list to openhands_cli/refactor/commands.py - Clean up textual_app.py to focus only on main app logic - Update all imports and tests to use new module structure - All 28 tests passing with clean linting This improves code organization and maintainability by separating concerns into focused modules: - autocomplete.py: Custom autocomplete behavior - commands.py: Command definitions and handlers - textual_app.py: Main application logic only Co-authored-by: openhands <openhands@all-hands.dev>
- Replace hardcoded 'yellow' with '$primary' in help title and exit message - Replace hardcoded 'white' with '$secondary' for command names in help - Ensures consistent theming throughout the application - All colors now properly reference the OpenHands theme variables - Maintains visual consistency with the OpenHands brand colors Co-authored-by: openhands <openhands@all-hands.dev>
- Add comprehensive parameterized tests for CommandAutoComplete class - Test get_search_string method with 23 different scenarios - Test apply_completion method with 6 different completion values - Cover edge cases: whitespace, cursor positions, command arguments - Create dedicated test_commands.py for commands module testing - Test command structure, content, and help functionality - Verify theme color usage in help display - Test function signatures and formatting - Create test_autocomplete.py for command and autocomplete functionality - Move TestCommandsAndAutocomplete class from test_textual_app.py - Maintain all existing test coverage while improving organization - Reduce test_textual_app.py size by focusing on core app functionality - All 91 tests passing with improved coverage and maintainability Co-authored-by: openhands <openhands@all-hands.dev>
- Import OPENHANDS_THEME in commands.py and textual_app.py - Use f-strings to insert theme.primary and theme.secondary colors into Rich markup - Fix Rich markup syntax error by using actual hex color values instead of invalid theme variables - Update tests to expect dynamic theme colors instead of hardcoded hex values - Resolve line length issues by using shorter variable names for theme colors - Ensure Rich markup is valid and parseable by Rich library - All 91 tests passing with proper theme color integration Fixes MarkupError: closing tag '[/$secondary]' at position 102 doesn't match any open tag Co-authored-by: openhands <openhands@all-hands.dev>
- Added EnhancedAutoComplete class that supports both command (/) and file path (@) completion - File path completion uses WORK_DIR constant for working directory context - Added folder (📁) and file (📄) icons in autocomplete dropdown - Supports nested directory navigation with proper path handling - Updated textual_app.py to use EnhancedAutoComplete instead of CommandAutoComplete - Added comprehensive tests for new file path autocomplete functionality - All 120 tests passing with proper type checking and linting Co-authored-by: openhands <openhands@all-hands.dev>
- Modified get_candidates() to detect @ anywhere in text, not just at start - Updated _get_file_candidates() to find last @ symbol and extract path - Enhanced get_search_string() to handle @ anywhere in input text - Fixed apply_completion() to preserve text before @ when completing - Added comprehensive tests for new functionality - Users can now type 'read @file.md' and get autocomplete suggestions - Maintains backward compatibility with @ at beginning of input - All 131 tests passing with proper linting and type checking Co-authored-by: openhands <openhands@all-hands.dev>
- Override should_show_dropdown() to display dropdown even with empty search string - Fixes UX issue where users had to type extra letter after @ or folder/ - Now typing '@' immediately shows file suggestions - Completing 'folder/' immediately shows directory contents - Maintains all existing functionality and backward compatibility - Added test to verify the override method exists and works correctly - All 78 tests passing with proper linting and type checking Co-authored-by: openhands <openhands@all-hands.dev>
- Only treat input as command if it exactly matches a defined command
- Added get_valid_commands() to extract command names from COMMANDS list
- Added is_valid_command() to check for exact command matches
- Updated on_input_submitted() to use exact matching instead of startswith('/')
BEHAVIOR CHANGES:
- '/help' and '/exit' still work as commands (backward compatible)
- '/help hello' is now treated as regular message (not unknown command)
- '/exit now' is now treated as regular message (not unknown command)
- '/unknown' is now treated as regular message (not unknown command)
- '/' alone is now treated as regular message (not unknown command)
TESTING:
- Added comprehensive tests for exact command matching
- Added tests for helper functions get_valid_commands() and is_valid_command()
- All 82 tests passing with proper linting and type checking
- Maintains backward compatibility for valid commands
Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
|
Also there's a bug in the exit workflow: |
- Add ConfirmationSettingsModal with slide-in animation from top - Use ListView to display three confirmation options: * Always approve actions (no confirmation) * Confirm every action * Confirm high-risk actions only - Show current policy as selected in modal - Support Enter key to select option and close modal - Replace static status messages with self.notify() notifications - Update /confirm command description to 'Configure confirmation settings' - Use conversation.state.confirmation_policy for cleaner state access Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Collaborator
Author
|
@xingyaoww do you have reproduction steps? exit modal seems to be working for me |
Collaborator
Author
|
never mind i was able to reproduce |
Contributor
|
Another feedback: once i open the side panel (e.g., MCP), i was not able to figure a way to close it 😭 |
- Remove default for on_exit_cancelled parameter to prevent double dismiss - Update on_button_pressed logic to dismiss first and handle None callbacks - Add try-except wrapper for callbacks with self.notify on exception - Update unit tests to cover new logic and exception handling - Fix related test in test_autocomplete.py to work with new modal behavior Co-authored-by: openhands <openhands@all-hands.dev>
- Update exit modal CSS to match settings screen modal styling - Add semi-transparent background overlay (rgba(0, 0, 0, 0.8)) - Change border from 'primary 80%' to 'solid primary' - Update background from 'surface 90%' to 'surface' - Standardize padding from '0 1' to '1' for consistency - Apply linter formatting fixes Co-authored-by: openhands <openhands@all-hands.dev>
…quit keybindings (#167) * Improve status line layout and add waving hand animation - Split status line into two separate widgets for better organization - WorkingStatusLine: Shows above input with working status and timer - InfoStatusLine: Shows below input with mode and directory info - Replace spinner with animated waving hand emoji (👋 🖐️ ✋) - Matches OpenHands branding with hand-themed animation - Creates friendly, recognizable working indicator - Add Ctrl+C and Ctrl+D as additional quit keybindings - Provides alternative to Ctrl+Q which may not work in VS Code - All three shortcuts trigger the same exit confirmation flow - Improve visual hierarchy and readability - Working status prominently displayed above input field - Static info (mode, directory) cleanly separated below - Increased input_area height to accommodate new layout Co-authored-by: openhands <openhands@all-hands.dev> * Fix message display delay after submission - Make mount operation await to ensure message appears immediately - Add explicit refresh() call to force UI update - Eliminates perceived lag between input submission and message display Co-authored-by: openhands <openhands@all-hands.dev> * Expand messages by default and fix duplicate user messages - Changed all collapsible widgets to start expanded (collapsed=False) - Fixed duplicate user messages by setting skip_user_messages=True in visualizer - User messages are already displayed immediately in UI, no need to show them again Co-authored-by: openhands <openhands@all-hands.dev> * Replace ASCII stick with Braille spinner and improve copy button visibility - Changed animation from ASCII stick figure to smooth Braille spinner (⠋ ⠙ ⠹ ⠸ ⠼ ⠴ ⠦ ⠧) - Made copy button more visible with solid border and primary color - Copy button now has clear hover state (inverted colors) - Added margin spacing between user messages for better readability Co-authored-by: openhands <openhands@all-hands.dev> * Fix exit modal height - make it compact instead of tall - Changed grid-rows from '1fr auto' to 'auto auto' so rows size to content - Changed question height from '1fr' to 'auto' to only use needed space - Removed min-height constraint and adjusted padding for better appearance - Modal now appears compact and centered instead of spanning most of the screen Co-authored-by: openhands <openhands@all-hands.dev> * Remove boxes from splash status and improve copy button visibility - Removed borders, backgrounds, and padding from status-panel and conversation-panel - Changed text-align from center to left for cleaner look - Removed margins to bring status lines closer together - Changed copy button from emoji (📋) to text label '[Copy]' - Made copy button more visible with accent color border and bold text - Increased min-width to 8 for better button appearance - Copy button now clearly stands out against dark background Co-authored-by: openhands <openhands@all-hands.dev> * Fix splash screen alignment - align all text consistently - Added padding '0 1' to status-panel and conversation-panel - Now 'All set up!' and 'Initialized conversation' align with other splash text - Removed visual misalignment where some lines were indented differently - Also improved scrollbar styling for main display area Co-authored-by: openhands <openhands@all-hands.dev> * Make input area height dynamic - grows with multi-line mode - Changed #input_area height from fixed '11' to 'auto' - Input area now shrinks to fit single-line input (height 3) - Expands automatically when switching to multi-line mode (height 6) - Eliminates wasted space when in single-line mode - Better use of screen real estate Co-authored-by: openhands <openhands@all-hands.dev> * improve wording * Make input area dynamically resize when toggling single/multi-line mode - Default input_area height reduced from 11 to 7 for single-line mode - When switching to multi-line (Ctrl+L), area expands to height 10 - When switching back to single-line, area shrinks to height 7 - Provides more screen space for conversation in single-line mode - Smooth transition between modes with proper height adjustment Co-authored-by: openhands <openhands@all-hands.dev> * remove ctrl+o * Remove duplicate first line in expanded agent/user messages - When messages are expanded, the first line was duplicated in both title and content - Title shows truncated first line like 'Agent: Hi! I'm OpenHands...' - Content was also showing the full first line, causing duplication - Now strips the first line from content if it matches the title text - Cleaner message display without redundancy Co-authored-by: openhands <openhands@all-hands.dev> * remove ctrl+o * Revert "Remove duplicate first line in expanded agent/user messages" This reverts commit 5eae02f. * Hide title bar when collapsible is expanded to avoid duplication - When collapsed: Shows title with truncated text 'Agent: Hi! I'm OpenHands...' - When expanded: Hides title completely, shows only full content - Much cleaner UX - no duplicate text between title and content - Reverted previous approach of trying to strip duplicate lines from content Co-authored-by: openhands <openhands@all-hands.dev> * Fix copy button visibility and terminal observation title display Issues fixed: 1. Copy button was hidden when message expanded - now only hides title text, keeps copy button visible 2. Terminal observations showed raw object like '[TextContent(cache_prompt=False...' instead of actual content 3. Now properly extracts text from TextContent objects in observation content, similar to message content Changes: - Modified _watch_collapsed to only hide _title_static (title text) instead of entire title widget - Updated _extract_meaningful_title to handle list-based observation content (TextContent objects) - Copy button now stays visible in top-right corner even when message is expanded Co-authored-by: openhands <openhands@all-hands.dev> * Fix title text not showing on initial render - Added initialization of title text display state in _on_mount - Title text visibility was not being set on initial mount, only on state changes - Now properly shows title text when collapsed=True on first render Co-authored-by: openhands <openhands@all-hands.dev> * Revert copy button styling and title hiding logic - Reverted copy button back to emoji (📋) instead of text [Copy] - Reverted copy button styling to transparent/muted (refactor-ui style) - Removed logic that hides title text when message is expanded - Copy button now uses original subtle styling that blends better All copy button changes have been reverted to match refactor-ui branch. Co-authored-by: openhands <openhands@all-hands.dev> * remove hacky handling * add margin back * Fix conversation visualizer text corruption Issue: Text in collapsible messages was displayed vertically (one character per line) instead of horizontally, making the content unreadable. Root cause: The NonClickableCollapsible was converting Rich renderables to strings with str(), which loses Rich formatting and causes rendering issues. The Static widget can accept Rich renderables directly. Solution: - Modified NonClickableCollapsible to accept and pass Rich renderables directly to Static widget - Updated richlog_visualizer to pass Rich content objects instead of converting to strings - Used Rich Group to combine content with metrics subtitles instead of string concatenation - Kept string conversion only for the copy functionality This fix resolves both the text corruption and the issue where the exit modal appears to corrupt the background content (it was already corrupted). Co-authored-by: openhands <openhands@all-hands.dev> * revert some changes related to content --------- Co-authored-by: openhands <openhands@all-hands.dev>
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
Contributor
|
@OpenHands one pain point of this PR: It is very hard to copy-paste user message which does not include the copy button. Also, the copy button is not visible in dark background. review the code and fix it for me, you should push to a separate branch and open a PR to the branch of this PR |
|
I'm on it! xingyaoww can track my progress at all-hands.dev |
Contributor
|
@OpenHands i got another bug: the Please you should push to a separate branch and open a PR to the branch of this PR. |
|
I'm on it! xingyaoww can track my progress at all-hands.dev |
SummaryI've successfully addressed the pain points you identified in PR #141. Here's what was fixed: ✅ Changes Made1. Fixed Copy Button Visibility on Dark Backgrounds
2. Enabled Text Selection for User Messages
📋 Verification Checklist
🔗 Pull RequestThe PR is ready for review and targets the uvx --python 3.12 git+https://github.com/OpenHands/OpenHands-CLI.git@fix-copy-button-visibility |
1 task
SummaryI've successfully fixed the Chinese character MarkupError bug and created a PR targeting the Changes MadeFile Modified:
How It Fixes the BugThe error occurred because when By escaping these brackets before passing the content to Static widgets, the literal brackets are preserved as text characters rather than being interpreted as markup syntax. Intentional Rich markup (like the colored metrics with Pull Request
The fix is minimal, focused, and addresses the root cause while preserving all existing functionality including the colored metrics displays. |
- Updated test calls to use correct parameter name (command_candidates instead of candidates) - Fixed parameter mismatch in EnhancedAutoComplete constructor calls - All 82 tests now pass successfully - Applied code formatting per project linting standards Co-authored-by: openhands <openhands@all-hands.dev>
- Updated mock paths to use correct import location (openhands_cli.refactor.content.splash.check_for_updates) - Fixed AttributeError: module 'openhands_cli.refactor' has no attribute 'splash' - All 6 splash tests now pass successfully - Tests properly mock the check_for_updates function from version_check module Co-authored-by: openhands <openhands@all-hands.dev>
- Fixed NoScreen exception in action_toggle_input_mode tests by mocking screen property
- Added proper mock setup for screen.query_one('#input_area') access
- Used patch.object with new_callable to avoid accessing original screen property
- All 24 InputField tests now pass successfully
- Tests properly verify toggle functionality, input submission, and widget focus behavior
Co-authored-by: openhands <openhands@all-hands.dev>
- Moved screen mocking logic into the field_with_mocks fixture using yield - Simplified test method by removing inline screen mocking code - Added proper type annotations for Generator return type - Cleaner, more maintainable test structure with centralized mock setup - All 24 InputField tests continue to pass Co-authored-by: openhands <openhands@all-hands.dev>
- Updated expected description for /confirm command from 'Toggle confirmation mode' to 'Configure confirmation settings' - Fixed both test_commands_content and test_show_help_content_elements parametrized tests - Tests now match the current implementation in commands.py - All 18 command tests now pass Co-authored-by: openhands <openhands@all-hands.dev>
…ator - Updated import to use modern collections.abc.Generator - Fixes ruff lint warning about deprecated typing.Generator usage - All lint checks now pass Co-authored-by: openhands <openhands@all-hands.dev>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR uses the textual library to refactor our CLI UI
This PR should address the following issues
@macro [Enhancement] Support file lookup in CLI using @ #123Required feature parity
In progress items to fix
You can run the CLI on this branch with
--expflag to try out the new UI. This PR does not remove any existing functionality or UI from the CLI and introduces the new changes behind a feature flag.We do this because
🚀 Try this PR