-
-
Notifications
You must be signed in to change notification settings - Fork 259
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 UI revamp #1074
Chat UI revamp #1074
Conversation
fda75e6
to
7d7a1c2
Compare
7d7a1c2
to
97bc85f
Compare
RE making placeholder more transparent: The placeholder is already set to a lighter color (as you can see in the second screenshot), but the theme you used in the first screenshot doesn't seem to distinguish it from the muted color. I don't think we should introduce a fourth muted level outside of the design system. WDYT? RE add context color: I intentionally made add context button pop out. If it uses the same text-faint token, the entire left section will use the same color. It will make placeholder harder to distinguish from the rest. Cursor adds a border to the button to make it stands out. Would you like to explore that option? |
RE timestamp: It's also using the most light color and the smallest font in the design system. We can break out and introduce a smaller and lighter one, but it will defy the purpose of the design system. I believe the main reason it doesn't look as good is that the theme does not distinguish between muted and faint colors. Consequently, everything marked as faint stands out more than it should. Again, this is the choice of the theme. There isn't much we can do about it. If we choose to use a gray color without a semantic token, it may look strange in another theme, potentially causing other issues. |
97bc85f
to
2d846b5
Compare
Gotcha. Then the placeholder should be fine as is. I think Cursor's plus icon with a border looks great, quite clean and minimal. Right now "Add context" with the placeholder text below looks a bit cluttered IMHO. We could imitate Cursor there. |
2d846b5
to
5e2e4c8
Compare
@zeroliu sorry I meant could the "Add context" text be removed? My main concern is the stacked text at very uneven spacings. Ideally, as you guys mentioned before, eventually we should get rid of chat mode selection completely. But as we still rely on it now and decide to move it to the left side, this area with "Add context" becomes stacked up. The placeholder text / text input can also benefit from a bit more top spacing imo. |
The text box has a line height. When measuring the gap between elements, we usually use the distance between the bounding boxes, instead of the characters. I think the term "add context" is needed because I often forget that the + button on the top right is for starting a new chat. The label here will help people identify the different functionalities of the two + buttons. You can find the same pattern in Cursor where the Add Context label will first shown and disappear when contexts are selected. The mix of borderless elements and the ones with border does make the space looks off. We can add a border to the chat model select too. But I think it's better without. I can also adjust the spacing to make it look more even. |
5e2e4c8
to
5a273a1
Compare
5a273a1
to
bf15e02
Compare
Oh yes you are right, Cursor also has "Add context" when there's no context. Let's keep it then. The latest version looks better with the new spacing. LGTM! |
Revamp chat UI to address a few pain points:
The new UI fixes these functionalities with the following improvements: