-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(web-shell): add mobile sidebar drawer with session list #6003
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.
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
Changes from all commits
227c121
ea5062b
4cf8be7
801af24
44f848d
763a56b
0209fa0
fd08ddd
9571668
a18ad9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -93,6 +93,7 @@ import { | |||||||||||||
| copyFromLastAssistantMessage, | ||||||||||||||
| COPY_MESSAGES, | ||||||||||||||
| } from './utils/copyCommand'; | ||||||||||||||
| import { isEditableTarget } from './utils/dom'; | ||||||||||||||
| import { getModelDisplayName } from './utils/modelDisplay'; | ||||||||||||||
| import { filterModelSwitchMessages } from './utils/modelSwitchMessages'; | ||||||||||||||
| import { decideEscapeIntent } from './utils/escapeIntent'; | ||||||||||||||
|
|
@@ -808,6 +809,63 @@ export function App({ | |||||||||||||
| const [sidebarSwitchingSessionId, setSidebarSwitchingSessionId] = useState< | ||||||||||||||
| string | null | ||||||||||||||
| >(null); | ||||||||||||||
| const [mobileDrawerOpen, setMobileDrawerOpen] = useState(false); | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| const closeMobileDrawer = useCallback(() => setMobileDrawerOpen(false), []); | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
|
|
||||||||||||||
| useEffect(() => { | ||||||||||||||
| const mql = window.matchMedia('(max-width: 760px)'); | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| const handler = (e: MediaQueryListEvent) => { | ||||||||||||||
| if (!e.matches) setMobileDrawerOpen(false); | ||||||||||||||
| }; | ||||||||||||||
| mql.addEventListener('change', handler); | ||||||||||||||
| return () => mql.removeEventListener('change', handler); | ||||||||||||||
| }, []); | ||||||||||||||
|
|
||||||||||||||
| useEffect(() => { | ||||||||||||||
| if (!mobileDrawerOpen) return; | ||||||||||||||
| const onKey = (e: KeyboardEvent) => { | ||||||||||||||
| if (e.key !== 'Escape') return; | ||||||||||||||
| // A pending tool/permission approval owns Escape (it rejects the call), | ||||||||||||||
| // so don't let the drawer swallow it while a prompt is visible. | ||||||||||||||
| if (pendingApprovalRef.current) return; | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] When the drawer is open and a tool/permission approval becomes pending, the approval UI renders inside Consider auto-closing the drawer when an approval appears: useEffect(() => {
if (pendingApproval) closeMobileDrawer();
}, [pendingApproval, closeMobileDrawer]);This ensures the approval UI is always accessible when it matters. — qwen3.7-max via Qwen Code /review |
||||||||||||||
| const target = e.target as HTMLElement | null; | ||||||||||||||
| // Only let an editable element keep Escape for itself when it lives | ||||||||||||||
| // outside the drawer; the drawer's own search input should still close | ||||||||||||||
| // the drawer on the first Escape. | ||||||||||||||
| if ( | ||||||||||||||
| isEditableTarget(target) && | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The editable-target guard only bails for inputs outside the drawer ( The search-input case appears intentional per the code comment, but the rename case is a data-loss edge case. Simplifying the guard to bail for all editable targets (regardless of drawer ancestry) lets both inputs handle Escape first — the drawer closes on the second Escape:
Suggested change
— qwen3.7-max via Qwen Code /review |
||||||||||||||
| !target?.closest('[data-mobile-drawer]') | ||||||||||||||
| ) { | ||||||||||||||
| return; | ||||||||||||||
| } | ||||||||||||||
| e.stopPropagation(); | ||||||||||||||
| e.preventDefault(); | ||||||||||||||
| closeMobileDrawer(); | ||||||||||||||
| }; | ||||||||||||||
| const prevOverflow = document.body.style.overflow; | ||||||||||||||
| document.body.style.overflow = 'hidden'; | ||||||||||||||
| const preventScroll = (e: TouchEvent) => { | ||||||||||||||
| // Allow native scrolling inside the drawer panel (e.g. the session list). | ||||||||||||||
| // The dim backdrop also lives under [data-mobile-drawer], so exclude it: | ||||||||||||||
| // a touchmove starting on the backdrop must still be blocked, otherwise | ||||||||||||||
| // iOS Safari scrolls the page behind the open drawer. | ||||||||||||||
| const el = e.target as HTMLElement | null; | ||||||||||||||
| if ( | ||||||||||||||
| el?.closest('[data-mobile-drawer]') && | ||||||||||||||
| !el.closest(`.${styles.mobileBackdrop}`) | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The Consider also exempting
Suggested change
— glm-5.2 via Qwen Code /review
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The backdrop exclusion in the If
Suggested change
And add <div data-mobile-backdrop="" className={styles.mobileBackdrop} onClick={closeMobileDrawer} aria-hidden="true" />— qwen3.7-max via Qwen Code /review |
||||||||||||||
| ) { | ||||||||||||||
| return; | ||||||||||||||
| } | ||||||||||||||
| e.preventDefault(); | ||||||||||||||
| }; | ||||||||||||||
| document.addEventListener('touchmove', preventScroll, { passive: false }); | ||||||||||||||
| window.addEventListener('keydown', onKey, true); | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| return () => { | ||||||||||||||
| document.body.style.overflow = prevOverflow; | ||||||||||||||
| document.removeEventListener('touchmove', preventScroll); | ||||||||||||||
| window.removeEventListener('keydown', onKey, true); | ||||||||||||||
| }; | ||||||||||||||
| }, [mobileDrawerOpen, closeMobileDrawer]); | ||||||||||||||
| const handleSidebarCollapsedChange = useCallback((collapsed: boolean) => { | ||||||||||||||
| setSidebarCollapsed(collapsed); | ||||||||||||||
| writeSidebarCollapsed(collapsed); | ||||||||||||||
|
|
@@ -2074,6 +2132,9 @@ export function App({ | |||||||||||||
| }, [branchCurrentSession]); | ||||||||||||||
|
|
||||||||||||||
| const createNewSession = useCallback(async () => { | ||||||||||||||
| // Close the drawer before awaiting so a failed createSession() doesn't leave | ||||||||||||||
| // it stuck open with the page scroll still locked, matching loadSidebarSession. | ||||||||||||||
| closeMobileDrawer(); | ||||||||||||||
| try { | ||||||||||||||
| const session = await ( | ||||||||||||||
| sessionActions as typeof sessionActions & SessionActionsWithCreate | ||||||||||||||
|
|
@@ -2092,11 +2153,15 @@ export function App({ | |||||||||||||
| reportError(error, 'Failed to create a new session'); | ||||||||||||||
| return false; | ||||||||||||||
| } | ||||||||||||||
| }, [onSessionIdChange, reportError, sessionActions]); | ||||||||||||||
| }, [closeMobileDrawer, onSessionIdChange, reportError, sessionActions]); | ||||||||||||||
|
|
||||||||||||||
| const loadSidebarSession = useCallback( | ||||||||||||||
| async (sessionId: string) => { | ||||||||||||||
| setSidebarSwitchingSessionId(sessionId); | ||||||||||||||
| // Close the drawer before awaiting the load so it doesn't linger over the | ||||||||||||||
| // old transcript while the new session streams in, matching the other | ||||||||||||||
| // session-switch paths (/resume, ResumeDialog). | ||||||||||||||
| closeMobileDrawer(); | ||||||||||||||
| try { | ||||||||||||||
| await sessionActions.loadSession(sessionId, { | ||||||||||||||
| deferTranscriptReset: true, | ||||||||||||||
|
|
@@ -2108,7 +2173,7 @@ export function App({ | |||||||||||||
| throw error; | ||||||||||||||
| } | ||||||||||||||
| }, | ||||||||||||||
| [sessionActions], | ||||||||||||||
| [closeMobileDrawer, sessionActions], | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| useEffect(() => { | ||||||||||||||
|
|
@@ -2838,10 +2903,12 @@ export function App({ | |||||||||||||
| if (cmd === 'resume') { | ||||||||||||||
| const sessionId = text.slice(match[0].length).trim(); | ||||||||||||||
| if (sessionId) { | ||||||||||||||
| closeMobileDrawer(); | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| sessionActions.loadSession(sessionId).catch((error: unknown) => { | ||||||||||||||
| reportError(error, 'Failed to load session'); | ||||||||||||||
| }); | ||||||||||||||
| } else { | ||||||||||||||
| closeMobileDrawer(); | ||||||||||||||
| setShowResumeDialog(true); | ||||||||||||||
| } | ||||||||||||||
| return true; | ||||||||||||||
|
|
@@ -3021,6 +3088,7 @@ export function App({ | |||||||||||||
| enqueuePrompt, | ||||||||||||||
| echoOrDeferLocalCommand, | ||||||||||||||
| branchCurrentSession, | ||||||||||||||
| closeMobileDrawer, | ||||||||||||||
| createNewSession, | ||||||||||||||
| handleBusyGoalClear, | ||||||||||||||
| handleGoalSlashCommand, | ||||||||||||||
|
|
@@ -3532,6 +3600,7 @@ export function App({ | |||||||||||||
| > | ||||||||||||||
| <ResumeDialog | ||||||||||||||
| onSelect={(sessionId) => { | ||||||||||||||
| closeMobileDrawer(); | ||||||||||||||
| sessionActions | ||||||||||||||
| .loadSession(sessionId) | ||||||||||||||
| .catch((error: unknown) => { | ||||||||||||||
|
|
@@ -3804,16 +3873,62 @@ export function App({ | |||||||||||||
|
|
||||||||||||||
| <div className={styles.appShell}> | ||||||||||||||
| {sidebarOptions.enabled && ( | ||||||||||||||
| <WebShellSidebar | ||||||||||||||
| collapsed={sidebarCollapsed} | ||||||||||||||
| onCollapsedChange={handleSidebarCollapsedChange} | ||||||||||||||
| onOpenSettings={() => setShowSettingsDialog(true)} | ||||||||||||||
| onNewSession={createNewSession} | ||||||||||||||
| onLoadSession={loadSidebarSession} | ||||||||||||||
| onError={reportError} | ||||||||||||||
| /> | ||||||||||||||
| <div | ||||||||||||||
| data-mobile-drawer="" | ||||||||||||||
| {...(mobileDrawerOpen | ||||||||||||||
| ? { role: 'dialog', 'aria-modal': 'true' as const } | ||||||||||||||
| : {})} | ||||||||||||||
| aria-label={t('sidebar.label')} | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| className={[ | ||||||||||||||
| styles.mobileDrawer, | ||||||||||||||
| mobileDrawerOpen ? styles.mobileDrawerOpen : undefined, | ||||||||||||||
| ] | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| .filter(Boolean) | ||||||||||||||
| .join(' ')} | ||||||||||||||
| > | ||||||||||||||
| <div | ||||||||||||||
| className={styles.mobileBackdrop} | ||||||||||||||
| onClick={closeMobileDrawer} | ||||||||||||||
| aria-hidden="true" | ||||||||||||||
| /> | ||||||||||||||
| <WebShellSidebar | ||||||||||||||
| collapsed={sidebarCollapsed && !mobileDrawerOpen} | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| onCollapsedChange={handleSidebarCollapsedChange} | ||||||||||||||
| onOpenSettings={() => { | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| closeMobileDrawer(); | ||||||||||||||
| setShowSettingsDialog(true); | ||||||||||||||
| }} | ||||||||||||||
| onNewSession={createNewSession} | ||||||||||||||
| onLoadSession={loadSidebarSession} | ||||||||||||||
| onError={reportError} | ||||||||||||||
| mobileOpen={mobileDrawerOpen} | ||||||||||||||
| /> | ||||||||||||||
| </div> | ||||||||||||||
| )} | ||||||||||||||
| <div className={styles.chatPane}> | ||||||||||||||
| {sidebarOptions.enabled && ( | ||||||||||||||
| <button | ||||||||||||||
| type="button" | ||||||||||||||
| className={styles.hamburgerButton} | ||||||||||||||
|
pomelo-nwu marked this conversation as resolved.
pomelo-nwu marked this conversation as resolved.
|
||||||||||||||
| onClick={() => setMobileDrawerOpen((open) => !open)} | ||||||||||||||
| aria-label={t('sidebar.toggleMenu')} | ||||||||||||||
| aria-expanded={mobileDrawerOpen} | ||||||||||||||
| > | ||||||||||||||
| <svg | ||||||||||||||
| viewBox="0 0 24 24" | ||||||||||||||
| fill="none" | ||||||||||||||
| stroke="currentColor" | ||||||||||||||
| strokeWidth="2" | ||||||||||||||
| strokeLinecap="round" | ||||||||||||||
| strokeLinejoin="round" | ||||||||||||||
| aria-hidden="true" | ||||||||||||||
| > | ||||||||||||||
| <line x1="3" y1="6" x2="21" y2="6" /> | ||||||||||||||
| <line x1="3" y1="12" x2="21" y2="12" /> | ||||||||||||||
| <line x1="3" y1="18" x2="21" y2="18" /> | ||||||||||||||
| </svg> | ||||||||||||||
| </button> | ||||||||||||||
| )} | ||||||||||||||
| <WebShellCustomizationProvider value={customization}> | ||||||||||||||
| <CompactModeContext.Provider value={compactMode}> | ||||||||||||||
| <TodoContextsProvider | ||||||||||||||
|
|
||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.