Skip to content

Conversation

@diarmidmackenzie
Copy link
Contributor

@diarmidmackenzie diarmidmackenzie commented Apr 3, 2025

Context

See issue 1 in this discussion: #1358

When "ask followup question" tool is invoked, Roo grabs focus away from whateever the user is doing which is annoying.

Implementation

A localized fix for the "ask followup question" tool, for reasons explained here: #1358 (comment)

Product code change was simple. Getting a Unit Test that reproduced the failure with the old code change proved rather tricky, but I got there in the end.

Screenshots

N/A

How to Test

Ask Roo to ask you a follow-up question (doesn't matter what).
Move your focus aay from the text area - e.g. start typing into another file.
Observe that when the follow up question is ready, focus does not get grabbed.

A couple of other focus-grabs that remain (I took care not to break these).

  • Switching into the Roo extension context from another extension
  • Switching to the Roo chat tab from another tab.

-->

Get in Touch

diarmid/diarmidm on Discord.


Important

Fixes focus grabbing issue in ChatView.tsx for follow-up questions and adds a unit test to verify the behavior.

  • Behavior:
    • Fixes focus grabbing issue in ChatView.tsx when a follow-up question is presented by setting setEnableButtons(true) to prevent focus change.
  • Testing:
    • Adds a unit test in ChatView.test.tsx to verify that focus is not grabbed when a follow-up question is presented.
    • Uses mockFocus to track focus calls and ensures it remains unchanged after follow-up questions.

This description was created by Ellipsis for cf6bb38. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented Apr 3, 2025

⚠️ No Changeset found

Latest commit: cf6bb38

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@diarmidmackenzie diarmidmackenzie marked this pull request as ready for review April 3, 2025 16:03
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Apr 3, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 3, 2025
@mrubens mrubens merged commit 43d1ab3 into RooCodeInc:main Apr 3, 2025
29 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Apr 3, 2025
@diarmidmackenzie
Copy link
Contributor Author

While this fixes the immediate issue, I have found some other related bugs, and a broader fix is probably warranted.

See notes here:
#2189 (comment)

In particular, ChatView needs a dedicated piece of state to record whether the TextArea is / is not focussed, because it can't reliably derive this from other state.

That will allow us to fix a range of related bugs in this area, as noted in #2189. I will pull together a further PR for this.

@libertyteeth
Copy link
Contributor

That will allow us to fix a range of related bugs in this area, as noted in #2189. I will pull together a further PR for this.

I mentioned this in #2189 (comment) , just wanted to note it here: I've worked on a fairly comprehensive state machine for maintaining focus.

It may be lacking a few edge cases? I put it here a few days ago, then got busy with life :)

https://github.com/libertyteeth/roo-fixes/pull/2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants