Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Sep 11, 2025

Summary

This PR attempts to address Issue #7882 by implementing auto-scroll functionality for the mode selector dropdown.

Problem

When opening the mode selector dropdown, the list always started at the top, requiring users to manually scroll to find the currently active mode, especially problematic when the active mode was far down the list.

Solution

Added scroll-to-center logic that automatically centers the active mode in the viewport when the dropdown opens:

  • Added refs to track the selected item and scroll container
  • Implemented scroll positioning calculation in useEffect hook
  • The selected mode now appears centered in the visible area

Changes

  • Modified webview-ui/src/components/chat/ModeSelector.tsx:
    • Added selectedItemRef and scrollContainerRef refs
    • Enhanced the existing useEffect to include scroll-to-center logic
    • Added conditional ref assignment to the selected mode item
    • Set scroll behavior to 'instant' for immediate positioning

Testing

  • ✅ All existing tests pass
  • ✅ Linting and type checking pass
  • ✅ Manual testing confirms the active mode is centered when dropdown opens
  • ✅ Search functionality remains unaffected

Review Confidence

Code review completed with 95% confidence - implementation is clean and follows existing patterns.

Fixes #7882

Feedback and guidance are welcome!


Important

Centers active mode in ModeSelector.tsx dropdown on open using scroll-to-center logic.

  • Behavior:
    • Centers active mode in dropdown on open in ModeSelector.tsx.
    • Uses selectedItemRef and scrollContainerRef to track elements.
    • Implements scroll-to-center logic in useEffect.
  • Implementation:
    • Adds selectedItemRef and scrollContainerRef refs.
    • Modifies useEffect to calculate and set scroll position.
    • Sets scroll behavior to 'instant'.
  • Testing:
    • All existing tests pass.
    • Manual testing confirms active mode is centered on open.

This description was created by Ellipsis for 268e6c5. You can customize this summary. It will automatically update as commits are pushed.

- Added refs to track selected item and scroll container
- Implemented scroll-to-center logic when popover opens
- Selected mode now appears centered in viewport instead of list starting at top
- Fixes issue where users had to manually scroll to find active mode

Fixes #7882
@roomote roomote bot requested review from cte, jr and mrubens as code owners September 11, 2025 04:24
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. UI/UX UI/UX related or focused labels Sep 11, 2025
@hannesrudolph hannesrudolph moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Sep 11, 2025
Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.

const itemHeight = item.offsetHeight

// Center the item in the container
const scrollPosition = itemTop - containerHeight / 2 + itemHeight / 2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be valuable to add test coverage for this scroll-to-center behavior? I notice the existing tests don't verify that the selected item is actually scrolled into view when the dropdown opens.

const item = selectedItemRef.current

// Calculate positions
const containerHeight = container.clientHeight
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider extracting this scroll calculation logic into a utility function like scrollItemToCenter(container, item) for better testability and reusability. This would also make the component logic cleaner.

- Replace setTimeout with requestAnimationFrame for more reliable DOM synchronization
- Add proper boundary checking to prevent scrolling past container limits
- Ensure scroll position stays within valid bounds using Math.min/max
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Sep 11, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 11, 2025
@mrubens mrubens merged commit a9e22f0 into main Sep 11, 2025
15 checks passed
@mrubens mrubens deleted the fix/mode-selector-scroll-to-active branch September 11, 2025 19:16
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Sep 11, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer PR - Needs Review size:M This PR changes 30-99 lines, ignoring generated files. UI/UX UI/UX related or focused

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Chat mode selector: center the active mode in view on open

5 participants