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

cody: new chat ui with stop generating button #53332

Merged
merged 24 commits into from
Jun 19, 2023
Merged

cody: new chat ui with stop generating button #53332

merged 24 commits into from
Jun 19, 2023

Conversation

abeatrix
Copy link
Contributor

@abeatrix abeatrix commented Jun 12, 2023

Close #53364 & #50058

  • Update Chat UI based on design from Tim (figma) (Thanks @SuperAuguste for the first draft!)
  • Add stop generating button to abort a request in flight (Thanks @thenamankumar for adding it to web that I can use as references!)

Demo

cody.stop.gen.mp4

Test plan

UI updated. Current tests are still passing:

cd client/cody
pnpm test:e2e

VS Code

image
Abort.Button.in.VS.Code.mp4
When Fixup is running
Cody.VS.Code_.Abort.button.for.Inline.and.Nonstop.fixups.mp4

Web

Screenshot 2023-06-16 at 2 28 23 PM Screenshot 2023-06-16 at 2 27 41 PM
Cody.AI.Chat.-.dev.Sourcegraph.-.16.June.2023.mp4

@cla-bot cla-bot bot added the cla-signed label Jun 12, 2023
@abeatrix abeatrix requested a review from toolmantim June 12, 2023 19:05
@abeatrix abeatrix marked this pull request as ready for review June 14, 2023 06:32
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Jun 14, 2023

📖 Storybook live preview

@abeatrix abeatrix changed the title cody: new chat ui cody: new chat ui with stop generating button Jun 15, 2023
@abeatrix abeatrix requested review from almeidapaulooliveira and a team June 15, 2023 18:02
}

.submit-button-disabled {
cursor: not-allowed;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

show the button with not-allowed cursor to address feedback from users where they thought the button is clickable when it's not

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally it would visually change too, but this works in a pinch.

I really like ChatGPT mobile’s interface, with the button transitioning to the stop button. I think that actually might be the better way for where we take this in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea i totally agree. I thought about updating that but i feel like i should do that in a different PR like you said in the future ;)

@toolmantim
Copy link
Contributor

I checked it out, and I really like where you've put the feedback and edit buttons. They look great!

Let's position the Stop Generating button fixed to the bottom, above the compose box:

Screen.Recording.2023-06-16.at.11.46.09.am.mov

I've transitioned the button in with a fade and small slide up, and a straight fade out to transition out. Let me know if you want me to help work on the transitions.

Here's the Figma (with clickable prototype):
https://www.figma.com/file/9112BsKsJc1BpO2j8XYwLL/%F0%9F%A4%96-Cody-VS-Code-%5BMain%5D?type=design&node-id=3532-142495&t=sGLSIMfj0YFYhiuN-4

(While I was here, I also designing the "..." animation. Maybe we can do that in a follow up? I can help with the CSS animations there too)

@toolmantim
Copy link
Contributor

@abeatrix are all the background shades okay to you? If there's any inconsistencies in the Figma to how we're using the VS Code theme color variables, let me know… because there's a good chance it was an accident/oversight, not by design.

Same for the "Stop Generating" button colours there… I used the small Figma VS Code button component, but the color and hover color etc should be the standard secondary one we're using.

@abeatrix
Copy link
Contributor Author

https://www.figma.com/file/9112BsKsJc1BpO2j8XYwLL/%F0%9F%A4%96-Cody-VS-Code-%5BMain%5D?type=design&node-id=3532-142495&t=sGLSIMfj0YFYhiuN-4

Let's position the Stop Generating button fixed to the bottom, above the compose box
I can update the styles for the button but i think we should update the position of the button in another PR so the web team can review, cause right now we are just creating the button for the editor that we can style freely and pass into the existing component that is shared between clients, which handles the position of the button.

If we update the position of the button in the shared package, I think I will also need to update the button styles in the web client since it behaves differently than the editor one right now ( see video below, you needs to chase the button to stop it before the request is done :P). What do you think?

Cody.AI.Chat.Web.Stop.Generating.Example.mp4

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Nice!

client/cody/webviews/Chat.tsx Outdated Show resolved Hide resolved
@toolmantim
Copy link
Contributor

If we update the position of the button in the shared package, I think I will also need to update the button styles in the web client since it behaves differently than the editor one right now ( see video below, you needs to chase the button to stop it before the request is done :P). What do you think?

I think we don't want people to be chasing buttons 😂

@abeatrix
Copy link
Contributor Author

abeatrix commented Jun 16, 2023

I think we don't want people to be chasing buttons 😂

haha I updated the button to get it fixed to the bottom, above the compose box, and added animation for transitioning in. i can't get it to fade out without adding a new state, so i just hold that off for now if that's ok.

are all the background shades okay to you

I'm using the VS code webview tool kit for the editor components so they should just work...? they look fine on my side at least 😅

VS Code

image
Abort.Button.in.VS.Code.mp4

Web

image image
Cody.AI.Chat.-.dev.Sourcegraph.-.16.June.2023.mp4

@thenamankumar
Copy link
Member

thenamankumar commented Jun 16, 2023

The gradient border on the left side on web looks a bit odd to me, given it will be of variable length. @almeidapaulooliveira can give approval from design side regarding this.

Otherwise LGTM.

@abeatrix
Copy link
Contributor Author

@thenamankumar Thanks for doing all the work for web and for letting me copy your homework 🥹

Copy link
Contributor

@dominiccooney dominiccooney left a comment

Choose a reason for hiding this comment

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

@abeatrix I'm thinking that inline fixups, non-stop fixups will need a signal when the response is cancelled so that they don't sit around spinning forever or try to respin something that the user cancelled. (What happens when you hit cancel on them today?)

@abeatrix
Copy link
Contributor Author

abeatrix commented Jun 16, 2023

@abeatrix I'm thinking that inline fixups, non-stop fixups will need a signal when the response is cancelled so that they don't sit around spinning forever or try to respin something that the user cancelled. (What happens when you hit cancel on them today?)

@dominiccooney I totally forgot about them 😅 I've updated so that notifyTurnComplete will be called when job is aborted. inline fixup will pick it up as error and display the error, while non-stop fixup will show no diff highlighted. I added a TODO for onError requests that I think we can work on in another PR :D

Here is how it looks like when users click on the abort button when running the fixups: https://www.loom.com/share/9431081714d448e8a022032bfcc61aed?sid=ae1bc365-d1b5-425b-b19b-5a9fd98f1c0b

@abeatrix
Copy link
Contributor Author

@toolmantim @almeidapaulooliveira Can i get your review for the Pending UI Review please?
image

@abeatrix
Copy link
Contributor Author

Thanks @toolmantim ❤️

@toolmantim
Copy link
Contributor

I gave the web styles a bit more of a thorough test, as I wanted to follow up on @thenamankumar’s comment above. And I've just pushed fixes for how it applies to Cody web.

10b7b1e 67d18e9
Screenshot 2023-06-19 at 10 21 55 pm Screenshot 2023-06-19 at 10 22 20 pm
Screenshot 2023-06-19 at 10 35 23 pm Screenshot 2023-06-19 at 10 35 09 pm

@abeatrix abeatrix merged commit 4201c7e into main Jun 19, 2023
21 of 23 checks passed
@abeatrix abeatrix deleted the bee/cody-new-ui branch June 19, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cody: Update Chat UI
6 participants