Optimization for mobile views and improved styling for chat/assistant sidebar #479
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe PR updates Vite to ^7.1.4, refactors multiple drawers to use Fluent UI Accordions with localStorage-persisted state, reworks several CSS modules for layout and accessibility, adds a mobile slide-in sidebar with backdrop, introduces minor UI/i18n adjustments, and updates related components to the new structure. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Drawer as Drawer (Assistant/Chat)
participant Accordion as Fluent Accordion
participant LS as localStorage
rect rgba(230,240,255,0.6)
note over Drawer: Init
Drawer->>LS: getItem(KEY)
LS-->>Drawer: saved openItems | null
Drawer->>Accordion: render with openItems
end
User->>Accordion: Toggle section
Accordion-->>Drawer: onToggle(event, data)
Drawer->>Drawer: update openItems state
Drawer->>LS: setItem(KEY, JSON.stringify(openItems))
sequenceDiagram
autonumber
actor User
participant Chat as ChatLayout
participant Sidebar as <aside>.sidebar
participant Backdrop as .sidebarBackdrop
rect rgba(240,255,240,0.6)
note over Chat: Mobile sidebar open
Chat->>Sidebar: add class .sidebarOpen
Chat->>Backdrop: render + show
end
User->>Backdrop: pointer down
Backdrop->>Chat: handleBackdropClose()
Chat->>Chat: preventDefault + stopPropagation
Chat->>Chat: onToggleMinimized()
Chat->>Sidebar: remove .sidebarOpen
Chat->>Backdrop: unrender / hide
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (3 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
mucgpt-frontend/src/components/History/History.module.css (9)
6-8: Clip rounded corners of the containerAdd overflow clipping so child hover/focus backgrounds don’t bleed past the rounded edges.
.historyContainer { display: flex; flex-direction: column; height: 100%; padding: 0; border-radius: var(--borderRadiusMedium, 6px); background-color: var(--colorNeutralBackground1); + overflow: hidden; }
26-29: Keyboard parity for expand/collapse icon colorMirror the hover treatment for focus-visible so keyboard users get the same affordance.
.header:hover .expandCollapseIcon, .categoryHeader:hover .expandCollapseIcon { color: var(--colorNeutralForeground2); } + +.header:focus-visible .expandCollapseIcon, +.categoryHeader:focus-visible .expandCollapseIcon { + color: var(--colorNeutralForeground2); +}
44-53: Unify control heights and improve touch targets (≥40–44px)36px is tight on mobile. Define a shared token and apply it for consistency across buttons.
+:root { + /* consider placing in a global tokens file */ + --controlHeightMd: 44px; +} .savedChatButton { - padding: var(--spacingVerticalXS, 4px) var(--spacingHorizontalM, 12px); + padding: var(--spacingVerticalXS, 4px) var(--spacingHorizontalM, 12px); overflow: hidden; text-overflow: ellipsis; white-space: nowrap; border-radius: var(--borderRadiusMedium, 6px); - transition: all 0.2s ease; - height: 36px; + transition: background-color 0.2s ease, color 0.2s ease; + height: var(--controlHeightMd); display: flex; align-items: center; } .loadMoreButton { font-size: var(--fontSizeBase200); color: var(--colorBrandForeground1); width: 95%; display: flex; align-items: center; justify-content: center; gap: var(--spacingHorizontalS, 8px); margin: 0 8px; padding: var(--spacingVerticalXS, 4px) var(--spacingHorizontalS, 8px); border-radius: var(--borderRadiusMedium, 6px); - height: 36px; + height: var(--controlHeightMd); } .actionButton { flex: 1; min-width: 0; - height: 36px; + height: var(--controlHeightMd); font-weight: var(--fontWeightSemibold); display: flex; align-items: center; justify-content: center; gap: var(--spacingHorizontalS, 8px); border-radius: var(--borderRadiusMedium, 6px); }Also applies to: 241-242, 255-266
65-65: Avoid !important on active statePrefer specificity over !important to keep theming/cascade predictable.
-.activeChat { - background-color: var(--colorNeutralBackground3) !important; +.activeChat { + background-color: var(--colorNeutralBackground3); }If needed, increase specificity (e.g.,
.singlechatcontainer .savedChatButton.activeChat).
110-114: Limit transitions and honor reduced motionAvoid
allto reduce layout/paint work and add a reduced-motion fallback..singlechatcontainer { display: flex; flex-direction: row; align-items: center; justify-content: space-between; padding: var(--spacingVerticalXS, 4px) var(--spacingHorizontalS, 8px); margin: 4px 0; border-radius: var(--borderRadiusMedium, 6px); - transition: all 0.2s ease; + transition: background-color 0.2s ease, box-shadow 0.2s ease; } +@media (prefers-reduced-motion: reduce) { + .singlechatcontainer { + transition: none; + } +}
135-146: Header transitions and focusability
- Limit
transition: allto the properties that actually animate.- Ensure the header element is keyboard-focusable (role/button + tabIndex) to make the focus styles reachable.
.header { font-size: var(--fontSizeBase300); font-weight: 600; color: var(--colorNeutralForeground2); margin-top: var(--spacingVerticalM, 12px); margin-bottom: var(--spacingVerticalS, 8px); padding: var(--spacingVerticalM, 12px) var(--spacingHorizontalL, 16px) var(--spacingVerticalS, 8px); display: flex; flex-direction: row; justify-content: space-between; border-bottom: 1px solid var(--colorNeutralStroke2); cursor: pointer; user-select: none; - transition: all 0.2s ease; + transition: background-color 0.2s ease, color 0.2s ease; border-radius: var(--borderRadiusMedium, 6px); }Please confirm the JSX/HTML gives
.headera keyboard focus target.
159-167: Add focus-visible style for CategoryHeaderMatch hover background on keyboard focus for parity.
.categoryHeader:hover { background-color: var(--colorNeutralBackground3); } + +.categoryHeader:focus-visible { + background-color: var(--colorNeutralBackground3); + outline: 2px solid var(--colorBrandStroke1); + outline-offset: 2px; +}Also applies to: 172-174
210-221: Firefox-compatible scrollbar styling for the sidebarOnly WebKit pseudo-elements are present; Firefox ignores them. Add
scrollbar-widthandscrollbar-colorfor parity (relevant to the Firefox sidebar objective)..historyContent { /* existing styles... */ + /* Firefox */ + scrollbar-width: thin; + scrollbar-color: var(--colorNeutralStroke1) var(--colorNeutralBackground1); } .historyContent::-webkit-scrollbar-track { background: var(--colorNeutralBackground1); } .historyContent::-webkit-scrollbar-thumb { background-color: var(--colorNeutralStroke1); border-radius: var(--borderRadiusMedium, 6px); } .historyContent::-webkit-scrollbar-thumb:hover { background-color: var(--colorNeutralForeground3); }
298-300: Mobile tweaks: use dynamic viewport units and avoid fixed widths
- Prefer
dvh/svhfor mobile browser UI chrome changes.- Let the chat button fill the row rather than a fixed 80% width.
@media (max-width: 768px) { .historyContent { - max-height: 50vh; /* Better height on mobile */ + max-height: 50vh; /* fallback */ + } + @supports (max-height: 100dvh) { + .historyContent { + max-height: 50dvh; + } } .singlechatcontainer { padding: 1px 4px; } .savedChatButton { - width: 80%; + width: 100%; } }Also applies to: 306-308
mucgpt-frontend/src/components/ChatLayout/ChatLayout.module.css (2)
89-106: Mobile drawer animation: offer a reduced-motion path.Add a prefers-reduced-motion override to remove slide transitions for users who opt out of animations.
.sidebar { transform: translateX(-100%); - transition: transform 180ms ease-out; + transition: transform 180ms ease-out; } +@media (prefers-reduced-motion: reduce) { + .sidebar { + transition: none; + } +}
118-134: Backdrop visibility is solid; add focus styles for keyboard users.Ensure focus is visible when tabbing to the backdrop.
.sidebarBackdrop { /* ... */ - display: none; /* hidden by default on mobile */ + display: none; /* hidden by default on mobile */ + outline: none; } +.sidebarBackdrop:focus-visible { + outline: 2px solid var(--colorNeutralStroke1); + outline-offset: 2px; +}mucgpt-frontend/src/components/ChatsettingsDrawer/ChatsettingsDrawer.tsx (2)
41-45: Minor: trailing comment clutter and unusedsidebarAction.
sidebarAction= null and the trailing comment can be dropped.- }; // sidebar action and content // No longer using sidebarAction as we've moved actions below the title - const sidebarAction = null; + }; + const sidebarAction = null;
46-58: Remove stray string literal node.
{" "}renders an extra text node; not needed.- <div className={styles.settingsContent}> - {" "} - {/* Title Section with cleaner layout */} + <div className={styles.settingsContent}> + {/* Title Section */}mucgpt-frontend/src/components/AssistantsettingsDrawer/AssistantsettingsDrawer.tsx (1)
103-105: Update comment to reflect accordion implementation.The comment mentions that visibility toggling is "no longer needed with Accordion" but the function is still being used for the edit dialog. Consider updating the comment to clarify this distinction.
- }, [showEditDialog]); // No longer needed with Accordion - // We're using the Accordion's built-in toggle functionality + }, [showEditDialog]); // Used for edit dialog toggle + // Note: Actions visibility now uses Accordion's built-in toggle
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
mucgpt-frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
mucgpt-frontend/package.json(1 hunks)mucgpt-frontend/src/components/AssistantsettingsDrawer/AssistantsettingsDrawer.module.css(8 hunks)mucgpt-frontend/src/components/AssistantsettingsDrawer/AssistantsettingsDrawer.tsx(7 hunks)mucgpt-frontend/src/components/ChatLayout/ChatLayout.module.css(2 hunks)mucgpt-frontend/src/components/ChatLayout/ChatLayout.tsx(1 hunks)mucgpt-frontend/src/components/ChatsettingsDrawer/ChatsettingsDrawer.module.css(7 hunks)mucgpt-frontend/src/components/ChatsettingsDrawer/ChatsettingsDrawer.tsx(2 hunks)mucgpt-frontend/src/components/History/History.module.css(8 hunks)mucgpt-frontend/src/components/MinimizeSidebarButton/MinimizeSidebarButton.tsx(1 hunks)mucgpt-frontend/src/components/Sidebar/Sidebar.module.css(1 hunks)mucgpt-frontend/src/i18n.ts(10 hunks)mucgpt-frontend/src/pages/chat/Chat.tsx(3 hunks)
🔇 Additional comments (74)
mucgpt-frontend/src/components/History/History.module.css (13)
13-13: LGTM: tokenized spacingUsing var(--spacingHorizontalS) here reads well and is consistent.
59-62: Nice: visible keyboard focusFocus ring is clear and offset avoids clipping. Good accessibility addition.
98-101: LGTM: category content layoutTokenized padding and gaps look consistent with the rest of the module.
116-119: LGTM: hover feedbackSubtle background on hover improves discoverability without overpowering.
120-123: LGTM: focus-within stateGood for keyboard and screen-reader users interacting with nested controls.
148-156: LGTM: focus-visible stylingConsistent with other interactive elements; good use of outline and token colors.
180-181: LGTM: full-width contentEnsures the header row captures clicks across the row.
245-247: LGTM: hover stateConsistent token usage for hover background.
248-253: LGTM: content section blockClear separation with padding and border; aligns with the new layout structure.
267-270: LGTM: focus ring on action buttonsGood accessibility affordance.
272-280: LGTM: action row layoutSimple, flexible row with tokenized spacing.
282-289: LGTM: title sectionTokenized vertical rhythm and borders look consistent.
291-291: LGTM: cleanupRemoving the unused animation keeps the CSS lean.
mucgpt-frontend/src/components/ChatLayout/ChatLayout.module.css (1)
107-112: Using [hidden] to keep the node in the DOM is a common trick—just a heads-up on a11y semantics.When [hidden] is set, the element is removed from the accessibility tree even if CSS forces display. That’s fine while closed; just be sure it’s removed when opening (you do). No action needed.
mucgpt-frontend/src/components/Sidebar/Sidebar.module.css (3)
2-15: Sticky header refactor looks good.Flex layout + sticky bar with proper z-index and border improves usability.
18-23: Drawer column + bounded height is appropriate.Flex column + max-height enables reliable internal scrolling.
32-42: Scrollable content region: good use of min-height:0 to enable flex scrolling.Nice detail that avoids overflow bugs.
mucgpt-frontend/src/pages/chat/Chat.tsx (2)
11-11: Scoped styles import is fine.Importing ChatsettingsDrawer.module.css as settingsStyles is appropriate for action wrappers.
540-546: Action wrappers improve layout consistency.Wrapping ClearChat and Minimize buttons in layout containers is clean.
mucgpt-frontend/src/components/ChatLayout/ChatLayout.tsx (1)
59-63: Conditional class toggle is correct.Applying styles.sidebarOpen when visible aligns with the CSS adjacent-sibling rule for the backdrop.
mucgpt-frontend/src/components/MinimizeSidebarButton/MinimizeSidebarButton.tsx (1)
23-25: Adding a text label improves a11y and clarity.Icon + label with Tooltip is a solid UX upgrade.
mucgpt-frontend/src/components/ChatsettingsDrawer/ChatsettingsDrawer.tsx (4)
34-39: Persisting Accordion state in localStorage is a nice touch.Defaulting to ["advanced"] when no state exists is sensible.
62-77: Accordion usage LGTM.Good use of controlled openItems with onToggle; works with a single “advanced” section.
81-82: Sidebar composition change is fine.Actions moved inside content via Accordion is cleaner.
51-53: Verify i18n key spelling in translation resources. The code usescomponents.chattsettingsdrawer.title(double “t”); confirm this exact key exists in your i18n entries (i18n.ts or JSON). If it’s actuallycomponents.chatsettingsdrawer.title, update the code or translations accordingly.mucgpt-frontend/src/components/AssistantsettingsDrawer/AssistantsettingsDrawer.tsx (7)
11-26: LGTM! Proper Fluent UI imports for accordion functionality.The import changes align with the refactoring from manual accordion to Fluent UI Accordion components. The addition of ClearChatButton import also supports the new embedded actions design.
78-90: Well-implemented localStorage-backed accordion state.The implementation correctly:
- Persists accordion state between sessions
- Handles JSON parsing safely with fallback to empty array
- Uses appropriate naming for the localStorage key
- Follows React state initialization best practices
87-90: Proper accordion toggle handler implementation.The handler correctly updates both local state and localStorage synchronously, ensuring state persistence across sessions.
166-189: Clean refactoring of actions to embedded layout.The actions component has been properly refactored to:
- Use dedicated container classes for layout structure
- Maintain proper accessibility with button labels and icons
- Support conditional rendering for the minimize button
- Follow consistent styling patterns
219-290: Excellent accordion integration maintaining full functionality.The content structure successfully:
- Maintains the existing title and description sections
- Integrates actions into the main content area with proper styling
- Implements Fluent UI Accordion with proper event handling
- Preserves all existing functionality (edit, delete, publish dialogs)
- Uses translation keys appropriately
251-284: Well-structured accordion implementation.The accordion structure properly:
- Uses collapsible accordion with state management
- Includes proper translation key with fallback
- Maintains existing button functionality and conditional rendering
- Uses consistent styling classes
291-293: Correct sidebar integration.Properly removes actions from Sidebar props since they're now embedded in the content, maintaining the component's intended architecture.
mucgpt-frontend/src/i18n.ts (10)
227-228: Consistent translation key additions for German.The new keys follow established naming conventions and provide appropriate German translations for the enhanced UI components.
242-242: Proper translation key for assistant actions.The German translation "Aktionen" is appropriate and consistent with the existing terminology.
656-657: Consistent English translations.The English translations properly mirror the German keys with appropriate terminology.
671-671: Consistent assistant actions translation.English "Actions" translation aligns with the German equivalent.
1084-1085: Consistent Bavarian translations.The Bavarian translations maintain the regional dialect appropriately while staying consistent with the UI functionality.
1099-1099: Proper Bavarian translation for actions."Aktionen" is correctly maintained in Bavarian as it aligns with the German standard.
1513-1514: Appropriate French translations.The French translations use proper terminology: "Paramètres du chat" and "Paramètres avancés" are accurate and professional.
1528-1528: Correct French translation for actions."Actions" is the appropriate French translation maintaining consistency across languages.
1943-1944: Proper Ukrainian translations.The Ukrainian translations are grammatically correct and follow proper terminology for UI elements.
1958-1958: Consistent Ukrainian translation."Дії" is the correct Ukrainian translation for "Actions".
mucgpt-frontend/src/components/ChatsettingsDrawer/ChatsettingsDrawer.module.css (15)
18-21: Improved hover and focus consistency.Combining hover and focus-visible states provides better accessibility and user experience consistency.
59-59: Better scrollbar aesthetics.Reducing scrollbar width from 8px to 6px provides a more modern, less intrusive appearance.
63-63: Improved scrollbar track styling.Using the neutral background color for better integration with the overall design.
72-72: Enhanced scrollbar hover feedback.The neutral foreground color on hover provides appropriate visual feedback.
102-109: Well-structured title section layout.The flexbox layout with proper spacing and gap creates a clean, organized header structure that accommodates the new action row layout.
115-119: Consistent title styling.The typography and spacing align well with the design system and maintain visual hierarchy.
122-122: Proper action section spacing.The padding adjustment provides appropriate spacing for the accordion content.
127-127: Consistent spacing for action content.The vertical spacing aligns with the overall design system.
133-142: Well-designed action row layout.The flexbox layout with space-between justification and proper spacing creates an effective layout for action buttons.
235-242: Enhanced button styling.The fixed height, improved typography, and consistent styling create professional-looking action buttons.
244-247: Proper accessibility focus styling.The focus-visible outline ensures keyboard navigation accessibility.
317-317: Improved icon hover feedback.The color change on hover provides appropriate visual feedback.
329-329: Good documentation with cleanup note.The comment about removing duplicates shows attention to code maintenance.
458-474: Excellent accordion styling integration.The global Fluent UI accordion styling provides:
- Consistent appearance with the design system
- Proper spacing and background colors
- Good hover states
- Clean integration with existing styles
475-498: Well-structured button container layout.The button section layout with proper flex distribution and container classes creates a clean, maintainable structure for action buttons.
mucgpt-frontend/src/components/AssistantsettingsDrawer/AssistantsettingsDrawer.module.css (17)
20-27: Improved title section layout structure.The flexbox layout with proper spacing creates a well-organized header section that accommodates the new action row design.
29-37: Consistent assistant title styling.The typography and layout align with the design system and provide proper visual hierarchy.
46-53: Enhanced description section presentation.The border, padding adjustments, and margin refinements create better visual separation and content organization.
55-61: Proper button section spacing.The padding and gap adjustments maintain consistency with the overall layout design.
66-73: Enhanced action button styling.The fixed height, typography improvements, and layout properties create professional, consistent button appearance.
75-90: Well-designed button container system.The container classes provide flexible layout control for action buttons with proper distribution and alignment.
107-107: Improved actions section spacing.The padding adjustment provides appropriate spacing for the accordion content.
112-127: Enhanced actions header styling.The background, border-radius, and spacing create a visually appealing header that integrates well with the accordion design.
130-130: Better hover feedback.The background color change provides appropriate hover state feedback.
133-141: Improved header content layout.The flexbox properties and spacing create proper alignment for header content elements.
160-164: Enhanced actions content spacing.The padding and gap adjustments provide appropriate spacing for action items within the accordion.
177-185: Simplified publish button styling.The removal of complex gradients and transitions in favor of solid styling creates a cleaner, more maintainable appearance.
188-188: Consistent focus styling.The brand stroke color for focus outline maintains consistency with the design system.
205-206: Improved markdown description handling.Removing max-width and adding overflow-wrap ensures better text handling and responsiveness.
236-241: Enhanced action row layout.The space-between justification and responsive spacing create an effective layout for action elements.
267-267: Proper collapse button alignment.The margin-left auto ensures proper alignment within the header layout.
271-271: Consistent hover styling.The background color change aligns with the overall design system hover states.
| "vite": "^7.1.4" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Vite 7 upgrade: verify tooling compatibility and CI node runtime.
- Check that @vitejs/plugin-react 4.7.0 supports Vite ^7.1.4 and your Node ">=20.0.0" CI image. Consider lockfile regen to surface peer warnings.
If you rely on legacy optimizeDeps/include or assetsInlineLimit defaults from Vite 5, revisit them under Vite 7.
🌐 Web query:
Is @vitejs/plugin-react 4.7.0 compatible with Vite 7.1.x and Node 20? Any breaking changes from Vite 5→7 affecting default config?
💡 Result:
Short answer: Yes — @vitejs/plugin-react 4.7.0 is usable with Vite 7.1.x on Node 20, but require Node >= 20.19 (Vite 7's stated minimum) and you should verify any project-specific config (Sass/postcss/browser targets). [1][2][3]
What to watch for when migrating Vite 5 → 7 (changes that can affect default behavior/config):
- Node: Vite 7 requires Node 20.19+ (Node 18 dropped). Ensure your runtime meets that. [2]
- Default browser target: Vite’s default modern target changed (to a "baseline widely available" target), which can change output/transpilation compared to Vite 5. Review build targets if you relied on older defaults. [2]
- Sass API: Vite moved from the legacy Sass API to the modern API (modern API used by default from v6; legacy support removed in v7). If you relied on the legacy Sass behavior, set css.preprocessorOptions.* (or adjust code). [3]
- postcss-load-config: upgraded (postcss-load-config v6) and loading TS/YAML configs changed (tsx/jiti & yaml required). If you used TypeScript/YAML PostCSS configs, update loader/setup. [3]
- Rollup/Rollup-related changes: Vite moved across Rollup versions across majors; library output naming and some Rollup-related defaults changed between earlier majors — check build.lib options if you build libraries. [3][5]
Sources:
- @vitejs/plugin-react release/registry info (4.7.0 release listed). [1]
- Vite 7 announcement (Node 20.19+ requirement; default browser target info). [2]
- Vite migration notes (changes for postcss, Sass, build output, etc.). [3][5]
If you want, I can: 1) check your vite.config for any settings that need updating, or 2) show the exact config snippets to add (e.g., css.preprocessorOptions or browserlist/build.target) to preserve previous behavior.
Ensure Node 20.19+ and regen lockfile for Vite 7 upgrade
- CI: bump Node runtime to v20.19 or later (Vite 7 minimum).
- Regenerate lockfile (
npm install/pnpm install) to surface peer-dependency warnings for Vite ^7.1.4 and @vitejs/plugin-react 4.7.0. - Review Vite 5→7 default-config changes:
- build.target (default browser targets)
- Sass API (set css.preprocessorOptions if you relied on legacy behavior)
- PostCSS loader (TS/YAML config support)
- Rollup build.lib defaults
🤖 Prompt for AI Agents
In mucgpt-frontend/package.json around lines 43-44, the project upgraded Vite to
^7.1.4 but the PR did not update the CI Node runtime or regenerate the lockfile;
update CI to use Node v20.19+ (or later), run your package manager install (npm
install / pnpm install) locally to regenerate package-lock.json or
pnpm-lock.yaml and commit the updated lockfile so peer-dependency warnings
surface, and then audit and adjust Vite-related config changes introduced in 5→7
(confirm build.target, update css.preprocessorOptions for any Sass changes,
verify PostCSS loader/config format, and review rollup build.lib defaults)
before merging.
| const handleBackdropClose: React.PointerEventHandler<HTMLDivElement> = e => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| if (onToggleMinimized) onToggleMinimized(); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Backdrop uses onPointerDown only—add click and keyboard support.
Keyboard users can’t close the drawer. Support Enter/Space/Escape and regular clicks.
const handleBackdropClose: React.PointerEventHandler<HTMLDivElement> = e => {
e.preventDefault();
e.stopPropagation();
if (onToggleMinimized) onToggleMinimized();
};
+
+const handleBackdropKeyDown: React.KeyboardEventHandler<HTMLDivElement> = e => {
+ if (e.key === "Escape" || e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ e.stopPropagation();
+ onToggleMinimized?.();
+ }
+};Committable suggestion skipped: line range outside the PR's diff.
| {size !== "none" && ( | ||
| <div className={styles.sidebarBackdrop} role="button" aria-label="Close sidebar" onPointerDown={handleBackdropClose} tabIndex={0} /> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wire up click/keyboard handlers and keep aria consistent.
Also consider localizing the aria-label in a follow-up.
- {size !== "none" && (
- <div className={styles.sidebarBackdrop} role="button" aria-label="Close sidebar" onPointerDown={handleBackdropClose} tabIndex={0} />
- )}
+ {size !== "none" && (
+ <div
+ className={styles.sidebarBackdrop}
+ role="button"
+ aria-label="Close sidebar"
+ onPointerDown={handleBackdropClose}
+ onClick={handleBackdropClose}
+ onKeyDown={handleBackdropKeyDown}
+ tabIndex={0}
+ />
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {size !== "none" && ( | |
| <div className={styles.sidebarBackdrop} role="button" aria-label="Close sidebar" onPointerDown={handleBackdropClose} tabIndex={0} /> | |
| )} | |
| {size !== "none" && ( | |
| <div | |
| className={styles.sidebarBackdrop} | |
| role="button" | |
| aria-label="Close sidebar" | |
| onPointerDown={handleBackdropClose} | |
| onClick={handleBackdropClose} | |
| onKeyDown={handleBackdropKeyDown} | |
| tabIndex={0} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
In mucgpt-frontend/src/components/ChatLayout/ChatLayout.tsx around lines 67 to
69, the sidebar backdrop only uses onPointerDown and an aria-label string;
update it to wire conventional click and keyboard handlers and keep ARIA
consistent by: replace/augment onPointerDown with onClick and onKeyDown handlers
that call handleBackdropClose on Enter or Space (and optionally Escape), ensure
the element remains focusable via tabIndex={0} and role="button", and replace
the hard-coded aria-label with a localized string (use the existing i18n wrapper
or translation hook) so the label is consistent with the app localization.
| onToggleMinimized={() => setShowSidebar(prev => !prev)} | ||
| clearChat={clearChat} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Persist sidebar state when toggled via backdrop/opener.
Backdrop toggling (via ChatLayout) doesn’t update localStorage, causing state mismatch after reload. Persist it here.
- onToggleMinimized={() => setShowSidebar(prev => !prev)}
+ onToggleMinimized={() =>
+ setShowSidebar(prev => {
+ const next = !prev;
+ localStorage.setItem(STORAGE_KEYS.SHOW_SIDEBAR, String(next));
+ return next;
+ })
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onToggleMinimized={() => setShowSidebar(prev => !prev)} | |
| clearChat={clearChat} | |
| onToggleMinimized={() => | |
| setShowSidebar(prev => { | |
| const next = !prev; | |
| localStorage.setItem(STORAGE_KEYS.SHOW_SIDEBAR, String(next)); | |
| return next; | |
| }) | |
| } | |
| clearChat={clearChat} |
🤖 Prompt for AI Agents
In mucgpt-frontend/src/pages/chat/Chat.tsx around lines 610-611, the
onToggleMinimized handler toggles showSidebar but doesn't persist that change to
localStorage, so backdrop/opener toggles don't survive reloads; update the
handler to both flip state and write the new value to localStorage (e.g.,
setShowSidebar(prev => { const next = !prev;
localStorage.setItem('sidebarMinimized', JSON.stringify(next)); return next;
})), and ensure the component’s initial state reads
JSON.parse(localStorage.getItem('sidebarMinimized')) (with a safe fallback) so
the persisted value is honored on load.
|
Is better done in #553. Close this now |
Reference
Closes #448
Summary by CodeRabbit
New Features
Improvements
Localization
Chores