-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fixed auto question timer unmount #5368
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.
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
Conversation
|
✅ No security or compliance issues detected. Reviewed everything up to 2d49c16. Security Overview
Detected Code Changes
Reply to this PR with |
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @liwilliam2021, I left a couple of suggestions that might be worth taking a look at.
Let me know what you think!
webview-ui/src/components/chat/__tests__/FollowUpSuggest.spec.tsx
Outdated
Show resolved
Hide resolved
|
@daniel-lxs nice comments! Fixed! |
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @liwilliam2021, thank you for addressing the earlier suggestions. I really appreciate the updates.
Looking at the latest implementation, I noticed a couple of new things worth tweaking based on the recent changes. Everything’s looking solid overall, just a few small follow-ups.
Let me know what you think!
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything from my previous feedback looks good!
One small suggestion: in FollowUpSuggest.tsx, the onUnmount prop is used both during unmount and when cancelling the auto-approval timer. That name might be a bit misleading in the click handler context.
What do you think about renaming it to something like onCancelAutoApproval to better reflect its intent? This would also apply to the corresponding props in ChatRow and ChatView.
It’s a minor naming cleanup, but could make the logic easier to follow.
the timer for auto question doesn't unmount when user sends input
Before: (the timer continues counting down even after the user enters a response to the follow-up)


After (the timer unmounts on any user input, eg. typed response or selection):
Important
Fix auto question timer to stop on user response, preventing unwanted auto-approvals, with updates in
ChatView.tsx,ChatRow.tsx, andFollowUpSuggest.tsx.isFollowUpAnsweredprop added toChatRowandFollowUpSuggestto track user response.ChatView.tsx: AddeduserRespondedRefandcurrentFollowUpTsto manage timer state.ChatRow.tsx: UpdatedChatRowContentto passisFollowUpAnsweredtoFollowUpSuggest.FollowUpSuggest.tsx: UsesisAnsweredto conditionally stop countdown.FollowUpSuggest.spec.tsxto test countdown behavior and user interaction handling.This description was created by
for 2d49c16. You can customize this summary. It will automatically update as commits are pushed.