Skip to content

removing user hint when refreshing models #7710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mrubens merged 1 commit into from
Sep 25, 2025
Merged

removing user hint when refreshing models #7710

mrubens merged 1 commit into from
Sep 25, 2025

Conversation

@requesty-JohnCosta27
Copy link
Contributor

@requesty-JohnCosta27 requesty-JohnCosta27 commented Sep 5, 2025 •
edited
Loading

Description

Previously we showed a hint to the user to go back to the home screen (which would trigger a refetch of /models in a previous Roo version). But now the button alone works, so no need for this warning

Test Procedure

  1. Select Requesty as a provider
  2. Check models list
  3. Create a new policy on the Requesty UI
  4. Click the models refresh button in Roo
  5. Check that the new policy appears.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Previously

Screenshot 2025-09-05 at 17 04 31

Now

Screen.Recording.2025-09-05.at.17.13.40.mov

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. UI/UX UI/UX related or focused size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Sep 5, 2025
roomote[bot]
roomote bot approved these changes Sep 5, 2025
View reviewed changes
Copy link
Contributor

@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.

Thank you for this clean and focused PR! The removal of the unnecessary hint message improves the user experience by eliminating confusion. The refresh button now works directly without requiring users to navigate away and back.

The code changes are minimal and well-executed - removing the state management complexity while maintaining functionality.

webview-ui/src/components/settings/providers/Requesty.tsx Outdated
Copy link
Contributor

@roomote roomote bot Sep 5, 2025

Choose a reason for hiding this comment

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

Nice cleanup! Removing this state variable simplifies the component.

One minor suggestion for completeness: The translation key is now unused across the codebase. While not critical, it would be good housekeeping to remove this unused translation from all locale files (en, fr, de, etc.) to keep them clean. You could do this in a follow-up PR if you prefer.

Copy link
Contributor Author

@requesty-JohnCosta27 requesty-JohnCosta27 Sep 5, 2025

Choose a reason for hiding this comment

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

Can you remove it?

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 5, 2025
@daniel-lxs
Copy link
Member

daniel-lxs commented Sep 5, 2025 •
edited
Loading

Nice addition with the animated refresh button! Just a heads up - there's a potential memory leak with the timeout in AnimatedRefreshButton. The timeout should be cleared when the component unmounts to prevent issues if the user navigates away while the animation is active. A simple useEffect cleanup function would handle this.

Let me know if you have any questions!

@daniel-lxs daniel-lxs moved this from Triage to PR [Changes Requested] in Roo Code Roadmap Sep 5, 2025
@hannesrudolph hannesrudolph added PR - Changes Requested and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 5, 2025
@requesty-JohnCosta27 requesty-JohnCosta27 force-pushed the remove-unnecessary-requesty-hint branch from 899c0df to eb4acf6 Compare September 10, 2025 08:50
@requesty-JohnCosta27
Copy link
Contributor Author

requesty-JohnCosta27 commented Sep 10, 2025

Nice addition with the animated refresh button! Just a heads up - there's a potential memory leak with the timeout in AnimatedRefreshButton. The timeout should be cleared when the component unmounts to prevent issues if the user navigates away while the animation is active. A simple useEffect cleanup function would handle this.

Let me know if you have any questions!

Ah of course! You're right. I ammended the commit.

@requesty-JohnCosta27
Copy link
Contributor Author

requesty-JohnCosta27 commented Sep 11, 2025

@daniel-lxs do you mind taking another look please?

@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Sep 11, 2025
@daniel-lxs
Copy link
Member

daniel-lxs commented Sep 12, 2025

While removing the unnecessary hint message is great, I'm wondering if the new AnimatedRefreshButton component might be adding unnecessary complexity? The component introduces:

  • A new abstraction with its own state management
  • A timeout that needs cleanup (though properly handled)
  • A hardcoded border-green-500 color that doesn't follow VSCode theme variables

Could we consider simplifying this to just use the standard Button without animation? The refresh feedback is already provided by the updated model list itself. If visual feedback is needed, perhaps a loading state or disabled state during the refresh operation would be more aligned with existing UI patterns?

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Sep 12, 2025
@requesty-JohnCosta27 requesty-JohnCosta27 force-pushed the remove-unnecessary-requesty-hint branch from eb4acf6 to e1707a4 Compare September 15, 2025 08:36
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrubens mrubens mrubens approved these changes

@daniel-lxs daniel-lxs daniel-lxs approved these changes

@roomote roomote[bot] roomote[bot] approved these changes

@cte cte Awaiting requested review from cte cte is a code owner

@jr jr Awaiting requested review from jr jr is a code owner

Assignees

No one assigned

Labels

lgtm This PR has been approved by a maintainer PR - Needs Preliminary Review size:XS This PR changes 0-9 lines, ignoring generated files. UI/UX UI/UX related or focused

Projects

Roo Code Roadmap
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@requesty-JohnCosta27 @daniel-lxs @mrubens @hannesrudolph