Skip to content

Improve side panel drawer UX - toggleable drawer, label #911

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.

Sign up for GitHub

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

Open
azidancorp wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve side panel drawer UX - toggleable drawer, label #911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed9c526
Improve side panel drawer UX
azidancorp Dec 20, 2025
56b416b
Auto-open drawer on wide IndependentPanel
azidancorp Dec 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/components/ConversationCard/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,11 @@ function ConversationCard(props) {
className="gpt-util-group"
style={{
padding: '15px 0 15px 15px',
...(props.notClampSize ? {} : { flexGrow: isSafari() ? 0 : 1 }),
...(props.pageMode
? { flexGrow: 1, minWidth: 0 }
: props.notClampSize
? {}
: { flexGrow: isSafari() ? 0 : 1 }),
...(isSafari() ? { maxWidth: '200px' } : {}),
}}
>
Expand All @@ -372,11 +376,28 @@ function ConversationCard(props) {
>
<Pin size={16} />
</span>
) : props.onToggleSidebar ? (
<button
type="button"
className="gpt-util-icon gpt-menu-toggle"
title="Toggle sidebar"
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hardcoded English string should use i18n.

The title attribute uses a hardcoded string "Toggle sidebar" while the rest of the component uses the t() function for translations.

🔎 Proposed fix 
             <button
               type="button"
               className="gpt-util-icon gpt-menu-toggle"
-              title="Toggle sidebar"
-              aria-label="Toggle sidebar"
+              title={t('Toggle sidebar')}
+              aria-label={t('Toggle sidebar')}
               aria-expanded={Boolean(props.sidebarOpen)}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/components/ConversationCard/index.jsx around line 383, the title
attribute is hardcoded as "Toggle sidebar"; replace it with the i18n call used
elsewhere (e.g. title={t('Toggle sidebar')} or, preferably, a semantic key like
title={t('conversation.toggle_sidebar')}) and ensure the t function is in scope
(import/useTranslation or prop) and add the new key/value to the translation
files so the string is localized.

aria-label="Toggle sidebar"
Comment on lines +383 to +384
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Route new sidebar control labels through i18n

The new sidebar toggle introduces hardcoded English accessibility text ("Toggle sidebar") instead of using the existing translation pipeline. This regresses localization for non-English users (including screen-reader labels/tooltips) in a UI that otherwise uses t(...) throughout, and it creates locale drift as new languages are added.

Useful? React with 👍 / 👎.

Comment on lines +383 to +384
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Localize the new sidebar toggle accessibility labels

The new sidebar toggle button ships hardcoded English UI text (title and aria-label) instead of using the existing t(...) translation path used across this component. This regresses accessibility/i18n for non-English locales because assistive text remains English even when the rest of the interface is localized.

Useful? React with 👍 / 👎.

aria-expanded={Boolean(props.sidebarOpen)}
onClick={(e) => {
e.preventDefault()
e.stopPropagation()
props.onToggleSidebar()
}}
>
☰
</button>
) : (
<img src={logo} style="user-select:none;width:20px;height:20px;" />
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In React/JSX, it's a best practice to provide styles as a camelCased object rather than a string. This improves code consistency, type safety, and prevents potential rendering issues.

Suggested change
<img src={logo} style="user-select:none;width:20px;height:20px;" />
<img src={logo} style={{ userSelect: 'none', width: '20px', height: '20px' }} />

)}
<select
style={props.notClampSize ? {} : { width: 0, flexGrow: 1 }}
style={
props.pageMode || !props.notClampSize ? { width: 0, flexGrow: 1, minWidth: 0 } : {}
}
Comment on lines +398 to +400
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditional logic props.pageMode || !props.notClampSize is complex and could be clearer. The condition appears to apply the same styles when either pageMode is true OR notClampSize is false. Consider extracting this to a named boolean variable (e.g., const shouldClampSelectWidth = props.pageMode || !props.notClampSize) to improve readability and make the intent more explicit.

Suggested change
style={
props.pageMode || !props.notClampSize ? { width: 0, flexGrow: 1, minWidth: 0 } : {}
}
style={(() => {
const shouldClampSelectWidth = props.pageMode || !props.notClampSize
return shouldClampSelectWidth ? { width: 0, flexGrow: 1, minWidth: 0 } : {}
})()}

Copilot uses AI. Check for mistakes.
className="normal-button"
required
onChange={(e) => {
Expand Down Expand Up @@ -425,7 +446,8 @@ function ConversationCard(props) {
style={{
padding: '15px 15px 15px 0',
justifyContent: 'flex-end',
flexGrow: props.draggable && !completeDraggable ? 0 : 1,
flexGrow: props.pageMode ? 0 : props.draggable && !completeDraggable ? 0 : 1,
flexShrink: 0,
}}
>
{!config.disableWebModeHistory && session && session.conversationId && (
Expand Down Expand Up @@ -622,6 +644,8 @@ ConversationCard.propTypes = {
notClampSize: PropTypes.bool,
pageMode: PropTypes.bool,
waitForTrigger: PropTypes.bool,
onToggleSidebar: PropTypes.func,
sidebarOpen: PropTypes.bool,
}

export default memo(ConversationCard)
161 changes: 106 additions & 55 deletions src/pages/IndependentPanel/App.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ import { openUrl } from '../../utils/index.mjs'
import Browser from 'webextension-polyfill'
import FileSaver from 'file-saver'

const logo = Browser.runtime.getURL('logo.png')

function App() {
const { t } = useTranslation()
const hasUserToggledRef = useRef(false)
const [collapsed, setCollapsed] = useState(true)
const config = useConfig(null, false)
const [sessions, setSessions] = useState([])
Expand Down Expand Up @@ -64,6 +67,22 @@ function App() {
})()
}, [])

useEffect(() => {
// Default open only on the standalone IndependentPanel (tab/window) when wide enough.
// Keep the existing side-panel behavior (closed by default).
void (async () => {
if (hasUserToggledRef.current) return
if (!window.matchMedia('(min-width: 900px)').matches) return
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The breakpoint value 900px is hardcoded and differs from the 600px breakpoint used elsewhere in the codebase (e.g., line 114 and styles.scss line 209). Consider defining these breakpoints as constants at the top of the file to ensure consistency and make future adjustments easier.

Copilot uses AI. Check for mistakes.
try {
// In a regular tab, this returns the current tab object; in the side panel it returns null.
const tab = await Browser.tabs.getCurrent()
if (!hasUserToggledRef.current && tab) setCollapsed(false)
} catch (e) {
// If we can't tell, keep current (closed) default.
}
})()
}, [])

useEffect(() => {
if ('sessions' in config && config['sessions']) setSessions(config['sessions'])
}, [config])
Expand All @@ -82,9 +101,21 @@ function App() {
}, [sessionId])

const toggleSidebar = () => {
hasUserToggledRef.current = true
setCollapsed(!collapsed)
}

const closeSidebar = () => {
hasUserToggledRef.current = true
setCollapsed(true)
}

const closeSidebarIfOverlay = () => {
if (window.matchMedia('(max-width: 600px)').matches) {
closeSidebar()
}
}

const createNewChat = async () => {
const { session, currentSessions } = await createSession()
setSessions(currentSessions)
Expand All @@ -105,62 +136,80 @@ function App() {

return (
<div className="IndependentPanel">
<div className="chat-container">
<div className={`chat-container ${collapsed ? 'sidebar-collapsed' : 'sidebar-open'}`}>
{!collapsed && (
<div className="chat-sidebar-backdrop" role="presentation" onClick={closeSidebar} />
)}
<div className={`chat-sidebar ${collapsed ? 'collapsed' : ''}`}>
<div className="chat-sidebar-button-group">
<button className="normal-button" onClick={toggleSidebar}>
{collapsed ? t('Pin') : t('Unpin')}
</button>
<button className="normal-button" onClick={createNewChat}>
{t('New Chat')}
</button>
<button className="normal-button" onClick={exportConversations}>
{t('Export')}
</button>
</div>
<hr />
<div className="chat-list">
{sessions.map(
(
session,
index, // TODO editable session name
) => (
<button
key={index}
className={`normal-button ${sessionId === session.sessionId ? 'active' : ''}`}
style="display: flex; align-items: center; justify-content: space-between;"
onClick={() => {
setSessionIdSafe(session.sessionId)
}}
>
{session.sessionName}
<span className="gpt-util-group">
<DeleteButton
size={14}
text={t('Delete Conversation')}
onConfirm={() =>
deleteSession(session.sessionId).then((sessions) => {
setSessions(sessions)
setSessionIdSafe(sessions[0].sessionId)
})
}
/>
</span>
</button>
),
)}
</div>
<hr />
<div className="chat-sidebar-button-group">
<ConfirmButton text={t('Clear conversations')} onConfirm={clearConversations} />
<button
className="normal-button"
onClick={() => {
openUrl(Browser.runtime.getURL('popup.html'))
}}
>
{t('Settings')}
</button>
<div className="chat-sidebar-content">
<div className="chat-sidebar-topbar">
<div className="chat-sidebar-brand-group">
<img className="chat-sidebar-logo" src={logo} alt="ChatGPTBox" />
<div className="chat-sidebar-brand">ChatGPTBox</div>
</div>
<button
type="button"
className="gpt-util-icon gpt-menu-toggle chat-sidebar-close"
aria-label="Close sidebar"
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Localize the sidebar close button aria label

The new drawer close button uses a hardcoded English aria-label ("Close sidebar") rather than the translation system. In non-English locales this leaves screen-reader-only text untranslated, which is a user-facing localization regression introduced by the new sidebar UI.

Useful? React with 👍 / 👎.

onClick={closeSidebar}
>
✕
</button>
Comment on lines +150 to +157
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Same i18n concern: hardcoded "Close sidebar" string.

The close button uses a hardcoded English aria-label. Consider using t() for consistency with the rest of the component.

🔎 Proposed fix 
               <button
                 type="button"
                 className="gpt-util-icon gpt-menu-toggle chat-sidebar-close"
-                aria-label="Close sidebar"
+                aria-label={t('Close sidebar')}
                 onClick={closeSidebar}
               >
                 ✕
               </button>
📝 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.

Suggested change
<button
type="button"
className="gpt-util-icon gpt-menu-toggle chat-sidebar-close"
aria-label="Close sidebar"
onClick={closeSidebar}
>
✕
</button>
<button
type="button"
className="gpt-util-icon gpt-menu-toggle chat-sidebar-close"
aria-label={t('Close sidebar')}
onClick={closeSidebar}
>
✕
</button>
🤖 Prompt for AI Agents 
In src/pages/IndependentPanel/App.jsx around lines 150 to 157, the close
button's aria-label is hardcoded as "Close sidebar"; replace this with the i18n
translator (t) so the label is localized (e.g. aria-label={t('closeSidebar')}),
ensure the t function is imported/available in this component (or pulled from
useTranslation hook) and add/update the 'closeSidebar' key in your translation
files as needed; keep the button behavior unchanged.

</div>
<div className="chat-sidebar-button-group">
<button className="normal-button" onClick={createNewChat}>
{t('New Chat')}
</button>
<button className="normal-button" onClick={exportConversations}>
{t('Export')}
</button>
</div>
<hr />
<div className="chat-list">
{sessions.map(
(
session,
index, // TODO editable session name
) => (
<button
key={index}
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using the array index as a key for list items is an anti-pattern in React, especially for lists where items can be added, removed, or reordered. This can lead to unpredictable UI behavior and state management issues. It's recommended to use a unique and stable identifier from your data, such as session.sessionId, to ensure React can correctly track each item.

Suggested change
key={index}
key={session.sessionId}

className={`normal-button ${sessionId === session.sessionId ? 'active' : ''}`}
style="display: flex; align-items: center; justify-content: space-between;"
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It's a best practice in React/JSX to provide styles as a camelCased object instead of a string. This improves consistency, maintainability, and leverages React's synthetic event system more effectively.

Suggested change
style="display: flex; align-items: center; justify-content: space-between;"
style={{ display: 'flex', alignItems: 'center', justifyContent: 'space-between' }}

onClick={() => {
setSessionIdSafe(session.sessionId)
closeSidebarIfOverlay()
Comment on lines +178 to +180
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent delete clicks from triggering session selection handler

On narrow layouts (max-width: 600px), clicking the trash control inside a session row also bubbles into the row’s onClick, which now calls closeSidebarIfOverlay(). Because DeleteButton does not stop click propagation, the drawer closes immediately when the user tries to delete a conversation, forcing an extra reopen/confirm cycle and making deletion interaction unreliable in the mobile drawer flow. The row handler should ignore nested control clicks (or the delete control should stop propagation) before closing the sidebar.

Useful? React with 👍 / 👎.

Comment on lines +178 to +180
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent delete clicks from closing the mobile drawer

In the session row click handler, selecting a conversation now also calls closeSidebarIfOverlay(), but clicks from nested controls (the trash icon/confirm control inside DeleteButton) still bubble to this handler. On narrow screens this closes the drawer when the user is trying to delete, so they must reopen the sidebar to finish the confirm flow. Guard this handler against nested control clicks (or stop propagation in the delete control) before closing the sidebar.

Useful? React with 👍 / 👎.

Comment on lines +178 to +180
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent delete clicks from closing the mobile drawer

The session row click handler now calls closeSidebarIfOverlay(), but clicks from nested controls (the trash/confirm controls rendered by DeleteButton) still bubble to that handler. On max-width: 600px, this closes the drawer while the user is trying to delete a conversation, forcing them to reopen the sidebar to complete the confirm flow. The row click path should ignore nested control clicks (or the nested control should stop propagation) before auto-closing.

Useful? React with 👍 / 👎.

}}
>
{session.sessionName}
<span className="gpt-util-group">
<DeleteButton
size={14}
text={t('Delete Conversation')}
onConfirm={() =>
deleteSession(session.sessionId).then((sessions) => {
setSessions(sessions)
setSessionIdSafe(sessions[0].sessionId)
})
}
/>
</span>
</button>
),
)}
Comment on lines +169 to +198
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Using array index as React key for session list.

Using index as a key for session buttons can cause reconciliation issues when sessions are reordered or deleted. Since each session has a unique sessionId, use that instead.

🔎 Proposed fix 
               {sessions.map(
                 (
                   session,
-                  index, // TODO editable session name
+                  // TODO editable session name
                 ) => (
                   <button
-                    key={index}
+                    key={session.sessionId}
                     className={`normal-button ${sessionId === session.sessionId ? 'active' : ''}`}
📝 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.

Suggested change
{sessions.map(
(
session,
index, // TODO editable session name
) => (
<button
key={index}
className={`normal-button ${sessionId === session.sessionId ? 'active' : ''}`}
style="display: flex; align-items: center; justify-content: space-between;"
onClick={() => {
setSessionIdSafe(session.sessionId)
closeSidebarIfOverlay()
}}
>
{session.sessionName}
<span className="gpt-util-group">
<DeleteButton
size={14}
text={t('Delete Conversation')}
onConfirm={() =>
deleteSession(session.sessionId).then((sessions) => {
setSessions(sessions)
setSessionIdSafe(sessions[0].sessionId)
})
}
/>
</span>
</button>
),
)}
{sessions.map(
(
session,
// TODO editable session name
) => (
<button
key={session.sessionId}
className={`normal-button ${sessionId === session.sessionId ? 'active' : ''}`}
style="display: flex; align-items: center; justify-content: space-between;"
onClick={() => {
setSessionIdSafe(session.sessionId)
closeSidebarIfOverlay()
}}
>
{session.sessionName}
<span className="gpt-util-group">
<DeleteButton
size={14}
text={t('Delete Conversation')}
onConfirm={() =>
deleteSession(session.sessionId).then((sessions) => {
setSessions(sessions)
setSessionIdSafe(sessions[0].sessionId)
})
}
/>
</span>
</button>
),
)}
🤖 Prompt for AI Agents 
In src/pages/IndependentPanel/App.jsx around lines 169 to 198, the list buttons
use the array index as the React key which can break reconciliation when
sessions are reordered or removed; change the key to use the stable unique
identifier session.sessionId instead (i.e., key={session.sessionId}) so React
can correctly track items during updates.

</div>
<hr />
<div className="chat-sidebar-button-group">
<ConfirmButton text={t('Clear conversations')} onConfirm={clearConversations} />
<button
className="normal-button"
onClick={() => {
openUrl(Browser.runtime.getURL('popup.html'))
closeSidebarIfOverlay()
}}
>
{t('Settings')}
</button>
</div>
</div>
</div>
<div className="chat-content">
Expand All @@ -170,6 +219,8 @@ function App() {
session={currentSession}
notClampSize={true}
pageMode={true}
sidebarOpen={!collapsed}
onToggleSidebar={toggleSidebar}
onUpdate={(port, session, cData) => {
currentPort.current = port
if (cData.length > 0 && cData[cData.length - 1].done) {
Expand Down
Loading