Skip to content
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

Chat: preserve unsent chat messages when switching tabs #5847

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

abeatrix
Copy link
Contributor

@abeatrix abeatrix commented Oct 8, 2024

CLOSE https://linear.app/sourcegraph/issue/CODY-3890/switching-tabs-with-an-unsent-chat-message-causes-the-unsent-message

Description: This PR addresses the issue where switching tabs with an unsent chat message causes the unsent message to be lost. The changes implement a mechanism to store and restore the state of unsent messages, ensuring a better user experience when navigating between different views in the Cody extension.

Cause of the issue: Previously, when users switched tabs (e.g., from Chat to History), the state of unsent messages in the chat input was not preserved. This resulted in users losing their draft messages when they returned to the Chat tab.

How these changes resolve the issue:

  1. We've added a lastStoredEditorValue property to the ChatMessage interface to store the state of unsent messages.
  2. Implemented state management in the CodyPanel component to keep track of both the current transcript and the last known editor states.
  3. Added an updateEditorStateOnChange function to update the editor state whenever the user types in the chat input.
  4. Modified the Chat, Transcript, and HumanMessageCell components to use and update the new state management system.
  5. Implemented effects in CodyPanel to reset transcripts on new chat sessions and preserve editor states when switching tabs.

Test plan

Updated unit tests and story for Chat & Transcript.

  1. Open the Cody extension and navigate to the Chat tab.
  2. Start typing a message in the chat input but don't send it.
  3. Switch to another tab (e.g., History).
  4. Switch back to the Chat tab.
  5. Verify that the unsent message is still present in the chat input.
  6. Repeat steps 2-5 multiple times with different messages and tab combinations.
  7. Test with both short and long unsent messages.
  8. Verify that sent messages are not affected by this change.
  9. Test switching between tabs rapidly to ensure no unexpected behavior occurs.

Changelog

Chat: preserve unsent chat messages when switching tabs.

@abeatrix abeatrix requested review from vovakulikov and a team October 8, 2024 16:59
@abeatrix abeatrix marked this pull request as draft October 8, 2024 17:24
@abeatrix abeatrix removed request for a team and vovakulikov October 8, 2024 17:24
@abeatrix abeatrix marked this pull request as ready for review October 9, 2024 01:45
@abeatrix abeatrix requested review from vovakulikov and a team October 9, 2024 01:45
Copy link
Contributor

@vovakulikov vovakulikov left a comment

Choose a reason for hiding this comment

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

Found a few problems. I think all problems come from the nature of syncing state.
Instead I would try to avoid working with transcripts but instead add persisted state
(localstorage) in human message editor component. (Save typed text in local storage by chat and transcript ID and reset it on message submit)

Recorded a loom with problematic behavior
https://github.com/user-attachments/assets/b69a7650-7511-4992-a3c6-cf3d183ae449

)

// Reset transcripts on new transcript change.
useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is the most weak spot of this change, syncing state might be very fragile and unsafe.
I guess because of this we have UI flashes, I checked it manually and I was able to see some UI
delays (when you submit the message but don't see the loading UI below the input)

The main case with switching from the welcome chat screen to history and back to chat produces
a chat UI with no welcome UI

Screenshot 2024-10-09 at 15 48 23

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main case with switching from the welcome chat screen to history and back to chat produces
a chat UI with no welcome UI

It was the expected behavior to me because the user has already typed something in with the updated state, so transcript.length is no longer 0, which would not trigger the welcome message. I agree it's confusing though. will update it to change that.

@@ -76,9 +123,10 @@ export const CodyPanel: FunctionComponent<
<TabContainer value={view} ref={tabContainerRef}>
{view === View.Chat && (
<Chat
transcript={activeTranscript ?? transcript}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the activeTranscript ?? transcript won't work when we want to preserve input text in the follow up message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are only saving the input text state but we don't want to use it until the user has switched to another tab.
This way we won't be re-rendering the transcript every time the state has changed, as we are still serving the same transcript up front if that makes sense?

Copy link
Contributor

@vovakulikov vovakulikov Oct 10, 2024

Choose a reason for hiding this comment

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

Yeah, but do we want to preserve text in the following scenario?

  1. you open a new chat
  2. you type something in the textbox but don't submit it
  3. you switch to the history tab or prompts tab
  4. you switch back to the chat UI
  5. your text has been preserved!
  6. now you submit the message
  7. and you type something in the follow-up
  8. you switch to another tab again
  9. you switch back
  10. Does the follow-up textbox have preserved text from step 7.?

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