Skip to content

Add full-name tooltips for columns#29

Merged
joce merged 1 commit intomasterfrom
feature/columns-tooltip
Jan 6, 2026
Merged

Add full-name tooltips for columns#29
joce merged 1 commit intomasterfrom
feature/columns-tooltip

Conversation

@joce
Copy link
Owner

@joce joce commented Jan 6, 2026

  • Add full-name metadata for quote columns and use it for header/list tooltips in the watchlist and column chooser.
  • Populate full column names in the quote definitions (shortened wording like β€œDay Low/Day High”).
  • Update tests to cover full-name defaults and tooltip behavior.

@claude
Copy link

claude bot commented Jan 6, 2026

Claude finished @joce's task in 1m 27s β€”β€” View job


Code Review Complete βœ“

I've completed a comprehensive review of your PR. Overall, the implementation is well-structured and follows the project's coding standards. Here are my detailed findings:


Summary

This PR successfully adds tooltip support for column headers and list items by introducing a full_name property to column metadata. The implementation is clean, well-tested, and consistent with the existing codebase patterns.

Verdict: βœ… Approved - This is high-quality code that's ready to merge.


Detailed Review

1. Code Quality βœ…

Strengths:

  • Clean separation of concerns with protocol-based design in column_protocols.py
  • Consistent naming conventions following Google Python Style Guide
  • Proper use of type hints throughout
  • Good use of default values with the __post_init__ pattern in EnhancedColumn
  • Clear docstrings following Google style

Observations:

  • The full_name property defaults gracefully to label when not provided (enhanced_data_table.py:156)
  • The tooltip logic properly handles None cases and boundary checks (enhanced_data_table.py:568-575)

2. Security βœ…

No security concerns identified. This PR only adds display-related functionality (tooltips) and doesn't:

  • Handle user input
  • Interact with external systems
  • Modify authentication/authorization logic
  • Introduce XSS, injection, or other OWASP risks

3. Performance βœ…

No performance concerns. The changes are minimal and efficient:

  • Tooltip updates happen on hover events (already throttled by Textual framework)
  • No additional loops or expensive operations
  • The full_name property is stored, not computed on-the-fly
  • Column lookups use direct indexing (O(1) access)

4. Testing βœ…

Excellent test coverage:

  1. Unit Tests (test_quote_table_data.py):

    • Tests default behavior (full_name defaults to label)
    • Tests explicit full_name values are preserved
    • Good coverage of edge cases
  2. Integration Tests (test_enhanced_data_table.py):

    • Tests tooltip setting/clearing on hover (lines 204-224)
    • Tests tooltip behavior during ordering mode
    • Comprehensive coverage of the tooltip lifecycle
  3. UI Tests (test_column_chooser_screen.py):

    • Tests tooltip on frozen labels (line 82, 198)
    • Tests tooltip on list items (line 172, 298)
    • Verifies tooltips use full_name correctly

Minor observation: All tests follow the project convention with clear docstrings and proper markers (@pytest.mark.ui, @pytest.mark.integration).

5. Documentation βœ…

Well-documented:

  • The full_name property has clear docstrings in both ColumnMetadata protocol (column_protocols.py:26-28) and EnhancedColumn (enhanced_data_table.py:135-136)
  • The PR description clearly explains the feature
  • Inline comments are present where needed (e.g., enhanced_data_table.py:156 explains the default behavior)
  • The quote_column function documents the full_name parameter (quote_table.py:45-46)

Code-Specific Observations

enhanced_data_table.py

  • Line 155-156: The __post_init__ logic correctly defaults full_name to label when empty. This is a good pattern.
  • Lines 561-575: The _set_header_tooltip method is well-implemented with proper boundary checks and fallback logic.
  • Line 273: Good use of early return to disable tooltips in ordering mode.

column_chooser_screen.py

  • Lines 78-83: Frozen labels properly get tooltips assigned during initialization.
  • Line 298: List items correctly use full_name for tooltips.

quote_column_definitions.py

  • Lines 207-394: All column definitions now include meaningful full_name values. The naming is consistent and descriptive.
  • Examples: "Ticker Symbol" (line 207), "Market Price" (line 219), "52-Week High" (line 318)

quote_table.py

  • Lines 67-68: The factory function correctly handles the full_name parameter with a sensible default.

Compliance with AGENTS.md

Checking against the project's coding standards:

  • βœ… Type hints: All functions have proper type annotations
  • βœ… Docstrings: Google-style docstrings present for all public APIs
  • βœ… Python 3.10+ compatibility: Uses from __future__ import annotations
  • βœ… Naming conventions: Follows Google Python Style Guide
  • βœ… Testing: Comprehensive pytest coverage with proper markers
  • βœ… No mutable defaults: No issues found
  • βœ… Textual patterns: Properly integrated with Textual's reactive system

Recommendations

None. This PR is excellent as-is. If I had to be extremely nitpicky, I could suggest:

  1. Optional enhancement (not required): Consider adding a tooltip preview in the PR description screenshot to showcase the feature visually.
  2. Documentation: The implementation is self-documenting, but if this project maintains a user guide, you might want to mention this feature there.

These are very minor suggestions and don't block merging.


Final Assessment

Status: βœ… APPROVED

This is high-quality, production-ready code that:

  • Follows all project conventions
  • Has excellent test coverage
  • Is properly documented
  • Introduces no security or performance concerns
  • Implements the feature cleanly and maintainably

Great work! πŸŽ‰

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.
βœ… All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/appui/enhanced_data_table.py 88.23% 1 Missing and 1 partial ⚠️

πŸ“’ Thoughts on this report? Let us know!

@joce joce merged commit bdf2030 into master Jan 6, 2026
15 checks passed
@joce joce deleted the feature/columns-tooltip branch January 15, 2026 02:30
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