Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Contributor

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"
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;" />

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 } : {}
}
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
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
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"
onClick={closeSidebar}
>
</button>
Comment on lines +150 to +157
Copy link
Contributor

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}

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;"

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()
}}
>
{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
Contributor

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