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: Display Enhanced Context settings on first chat #3547

Merged
merged 22 commits into from
Mar 26, 2024

Conversation

abeatrix
Copy link
Contributor

@abeatrix abeatrix commented Mar 25, 2024

CLOSE #2839

image

  • As discussed with @toolmantim in this figma comment thread, we will open Enhanced Settings on first chat by default.
  • Add dropdown button for Enhanced Context Settings
  • Existing button for Enhanced Context Settings is now for toggling enhanced context enablement.

Test plan

  • Updated e2e tests

To test this manually:

  1. Open a new Cody Chat
  2. The Enhanced Context settings should be opened by default
  3. Send a new message
  4. the Enhanced Context settings windows is closed when you move focus to the chat input box

After

First chat for Dot Com users:

image

First chat for enterprise users:

image

@abeatrix abeatrix changed the title Bee/context settings Chat: Display Enhanced Context settings on first chat Mar 25, 2024
@abeatrix
Copy link
Contributor Author

@toolmantim before I fix the e2e tests to reflect the new behavior, can you take a look and let me know if this aligns with what you had in mind?

@toolmantim
Copy link
Contributor

toolmantim commented Mar 25, 2024

@abeatrix Nice! Yeah checking it out now. Quick one: I spotted CodyVSCodeExtension:useEnhancedContextToggler:clicked event isn't triggered when you click it via the new button. (edit: we may want to include a metadata property of the source, so we can tell where they click it from — popover or input button)

@abeatrix
Copy link
Contributor Author

@abeatrix Nice! Yeah checking it out now. Quick one: I spotted CodyVSCodeExtension:useEnhancedContextToggler:clicked event isn't triggered when you click it via the new button. (edit: we may want to include a metadata property of the source, so we can tell where they click it from — popover or input button)

added a new event useEnhancedContextInputToggler event for the new icon click

@toolmantim
Copy link
Contributor

Just pushed a bunch of style fixes, and the only thing left I think is some of the keyboard accessibility fixes:

  • Tabbing to enhanced context indicator button and hitting space/enter shouldn't refocus the input, that should only happen when context indicator button is clicked. Pressing escape while the enhanced context indicator button is focused should focus the message input.
  • If you shift-tab from the popover checkbox back into the message input, the popover should be dismissed

Copy link
Contributor

@toolmantim toolmantim left a comment

Choose a reason for hiding this comment

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

Approving to unblock, given how close this is

@abeatrix
Copy link
Contributor Author

Approving to unblock, given how close this is

Thank you so much for your help with the css ❤️ I'll work on the keyboard shortcuts tomorrow, and then update all the outdated e2e tests and add new ones to cover these new behavior. Thanks again!

@abeatrix
Copy link
Contributor Author

If you shift-tab from the popover checkbox back into the message input, the popover should be dismissed
@toolmantim I couldn't get this to work, but I will work on it in a follow-up as this is an existing behavior. For now, users can hit Escape when the input box is focused though to dismiss the popover.

@abeatrix abeatrix merged commit 0eccb1d into main Mar 26, 2024
19 of 20 checks passed
@abeatrix abeatrix deleted the bee/context-settings branch March 26, 2024 20:01
presentationMode={userInfo.isDotComUser ? 'consumer' : 'enterprise'}
isFirstChat={transcript.length < 1}
Copy link
Contributor

Choose a reason for hiding this comment

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

I only spotted this logic. This is more like isEmptyChat which isn't what we want here. We want it to appear on the first chat that's created for first time users, not every time a chat is created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants