refactor: update styles and prompt selection logic (#103)#124
refactor: update styles and prompt selection logic (#103)#124
Conversation
- Improved accessibility by standardizing `onKeyDown` event handlers to use consistent formatting for key checks across multiple components. - Refactored layout of JSX elements for better readability in `chat.pb.go`, `embed-sidebar`, and various components in the message entry container. - General code cleanup to enhance maintainability and readability.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on March 6
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| setDisplayMode(displayMode); | ||
| } | ||
|
|
||
| useThemeSync(); |
There was a problem hiding this comment.
Duplicate useThemeSync hook call accidentally added
Medium Severity
A second useThemeSync() call was introduced at line 37, while the original call already exists at line 29. This results in duplicate useEffect hooks running — in "auto" theme mode, two matchMedia change listeners are registered simultaneously. While applyTheme is idempotent so behavior isn't visibly broken, this is clearly an accidental duplication introduced during the refactor, not an intentional change.
There was a problem hiding this comment.
Pull request overview
This PR aims to improve accessibility and code readability by standardizing keyboard event handlers and reformatting JSX elements across multiple components. The PR focuses on refactoring onKeyDown handlers to use consistent multi-line formatting and converting single quotes to double quotes for key checks.
Changes:
- Standardized formatting of
onKeyDownevent handlers across 17+ TypeScript/React files, converting inline handlers to multi-line format with consistent double-quote usage - Added a
useThemeSync()hook call to the Office app component - Reformatted JSX elements for improved readability (multi-line attributes for resize handles, labels, etc.)
- Minor code cleanup in inline-suggestion.ts (collapsed multi-line statements)
- Regenerated protobuf Go code with reordered imports
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| webapp/_webapp/src/views/settings/sections/footer.tsx | Changed quote style in onKeyDown handler from single to double quotes |
| webapp/_webapp/src/views/office/app.tsx | Added useThemeSync() hook call (appears to be duplicate) |
| webapp/_webapp/src/views/login/login-with-overleaf.tsx | Reformatted onKeyDown handler to multi-line format with double quotes |
| webapp/_webapp/src/views/login/index.tsx | Reformatted onKeyDown handler to multi-line format with double quotes |
| webapp/_webapp/src/views/embed-sidebar.tsx | Reformatted resize handle div element to multi-line format |
| webapp/_webapp/src/views/devtools/index.tsx | Reformatted label element to multi-line format |
| webapp/_webapp/src/views/chat/header/chat-button.tsx | Changed quote style in onKeyDown handler from single to double quotes |
| webapp/_webapp/src/views/chat/footer/toolbar/selection.tsx | Reformatted reducer type definition and changed quote style in onKeyDown handler |
| webapp/_webapp/src/views/chat/body/index.tsx | Changed quote style in onKeyDown handler from single to double quotes |
| webapp/_webapp/src/libs/inline-suggestion.ts | Collapsed multi-line variable assignments and return statement for readability |
| webapp/_webapp/src/components/tabs.tsx | Reformatted resize handle div element to multi-line format |
| webapp/_webapp/src/components/onboarding-guide.tsx | Reformatted two onKeyDown handlers to multi-line format with double quotes |
| webapp/_webapp/src/components/message-entry-container/tools/xtramcp/*.tsx | Reformatted onKeyDown handlers across 6 XtraMCP tool components to multi-line format |
| webapp/_webapp/src/components/message-entry-container/tools/paper-score-comment/filter-controls.tsx | Reformatted onKeyDown handler to multi-line format with double quotes |
| webapp/_webapp/src/components/message-entry-container/tools/general.tsx | Reformatted div element and onKeyDown handler to multi-line format |
| webapp/_webapp/src/components/message-entry-container/assistant.tsx | Reformatted span element and onKeyDown handler to multi-line format |
| pkg/gen/api/chat/v2/chat.pb.go | Reordered imports in auto-generated protobuf file |
Comments suppressed due to low confidence (4)
webapp/_webapp/src/views/settings/sections/footer.tsx:59
- Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
setVersionClickCount((prev: number) => {
const next = prev + 1;
if (next >= 5) {
setEnableUserDeveloperTools(!enableUserDeveloperTools);
return 0;
}
return next;
});
if (versionClickTimeout) {
clearTimeout(versionClickTimeout);
}
const timeout = setTimeout(() => {
setVersionClickCount(0);
}, 1500);
setVersionClickTimeout(timeout);
}
}}
webapp/_webapp/src/views/chat/footer/toolbar/selection.tsx:196
- Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior, similar to how it's done in other keyboard handlers in this file (see lines 77, 113, 127, 141).
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
if (item.disabled) return;
googleAnalytics.fireEvent(user?.id, `select_${normalizeName(item.title)}`, {});
onSelect?.(item);
}
}}
webapp/_webapp/src/views/chat/header/chat-button.tsx:82
- Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
if (disabled) return;
googleAnalytics.fireEvent(user?.id, "click_chat_button_" + normalizeName(text ?? alt ?? icon), {});
onClick?.();
}
}}
webapp/_webapp/src/views/chat/body/index.tsx:154
- Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
onKeyDown={async (e) => {
if (e.key === "Enter" || e.key === " ") {
try {
const response = await getConversation({ conversationId: conversation?.id ?? "" });
if (!response.conversation) {
throw new Error(`Failed to load conversation ${conversation?.id ?? "unknown"}`);
}
setCurrentConversation(response.conversation);
useStreamingStateMachine.getState().reset();
setReloadSuccess(ReloadStatus.Success);
} catch {
setReloadSuccess(ReloadStatus.Failed);
} finally {
setTimeout(() => {
setReloadSuccess(ReloadStatus.Default);
}, 3000);
}
}
}}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useThemeSync(); | ||
|
|
There was a problem hiding this comment.
Duplicate call to useThemeSync(). This hook is already called on line 29, and calling it twice in the same component serves no purpose and should be removed.
| useThemeSync(); |
| onClick={() => setIsMetadataCollapsed(!isMetadataCollapsed)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsMetadataCollapsed(!isMetadataCollapsed); } }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === " ") { | |
| e.preventDefault(); | |
| } |
| onClick={() => setIsMetadataCollapsed(!isMetadataCollapsed)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsMetadataCollapsed(!isMetadataCollapsed); } }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); |
| onClick={() => setIsSuggestionsExpanded(!isSuggestionsExpanded)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsSuggestionsExpanded(!isSuggestionsExpanded); } }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); |
| tabIndex={0} | ||
| onClick={toggleCollapse} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === " ") { | |
| e.preventDefault(); | |
| } |
| onClick={() => setIsMetadataCollapsed(!isMetadataCollapsed)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsMetadataCollapsed(!isMetadataCollapsed); } }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); |
| onClick={() => setIsMetadataCollapsed(!isMetadataCollapsed)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsMetadataCollapsed(!isMetadataCollapsed); } }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === " ") { | |
| e.preventDefault(); | |
| } |
| onClick={() => setIsMetadataCollapsed(!isMetadataCollapsed)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsMetadataCollapsed(!isMetadataCollapsed); } }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === " ") { | |
| e.preventDefault(); | |
| } |
| onClick={() => setIsMetadataCollapsed(!isMetadataCollapsed)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsMetadataCollapsed(!isMetadataCollapsed); } }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); |
| onClick={() => setIsMetadataCollapsed(!isMetadataCollapsed)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsMetadataCollapsed(!isMetadataCollapsed); } }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { |
There was a problem hiding this comment.
Missing e.preventDefault() call when handling Space key. When Space is pressed on an element with role="button", the browser will scroll the page by default. Add e.preventDefault() inside the if statement to prevent unwanted scrolling behavior.
| if (e.key === "Enter" || e.key === " ") { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); |
onKeyDownevent handlers to use consistent formatting for key checks across multiple components.chat.pb.go,embed-sidebar, and various components in the message entry container.Note
Medium Risk
Mostly formatting/accessibility refactors, but the duplicate
useThemeSync()call may register effects twice and cause unexpected theme sync behavior.Overview
Standardizes keyboard accessibility handlers across the webapp by reformatting many
onKeyDownblocks (consistent Enter/Space checks, quoting, and multi-line JSX) for message actions, collapsible tool cards, onboarding images/modals, prompt selection items, and chat/header buttons.Includes minor cleanup/format-only changes such as reflowed JSX for resize handles/labels, small readability tweaks in
inline-suggestion.ts, and a generatedchat.pb.goimport reorder; notablyviews/office/app.tsxnow callsuseThemeSync()twice (likely unintended).Written by Cursor Bugbot for commit 0a06800. This will update automatically on new commits. Configure here.