-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Rewrite chat input to use textarea and ChatGPT-style command selection (fixes inline command bugs) #2393
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
Thank you for your contribution @AidanShipperley ! This looks like a nice improvement and solves a lot of these whack-a-mole issues we've been seeing. Can you take a look at the CI there's an issue with the copilot e2e. Everything else looks great and I'm eager to merge this. @asvishnyakov can you check this out please? I know you have been doing lots of work on improving this area of the code and it would be good to get your review. |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
@AidanShipperley tested and works perfectly! Thank you for the high-quality PR! Test Codeimport chainlit as cl
from chainlit.types import CommandDict
a = cl.ChatProfile(name="Assistant A", markdown_description="Assistant A")
b = cl.ChatProfile(name="Assistant B", markdown_description="Assistant B")
search = CommandDict(
id="Search",
icon="globe",
description="Web Search",
button=False,
persistent=True,
)
image = CommandDict(
id="Picture",
icon="image",
description="DALLE 3",
button=True,
persistent=True,
)
tool = CommandDict(
id="tool",
icon="sparkle",
description="Tool",
button=True,
persistent=False,
)
tool2 = CommandDict(
id="tool2",
icon="sparkle",
description="Tool 2",
button=False,
persistent=False,
)
commands = [search, image, tool, tool2]
@cl.set_chat_profiles
async def send_profiles():
return [a, b]
@cl.on_chat_start
async def start():
if cl.user_session.get("chat_profile") == "Assistant A":
await cl.context.emitter.set_commands(commands)
elif cl.user_session.get("chat_profile") == "Assistant B":
await cl.context.emitter.set_commands([])
@cl.on_message
async def message(msg: cl.Message):
print(f"on_message: {msg.to_dict()}") I'm re-running the CI now to check if this was a transient failure. If it's still failed can you please check/fix? Once passing I'll approve/merge. Thanks again @AidanShipperley ! |
@hayescode I definitely will! But I would appreciate your, @rajanarahul93’s, @mihidumh’s, and anyone else’s review and testing, especially for edge cases I may not be aware of |
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.
@hayescode @AidanShipperley CI is failing because copilot tests depend on contenteditable
attribute, which no longer exist in this PR. Please update them
Thank you for identifying the issue @asvishnyakov & @hayescode! I adjusted the copilot tests to now check for the native placeholder attribute for |
@AidanShipperley I'll check & test everything a bit later. Thank you for the contribution! |
@asvishnyakov I’ve tested the changes locally and they’re working great. The textarea-based input feels much more stable, and the command selection flow is good. No issues encountered so far. |
@mihidumh Yeah, I agree it confuses users and should probably be an opt-out option, but that’s a separate issue. I’d appreciate it if you could create one. |
cypress/e2e/command/spec.cy.ts
Outdated
cy.get('#chat-input').should('exist'); | ||
cy.wait(500); | ||
|
||
// Test 1: Check initial command buttons (Search and StickyButton are button commands) |
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.
You can use Cypress Steps now
textarea.addEventListener('paste', onPaste as any); | ||
|
||
return () => { | ||
textarea.removeEventListener('paste', onPaste); | ||
textarea.removeEventListener('paste', onPaste as any); |
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.
Why? Types seems already match
<style>{` | ||
@keyframes slide-up { | ||
0% { | ||
opacity: 0; | ||
transform: translateY(10px) scale(0.95); | ||
} | ||
100% { | ||
opacity: 1; | ||
transform: translateY(0) scale(1); | ||
} | ||
} | ||
// Set a small timeout to ensure state updates are processed | ||
setTimeout(() => { | ||
setSelectedCommand(command); | ||
.command-menu-animate { | ||
animation: slide-up 0.2s cubic-bezier(0.34, 1.56, 0.64, 1); | ||
} | ||
// Clean up the command input from contentEditable | ||
if (contentEditableRef.current && command && commandInput) { | ||
const content = contentEditableRef.current.textContent || ''; | ||
const cleanedContent = content.replace(commandInput, '').trimStart(); | ||
contentEditableRef.current.textContent = cleanedContent; | ||
} | ||
.command-item-stagger { | ||
animation: slide-up 0.2s cubic-bezier(0.34, 1.56, 0.64, 1) both; | ||
} | ||
setSelectedIndex(0); | ||
setCommandInput(''); | ||
}, 0); | ||
}; | ||
.command-list-container::-webkit-scrollbar { | ||
width: 4px; | ||
} | ||
return ( | ||
<div className="relative w-full"> | ||
<div | ||
.command-list-container::-webkit-scrollbar-track { | ||
background: transparent; | ||
} | ||
.command-list-container::-webkit-scrollbar-thumb { | ||
background-color: hsl(var(--muted-foreground) / 0.3); | ||
border-radius: 2px; | ||
} | ||
.command-list-container { | ||
scrollbar-width: thin; | ||
scrollbar-color: hsl(var(--muted-foreground) / 0.3) transparent; | ||
} | ||
`}</style> |
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.
These animations should be in tailwind.config.js to allow easy customization in projects
We already have animations here
This will also allow you to reuse code from above and avoid duplication
style={{ | ||
bottom: '100%', | ||
marginBottom: '12px' | ||
}} |
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.
Why not bottom-full mb-3
?
</PopoverTrigger> | ||
</TooltipTrigger> | ||
<TooltipContent> | ||
<p>{hasSelectedNonButtonCommand ? 'Change Tool' : 'Available Tools'}</p> |
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.
This should be put to translations and used via Translator
<style>{` | ||
@keyframes command-shift { | ||
0% { transform: translateX(-10px); opacity: 0.8; } | ||
50% { transform: translateX(5px); } | ||
100% { transform: translateX(0); opacity: 1; } | ||
} | ||
.animate-command-shift { | ||
animation: command-shift 0.3s cubic-bezier(0.34, 1.56, 0.64, 1); | ||
} | ||
.command-list-container::-webkit-scrollbar { | ||
width: 4px; | ||
} | ||
.command-list-container::-webkit-scrollbar-track { | ||
background: transparent; | ||
} | ||
.command-list-container::-webkit-scrollbar-thumb { | ||
background-color: hsl(var(--muted-foreground) / 0.3); | ||
border-radius: 2px; | ||
} | ||
.command-list-container { | ||
scrollbar-width: thin; | ||
scrollbar-color: hsl(var(--muted-foreground) / 0.3) transparent; | ||
} | ||
.command-button-transition { | ||
transition: width 0.3s cubic-bezier(0.34, 1.56, 0.64, 1), | ||
padding 0.3s cubic-bezier(0.34, 1.56, 0.64, 1); | ||
} | ||
`}</style> |
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.
These animations should be in tailwind.config.js to allow easy customization in projects
We already have animations here
This will also allow you to reuse code from above and avoid duplication
const handleMouseMove = (index: number) => { | ||
const now = Date.now(); | ||
// Only update if mouse actually moved (not just from render) | ||
if (now - lastMouseMove > 50) { | ||
setSelectedIndex(index); | ||
setLastMouseMove(now); | ||
} | ||
}; | ||
|
||
const handleMouseLeave = () => { | ||
// Keep the last hovered item selected when mouse leaves | ||
setLastMouseMove(Date.now()); | ||
}; | ||
|
||
const handleKeyDown = (e: React.KeyboardEvent) => { | ||
if (!open || nonButtonCommands.length === 0) return; | ||
|
||
// Check if mouse was recently moved | ||
const timeSinceMouseMove = Date.now() - lastMouseMove; | ||
const isUsingKeyboard = timeSinceMouseMove > 100; | ||
|
||
switch (e.key) { | ||
case 'ArrowDown': | ||
e.preventDefault(); | ||
e.stopPropagation(); | ||
if (isUsingKeyboard) { | ||
setSelectedIndex((prev) => | ||
prev < nonButtonCommands.length - 1 ? prev + 1 : 0 | ||
); | ||
} | ||
break; | ||
|
||
case 'ArrowUp': | ||
e.preventDefault(); | ||
e.stopPropagation(); | ||
if (isUsingKeyboard) { | ||
setSelectedIndex((prev) => | ||
prev > 0 ? prev - 1 : nonButtonCommands.length - 1 | ||
); | ||
} | ||
break; | ||
|
||
case 'Enter': { | ||
e.preventDefault(); | ||
e.stopPropagation(); | ||
const selectedCmd = nonButtonCommands[selectedIndex]; | ||
if (selectedCmd) { | ||
onCommandSelect(selectedCmd); | ||
setOpen(false); | ||
cancelTooltipOpen(); | ||
} | ||
break; | ||
} | ||
|
||
case 'Escape': | ||
e.preventDefault(); | ||
e.stopPropagation(); | ||
setOpen(false); | ||
cancelTooltipOpen(); | ||
// Return focus to trigger button (tooltip won't open on focus) | ||
buttonRef.current?.focus(); | ||
break; | ||
} | ||
}; | ||
|
||
const handleCommandSelect = (command: ICommand) => { | ||
onCommandSelect(command); | ||
setOpen(false); | ||
cancelTooltipOpen(); | ||
}; |
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.
This code seems to be copy-pasted from Index.tsx
. Can we reuse it? May be create separate component
<style>{` | ||
@keyframes bounce-subtle { | ||
0%, 100% { transform: translateY(0) scale(1); } | ||
50% { transform: translateY(-2px) scale(1.05); } | ||
} | ||
.animate-bounce-subtle { | ||
animation: bounce-subtle 0.3s cubic-bezier(0.34, 1.56, 0.64, 1); | ||
} | ||
.command-button { | ||
position: relative; | ||
} | ||
/* Direct hover state without intermediate colors */ | ||
.command-button:hover { | ||
background-color: var(--hover-bg) !important; | ||
color: var(--hover-color) !important; | ||
} | ||
.command-button:hover span, | ||
.command-button:hover svg { | ||
color: var(--hover-color) !important; | ||
} | ||
.command-selected::after { | ||
content: ''; | ||
position: absolute; | ||
bottom: -2px; | ||
left: 50%; | ||
transform: translateX(-50%); | ||
width: 30%; | ||
height: 2px; | ||
background-color: #0066FF; | ||
border-radius: 1px; | ||
animation: expand-width 0.3s cubic-bezier(0.34, 1.56, 0.64, 1); | ||
} | ||
@keyframes expand-width { | ||
0% { width: 0; } | ||
100% { width: 30%; } | ||
} | ||
`}</style> |
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.
These animations should be in tailwind.config.js to allow easy customization in projects
We already have animations here
This will also allow you to reuse code from above and avoid duplication
isSelected ? "w-4 opacity-60" : "w-0 opacity-0" | ||
)} | ||
> | ||
<X className="!size-4 text-[#0066FF]" /> |
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.
Please use one of predefined colors from theme or define your own in tailwind.config.js
'--hover-bg': 'hsl(var(--muted))', | ||
'--hover-color': isSelected ? '#0066FF' : 'inherit' |
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.
Please use tailwind's utils
.command-button:hover { | ||
background-color: var(--hover-bg) !important; | ||
color: var(--hover-color) !important; | ||
} | ||
.command-button:hover span, | ||
.command-button:hover svg { | ||
color: var(--hover-color) !important; | ||
} | ||
.command-selected::after { | ||
content: ''; | ||
position: absolute; | ||
bottom: -2px; | ||
left: 50%; | ||
transform: translateX(-50%); | ||
width: 30%; | ||
height: 2px; | ||
background-color: #0066FF; | ||
border-radius: 1px; |
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.
All of this can be done with Tailwind utils
.command-list-container::-webkit-scrollbar { | ||
width: 4px; | ||
} | ||
return ( | ||
<div className="relative w-full"> | ||
<div | ||
.command-list-container::-webkit-scrollbar-track { | ||
background: transparent; | ||
} | ||
.command-list-container::-webkit-scrollbar-thumb { | ||
background-color: hsl(var(--muted-foreground) / 0.3); | ||
border-radius: 2px; | ||
} | ||
.command-list-container { | ||
scrollbar-width: thin; | ||
scrollbar-color: hsl(var(--muted-foreground) / 0.3) transparent; | ||
} |
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.
Do we really need this scrollbar (and WebKit-specific) code here? I know it looks good, but may we at least move it to frontend's index.css
then and reuse
cypress/e2e/command/spec.cy.ts
Outdated
$el.hasClass('text-[#0066FF]') || | ||
$el.find('span').hasClass('text-[#0066FF]') || | ||
$el.find('.text-\\[\\#0066FF\\]').length > 0 |
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.
If you check something like command now disabled
here, it's better to check :disabled
or use other attribute, even if it's introduced specifically for testing purposes. Styles may change over time
cypress/e2e/command/spec.cy.ts
Outdated
cy.get('#chat-input').click().clear(); | ||
cy.get('#chat-input').type('/'); | ||
cy.get('[data-index]').should('have.length.at.least', 3); | ||
cy.get('[data-index="0"]').should('have.class', 'bg-accent'); |
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.
If you check something like command now selected
here, it's better to check :hover
or use other attribute, even if it's introduced specifically for testing purposes. Styles may change over time
@AidanShipperley And regardless of testing: This seems like a bug to me: if you send the Picture command, all commands disappear: Screencast.from.2025-08-15.17-44-37.mp4Let me know if it doesnt’t, as I’m not very familiar with the specifics of commands. At first, I also thought that command deselection was a bug, until I found out about the Everything else seems fine to me. To be honest, I don’t like these animations, especially the one that enlarges a command when you select it, as they feel too heavy and unnatural for this UI. However, that’s more of a personal preference, so you may keep them all if you want. |
@AidanShipperley I want to include your PR here in the next release, can you address @asvishnyakov 's comments? |
@hayescode Yeah, I can take a look at them today! |
Thank you @AidanShipperley ! |
@AidanShipperley thank you again for this, major improvement! One thing i noticed in 3.7.1.1 is that the Tools button is now not wide enough. I think it looks great before when it said "Tools". Would you mind making a PR to fix this? ![]() |
@AidanShipperley, this is such a great feature, and thank you for this! Just wanted to share the Copilot mode in 3.7.1.1 — I noticed that when tools have long names they can overflow and visually crowd the interface, as shown in the attached screenshot. If you change the label's three dots It would be great if the tools names hid and icon only displayed automatically based on width especially in the Copilot mode to keep the layout clean and user-friendly? I was able to hide the tools name and display icons only somehow by zooming in the browser. But if we could achive this same behavior in the previous screenshot's zoom level as well. Thanks again for all the thoughtful work — really appreciate it! |
Updated the structure of command translations and add them to every translation file. Follow-up to #2393 Before: <img width="310" height="129" alt="image" src="https://github.com/user-attachments/assets/0f88dba7-ae17-46b4-a41a-93230a6f9539" /> After: <img width="300" height="116" alt="image" src="https://github.com/user-attachments/assets/f2c9b51a-634f-4f8e-9da5-e1b9d116a401" /> ```json "commands": { "button": "Tools", "changeTool": "Change Tool", "availableTools": "Available Tools" }, ``` No Text is also allowed (for copilot): <img width="294" height="128" alt="image" src="https://github.com/user-attachments/assets/af5998dc-7d7c-44f6-87b9-27113739e485" /> ```json "commands": { "button": "", "changeTool": "", "availableTools": "" }, ```
Updated the structure of command translations and add them to every translation file. Follow-up to Chainlit#2393 Before: <img width="310" height="129" alt="image" src="https://github.com/user-attachments/assets/0f88dba7-ae17-46b4-a41a-93230a6f9539" /> After: <img width="300" height="116" alt="image" src="https://github.com/user-attachments/assets/f2c9b51a-634f-4f8e-9da5-e1b9d116a401" /> ```json "commands": { "button": "Tools", "changeTool": "Change Tool", "availableTools": "Available Tools" }, ``` No Text is also allowed (for copilot): <img width="294" height="128" alt="image" src="https://github.com/user-attachments/assets/af5998dc-7d7c-44f6-87b9-27113739e485" /> ```json "commands": { "button": "", "changeTool": "", "availableTools": "" }, ```
This PR aims to resolve #2357.
This PR rewrites the chat input to use a standard
<textarea>
, removing allcontentEditable
and inline command rendering logic. The input now only handles user text, which fixes a long-standing set of bugs around pasted content (see #2326, #2264, #1890, #2032, #2087). From my testing, I believe this solves all issues with pasted content and maintains compatibility with the existing command APIs, so it should be non-breaking.Key changes:
/
command menu: Typing/
brings up a floating, animated command menu with full keyboard and mouse support.Video demo:
This should resolve all the major issues with the previous inline command implementation and make the input UX much more reliable.
Let me know if you have any feedback or want any tweaks, as I am not as experienced with frontend development.