feat: add MoreMenu component for toolbar actions#71
Conversation
- Introduced MoreMenu component to consolidate less-frequent actions (Search, Export, Analyze, Settings) behind a dropdown menu. - Removed individual action buttons from TabBar for a cleaner interface. - Implemented functionality to close the menu on outside clicks and Escape key press.
Summary of ChangesHello @matt1398, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the application's toolbar by introducing a centralized 'MoreMenu' component. This change aims to declutter the user interface by grouping several less-frequently used actions into a single, accessible dropdown. The consolidation of these actions enhances the overall visual simplicity and navigability of the toolbar, providing a cleaner and more intuitive experience for users without removing any core functionality. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully refactors the toolbar by introducing a MoreMenu component to house less-frequently used actions, which significantly cleans up the TabBar interface. The new component is well-implemented. I have one suggestion to combine the useEffect hooks for closing the menu to improve code clarity and maintainability. Overall, this is a great improvement to the UI.
| useEffect(() => { | ||
| if (!isOpen) return; | ||
|
|
||
| const handleClickOutside = (event: MouseEvent): void => { | ||
| if (containerRef.current && !containerRef.current.contains(event.target as Node)) { | ||
| setIsOpen(false); | ||
| } | ||
| }; | ||
|
|
||
| document.addEventListener('mousedown', handleClickOutside); | ||
| return () => document.removeEventListener('mousedown', handleClickOutside); | ||
| }, [isOpen]); | ||
|
|
||
| // Close on Escape | ||
| useEffect(() => { | ||
| if (!isOpen) return; | ||
|
|
||
| const handleEscape = (event: KeyboardEvent): void => { | ||
| if (event.key === 'Escape') { | ||
| setIsOpen(false); | ||
| } | ||
| }; | ||
|
|
||
| document.addEventListener('keydown', handleEscape); | ||
| return () => document.removeEventListener('keydown', handleEscape); | ||
| }, [isOpen]); |
There was a problem hiding this comment.
To improve code clarity and reduce duplication, you can combine the two useEffect hooks for closing the menu into a single hook. This groups the related logic for handling 'mousedown' and 'keydown' events together.
// Close on outside click or Escape
useEffect(() => {
if (!isOpen) return;
const handleClickOutside = (event: MouseEvent): void => {
if (containerRef.current && !containerRef.current.contains(event.target as Node)) {
setIsOpen(false);
}
};
const handleEscape = (event: KeyboardEvent): void => {
if (event.key === 'Escape') {
setIsOpen(false);
}
};
document.addEventListener('mousedown', handleClickOutside);
document.addEventListener('keydown', handleEscape);
return () => {
document.removeEventListener('mousedown', handleClickOutside);
document.removeEventListener('keydown', handleEscape);
};
}, [isOpen]);
📝 WalkthroughWalkthroughThe changes introduce a new MoreMenu dropdown component that consolidates search, session analysis, export, and settings actions. TabBar is refactored to use MoreMenu instead of individual action buttons. Changes
Suggested labels
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/renderer/components/layout/MoreMenu.tsx (2)
8-18: Reorder imports to match project conventions.Move
lucide-reactinto the external group before path aliases.♻️ Suggested import ordering
-import React, { useCallback, useEffect, useRef, useState } from 'react'; - -import { useStore } from '@renderer/store'; -import { triggerDownload } from '@renderer/utils/sessionExporter'; -import { formatShortcut } from '@renderer/utils/stringUtils'; -import { Activity, Braces, FileText, MoreHorizontal, Search, Settings, Type } from 'lucide-react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; +import { Activity, Braces, FileText, MoreHorizontal, Search, Settings, Type } from 'lucide-react'; + +import { useStore } from '@renderer/store'; +import { triggerDownload } from '@renderer/utils/sessionExporter'; +import { formatShortcut } from '@renderer/utils/stringUtils';As per coding guidelines, organize imports in order: external packages, path aliases (
@main,@renderer,@shared,@preload), then relative imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/layout/MoreMenu.tsx` around lines 8 - 18, The import ordering in MoreMenu.tsx is out of convention: move the lucide-react import (icons: Activity, Braces, FileText, MoreHorizontal, Search, Settings, Type) into the external packages group above the path-alias imports (useStore, triggerDownload, formatShortcut, types, utils), so imports are ordered as external packages first, then path aliases (`@renderer/`... symbols like useStore, triggerDownload, formatShortcut, SessionDetail, Tab, ExportFormat), then any relative imports; adjust spacing between groups accordingly to match project import conventions.
149-190: Addtype="button"to prevent accidental form submits.If this component is ever rendered inside a
<form>, the defaultsubmittype could trigger unintended submits.✅ Safe button types
- <button + <button + type="button" key={item.id} onClick={item.onClick} @@ - <button + <button + type="button" onClick={() => setIsOpen(!isOpen)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/layout/MoreMenu.tsx` around lines 149 - 190, The buttons in MoreMenu can accidentally submit a surrounding form because they lack explicit types; update the button in renderItem (the MenuItem button returned by the renderItem function) and the trigger button that toggles isOpen (the button rendering MoreHorizontal and calling setIsOpen/setButtonHover) to include type="button" to prevent default submit behavior while preserving existing onClick and hover handlers.src/renderer/components/layout/TabBar.tsx (1)
10-20: Reorder imports so externals come before path aliases.
lucide-reactandzustand/react/shallowshould be grouped with other externals ahead of@renderer/*imports.♻️ Suggested import ordering
-import { useDroppable } from '@dnd-kit/core'; -import { horizontalListSortingStrategy, SortableContext } from '@dnd-kit/sortable'; -import { isElectronMode } from '@renderer/api'; -import { HEADER_ROW1_HEIGHT } from '@renderer/constants/layout'; -import { useStore } from '@renderer/store'; -import { formatShortcut } from '@renderer/utils/stringUtils'; -import { Bell, PanelLeft, Plus, RefreshCw } from 'lucide-react'; -import { useShallow } from 'zustand/react/shallow'; +import { useDroppable } from '@dnd-kit/core'; +import { horizontalListSortingStrategy, SortableContext } from '@dnd-kit/sortable'; +import { Bell, PanelLeft, Plus, RefreshCw } from 'lucide-react'; +import { useShallow } from 'zustand/react/shallow'; + +import { isElectronMode } from '@renderer/api'; +import { HEADER_ROW1_HEIGHT } from '@renderer/constants/layout'; +import { useStore } from '@renderer/store'; +import { formatShortcut } from '@renderer/utils/stringUtils';As per coding guidelines, organize imports in order: external packages, path aliases (
@main,@renderer,@shared,@preload), then relative imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/layout/TabBar.tsx` around lines 10 - 20, The import block in TabBar.tsx violates the project's ordering rules: external packages must come before path-alias imports; move lucide-react (Bell, PanelLeft, Plus, RefreshCw) and zustand/react/shallow (useShallow) up into the external imports group so they appear before any `@renderer/`* imports (isElectronMode, HEADER_ROW1_HEIGHT, useStore, formatShortcut), keeping the existing internal import grouping and relative order otherwise; ensure the import groups are separated consistently to match the project convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/layout/MoreMenu.tsx`:
- Around line 8-18: The import ordering in MoreMenu.tsx is out of convention:
move the lucide-react import (icons: Activity, Braces, FileText, MoreHorizontal,
Search, Settings, Type) into the external packages group above the path-alias
imports (useStore, triggerDownload, formatShortcut, types, utils), so imports
are ordered as external packages first, then path aliases (`@renderer/`... symbols
like useStore, triggerDownload, formatShortcut, SessionDetail, Tab,
ExportFormat), then any relative imports; adjust spacing between groups
accordingly to match project import conventions.
- Around line 149-190: The buttons in MoreMenu can accidentally submit a
surrounding form because they lack explicit types; update the button in
renderItem (the MenuItem button returned by the renderItem function) and the
trigger button that toggles isOpen (the button rendering MoreHorizontal and
calling setIsOpen/setButtonHover) to include type="button" to prevent default
submit behavior while preserving existing onClick and hover handlers.
In `@src/renderer/components/layout/TabBar.tsx`:
- Around line 10-20: The import block in TabBar.tsx violates the project's
ordering rules: external packages must come before path-alias imports; move
lucide-react (Bell, PanelLeft, Plus, RefreshCw) and zustand/react/shallow
(useShallow) up into the external imports group so they appear before any
`@renderer/`* imports (isElectronMode, HEADER_ROW1_HEIGHT, useStore,
formatShortcut), keeping the existing internal import grouping and relative
order otherwise; ensure the import groups are separated consistently to match
the project convention.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/renderer/components/layout/MoreMenu.tsxsrc/renderer/components/layout/TabBar.tsx
Summary by CodeRabbit
New Features
Refactor