* fix(mobile): always mount SessionSidebar to eliminate >10s drawer open delay (#1695) On mobile (Android PWA), SessionSidebar was conditionally mounted via {mobileLeftDrawerVisible && ...}, causing the component to unmount on drawer close and remount on every open. Each remount fired a full data-loading cascade: paginated sessions fetch (PAGE_SIZE=500 with retry), worktree discovery, repo status, PR status, 10+ useMemo recomputations, and localStorage reads, manifesting as a >10s delay before the drawer became interactive. Desktop already avoided this by keeping SessionSidebar always mounted inside <Sidebar> with a CSS visibility toggle. Fix: remove the mobileLeftDrawerVisible conditional wrapper so SessionSidebar stays mounted on mobile too, matching desktop behavior. Visibility remains controlled by the leftDrawerX transform (off-screen when closed). Added pointer-events-none when hidden as a defensive guard. Added a regression test that fails if the conditional mount pattern is reintroduced around the mobile SessionSidebar. * fix(mobile): hide closed drawer to avoid rotation offset leak --------- Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
This commit is contained in:
committed by
GitHub
co-authored by
Bohdan Triapitsyn
parent
c9ac8676e7
commit
e8be7ef55b
@@ -457,13 +457,30 @@ export const MainLayout: React.FC = () => {
|
|||||||
</ErrorBoundary>
|
</ErrorBoundary>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
{mobileLeftDrawerVisible && (
|
{/* Always mount SessionSidebar on mobile to match desktop behavior.
|
||||||
<motion.div className="absolute inset-0 z-20 bg-sidebar" data-page-scroll-lock="true" style={{ x: leftDrawerX }} aria-hidden={!mobileLeftDrawerOpen}>
|
Conditional mount (mobileLeftDrawerVisible && ...) caused a
|
||||||
<ErrorBoundary>
|
data-loading cascade on every drawer open: paginated sessions
|
||||||
<SessionSidebar mobileVariant />
|
fetch, worktree discovery, repo status, PR status, and 10+ memo
|
||||||
</ErrorBoundary>
|
recomputations. On Android PWA this manifested as a >10s delay
|
||||||
</motion.div>
|
before the drawer became interactive (issue #1695). Visibility is
|
||||||
)}
|
controlled by the leftDrawerX transform (off-screen when closed).
|
||||||
|
The invisible class matters when fully hidden: leftDrawerWidth is
|
||||||
|
not recomputed on resize/rotation, so a closed drawer translated by
|
||||||
|
the old width could otherwise peek into the viewport; it also keeps
|
||||||
|
the off-screen sidebar out of the tab order and skips painting it. */}
|
||||||
|
<motion.div
|
||||||
|
className={cn(
|
||||||
|
'absolute inset-0 z-20 bg-sidebar',
|
||||||
|
!mobileLeftDrawerVisible && 'pointer-events-none invisible',
|
||||||
|
)}
|
||||||
|
data-page-scroll-lock="true"
|
||||||
|
style={{ x: leftDrawerX }}
|
||||||
|
aria-hidden={!mobileLeftDrawerOpen}
|
||||||
|
>
|
||||||
|
<ErrorBoundary>
|
||||||
|
<SessionSidebar mobileVariant />
|
||||||
|
</ErrorBoundary>
|
||||||
|
</motion.div>
|
||||||
{mobileRightDrawerVisible && (
|
{mobileRightDrawerVisible && (
|
||||||
<motion.div className="absolute inset-0 z-20 bg-sidebar" data-page-scroll-lock="true" style={{ x: rightDrawerX }} aria-hidden={!mobileRightSidebarOpen}>
|
<motion.div className="absolute inset-0 z-20 bg-sidebar" data-page-scroll-lock="true" style={{ x: rightDrawerX }} aria-hidden={!mobileRightSidebarOpen}>
|
||||||
<ErrorBoundary>
|
<ErrorBoundary>
|
||||||
|
|||||||
@@ -0,0 +1,35 @@
|
|||||||
|
import { describe, expect, test } from 'bun:test';
|
||||||
|
import { readFileSync } from 'node:fs';
|
||||||
|
import { dirname, join } from 'node:path';
|
||||||
|
import { fileURLToPath } from 'node:url';
|
||||||
|
|
||||||
|
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||||
|
const mainLayoutSource = readFileSync(
|
||||||
|
join(__dirname, '..', 'MainLayout.tsx'),
|
||||||
|
'utf-8',
|
||||||
|
);
|
||||||
|
|
||||||
|
describe('MainLayout mobile SessionSidebar mount (issue #1695 regression guard)', () => {
|
||||||
|
test('mobile SessionSidebar is not conditionally mounted on mobileLeftDrawerVisible', () => {
|
||||||
|
const mobileSidebarIndex = mainLayoutSource.indexOf('<SessionSidebar mobileVariant');
|
||||||
|
expect(mobileSidebarIndex).toBeGreaterThan(-1);
|
||||||
|
|
||||||
|
const windowStart = Math.max(0, mobileSidebarIndex - 400);
|
||||||
|
const precedingWindow = mainLayoutSource.slice(windowStart, mobileSidebarIndex);
|
||||||
|
|
||||||
|
expect(/\{\s*mobileLeftDrawerVisible\s*&&\s*\(/.test(precedingWindow)).toBe(false);
|
||||||
|
|
||||||
|
expect(precedingWindow.includes('pointer-events-none')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('desktop SessionSidebar is rendered inside Sidebar without drawer-visibility gating', () => {
|
||||||
|
const desktopSidebarIndex = mainLayoutSource.indexOf('<SessionSidebar />');
|
||||||
|
expect(desktopSidebarIndex).toBeGreaterThan(-1);
|
||||||
|
|
||||||
|
const windowStart = Math.max(0, desktopSidebarIndex - 300);
|
||||||
|
const precedingWindow = mainLayoutSource.slice(windowStart, desktopSidebarIndex);
|
||||||
|
|
||||||
|
expect(precedingWindow).toContain('<Sidebar');
|
||||||
|
expect(/mobileLeftDrawerVisible\s*&&/.test(precedingWindow)).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user