feat: add click-to-edit, ESC-to-cancel, and fix padding consistency for chat messages - #7790
Conversation
- Enable click-to-edit for past messages by making message text clickable - Add ESC key handler to cancel edit mode in ChatTextArea - Fix padding consistency between past and queued message editors - Adjust right padding for edit mode to accommodate cancel button Fixes #7788
| ) : ( | ||
| <div className="flex justify-between"> | ||
| <div className="flex-grow px-2 py-1 wrap-anywhere"> | ||
| <div |
There was a problem hiding this comment.
The clickable message text container now calls handleEditClick correctly. Consider adding accessibility attributes (e.g., role='button' and an appropriate aria-label) to improve keyboard navigation and screen reader support.
ghost
left a comment
There was a problem hiding this comment.
Reviewing my own code is like looking in a mirror after writing it at 3 AM - equally horrifying and enlightening.
| <div className="flex justify-between"> | ||
| <div className="flex-grow px-2 py-1 wrap-anywhere"> | ||
| <div | ||
| className="flex-grow px-2 py-1 wrap-anywhere cursor-pointer hover:bg-vscode-list-hoverBackground rounded transition-colors" |
There was a problem hiding this comment.
Consider adding an aria-label attribute here for better accessibility. Screen reader users would benefit from knowing this area is clickable for editing:
| className="flex-grow px-2 py-1 wrap-anywhere cursor-pointer hover:bg-vscode-list-hoverBackground rounded transition-colors" | |
| <div | |
| className="flex-grow px-2 py-1 wrap-anywhere cursor-pointer hover:bg-vscode-list-hoverBackground rounded transition-colors" | |
| aria-label={t("chat:queuedMessages.clickToEdit")} | |
| onClick={(e) => { |
| "overflow-x-hidden", | ||
| "overflow-y-auto", | ||
| "pr-9", | ||
| isEditMode ? "pr-[72px]" : "pr-9", |
There was a problem hiding this comment.
For consistency, could we use the same unit here? Currently mixing px and rem units:
| isEditMode ? "pr-[72px]" : "pr-9", | |
| isEditMode ? "pr-[4.5rem]" : "pr-9", |
This would make the spacing more maintainable and consistent with the design system.
| handleEditClick() | ||
| } | ||
| }} | ||
| title={t("chat:queuedMessages.clickToEdit")}> |
There was a problem hiding this comment.
This reuses the queued messages translation key. Would it be clearer to have a dedicated key like chat:pastMessages.clickToEdit for future maintenance? This way, if we need different text for past vs queued messages, we won't break anything.
Summary
This PR addresses Issue #7788 by implementing three UX improvements for the chat message editing experience:
Changes
Click-to-Edit Implementation
ChatRow.tsxhover:bg-vscode-list-hoverBackground) for better UXhandleEditClick()function with proper event propagation controlESC-to-Cancel Implementation
ChatTextArea.tsxfor edit modeonCancelcallback when ESC is pressedPadding Fix
p-2wrapper padding from edit container inChatRow.tsxpr-[72px]in edit mode to accommodate cancel buttonpx-2padding for visual consistencyTesting
Acceptance Criteria Met
Fixes #7788
Important
Adds click-to-edit, ESC-to-cancel, and padding consistency for chat messages in
ChatRow.tsxandChatTextArea.tsx.ChatRow.tsx.ChatTextArea.tsx.ChatRow.tsxandChatTextArea.tsxfor visual consistency.handleEditClick(): Initiates edit mode inChatRow.tsx.onCancel: Cancels edit mode inChatTextArea.tsx.ChatRow.tsx.ChatRow.tsxandChatTextArea.tsx.This description was created by
for c9897de. You can customize this summary. It will automatically update as commits are pushed.