diff --git a/frontend/src/__tests__/components/BugReportTriggerPlacement.test.tsx b/frontend/src/__tests__/components/BugReportTriggerPlacement.test.tsx new file mode 100644 index 000000000..674a2452a --- /dev/null +++ b/frontend/src/__tests__/components/BugReportTriggerPlacement.test.tsx @@ -0,0 +1,206 @@ +/** + * Where the bug-report trigger lives, and what shape its panel takes (#2750, + * reporter @goodjaltman). + * + * The floating disc is pinned to the bottom-right corner, which is the most + * contended region in the app — the Profiles scroll-to-top FAB, the floating + * camera window, the Group Edit save bar, the bulk-selection toolbars and the + * per-card action buttons on File Manager and Archives all sit there, and a + * `fixed` disc covers whichever happens to be underneath at the time. Below the + * sidebar-compact breakpoint the trigger moves into the compact header instead, + * which frees the corner outright. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { waitFor, fireEvent, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { render } from '../utils'; +import { Layout } from '../../components/Layout'; +import { BugReportBubble } from '../../components/BugReportBubble'; +import { http, HttpResponse } from 'msw'; +import { server } from '../mocks/server'; + +/** The always-false stub from setup.ts, restored after each test. */ +const defaultMatchMedia = (query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: () => {}, + removeListener: () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + dispatchEvent: () => true, +}); + +/** + * Pretend the viewport is ``width`` px wide. + * + * Both breakpoint hooks read `window.innerWidth` for their initial value and + * `matchMedia('(max-width: Npx)')` thereafter, so the stub has to answer the + * query rather than return a fixed boolean — useIsMobile (768) and + * useIsSidebarCompact (1144) ask different questions and a flat `true` would + * conflate them. + */ +function stubViewport(width: number) { + Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: width }); + Object.defineProperty(window, 'matchMedia', { + writable: true, + configurable: true, + value: (query: string) => { + const max = /max-width:\s*(\d+)px/.exec(query); + return { ...defaultMatchMedia(query), matches: max ? width <= Number(max[1]) : false }; + }, + }); +} + +function setupLayoutHandlers() { + server.use( + http.get('/api/v1/printers/', () => HttpResponse.json([])), + http.get('/api/v1/printers/:id/status', () => HttpResponse.json({ connected: true, state: 'IDLE' })), + http.get('/api/v1/version', () => HttpResponse.json({ version: '0.1.6', build: 'test' })), + http.get('/api/v1/settings/', () => + HttpResponse.json({ check_updates: false, check_printer_firmware: false, auto_archive: true }), + ), + http.get('/api/v1/external-links/', () => HttpResponse.json([])), + http.get('/api/v1/smart-plugs/', () => HttpResponse.json([])), + http.get('/api/v1/support/debug-logging', () => HttpResponse.json({ enabled: false })), + http.get('/api/v1/queue/', () => HttpResponse.json([])), + http.get('/api/v1/pending-uploads/count', () => HttpResponse.json({ count: 0 })), + http.get('/api/v1/updates/check', () => HttpResponse.json({ update_available: false })), + http.get('/api/v1/auth/status', () => HttpResponse.json({ auth_enabled: false, requires_setup: false })), + http.get('/api/v1/printers/developer-mode-warnings', () => HttpResponse.json([])), + http.get('/api/v1/system/health', () => HttpResponse.json({ findings: [] })), + ); +} + +/** The floating disc, identified by the shape only it has. */ +const floatingDisc = () => + Array.from(document.querySelectorAll('button')).find( + (b) => b.className.includes('rounded-full') && b.className.includes('bottom-4'), + ); + +describe('bug-report trigger placement', () => { + beforeEach(() => { + vi.mocked(localStorage.getItem).mockReturnValue(null); + setupLayoutHandlers(); + }); + + afterEach(() => { + Object.defineProperty(window, 'matchMedia', { + writable: true, + configurable: true, + value: defaultMatchMedia, + }); + }); + + it('puts the trigger in the compact header and frees the corner below 1144px', async () => { + stubViewport(390); + render(); + + await waitFor(() => expect(document.querySelector('header')).toBeInTheDocument()); + const header = document.querySelector('header')!; + expect(within(header).getByRole('button', { name: /report a bug|bug/i })).toBeInTheDocument(); + expect(floatingDisc()).toBeUndefined(); + }); + + it('keeps the floating disc when the sidebar is not compact', async () => { + stubViewport(1440); + render(); + + await waitFor(() => expect(floatingDisc()).toBeDefined()); + // No compact header exists at this width, so there is nowhere else for it. + expect(document.querySelector('header')).toBeNull(); + }); + + it('opens the same panel from the header trigger', async () => { + stubViewport(390); + render(); + + await waitFor(() => expect(document.querySelector('header')).toBeInTheDocument()); + const header = document.querySelector('header')!; + fireEvent.click(within(header).getByRole('button', { name: /report a bug|bug/i })); + + await waitFor(() => expect(document.getElementById('bug-report-modal')).toBeInTheDocument()); + }); +}); + +describe('bug-report panel geometry', () => { + afterEach(() => { + Object.defineProperty(window, 'matchMedia', { + writable: true, + configurable: true, + value: defaultMatchMedia, + }); + }); + + it('is a full-width bottom sheet on a phone', async () => { + // The regression: `fixed ... right-4 w-full max-w-md` resolves w-full + // against the viewport, so on a 390px screen the panel was 390px wide and + // then inset 16px from the right — putting its left edge at -16px and + // cutting a strip of the form off-screen. max-w-md hid this above ~464px. + stubViewport(390); + render( {}} />); + + const panel = await waitFor(() => { + const el = document.getElementById('bug-report-modal'); + expect(el).toBeInTheDocument(); + return el!; + }); + expect(panel.className).toContain('inset-x-0'); + expect(panel.className).not.toContain('right-4'); + expect(panel.className).not.toContain('w-full'); + }); + + it('anchors under the header when the trigger lives there (tablet band)', async () => { + // 768-1143px: past the bottom-sheet breakpoint but the trigger is in the + // compact header, so the panel must not open in a corner the user never + // touched. + stubViewport(900); + render( {}} />); + + const panel = await waitFor(() => { + const el = document.getElementById('bug-report-modal'); + expect(el).toBeInTheDocument(); + return el!; + }); + expect(panel.className).toContain('top-16'); + expect(panel.className).not.toContain('bottom-20'); + }); + + it('keeps the anchored card on desktop', async () => { + stubViewport(1440); + render( {}} />); + + const panel = await waitFor(() => { + const el = document.getElementById('bug-report-modal'); + expect(el).toBeInTheDocument(); + return el!; + }); + expect(panel.className).toContain('right-4'); + expect(panel.className).toContain('max-w-md'); + }); +}); + +describe('bug-report form reset', () => { + it('clears a half-filled form when reopened from a controlled trigger', async () => { + // The reset used to live in the floating disc's click handler. With the + // header trigger only flipping a controlled flag, that would have left the + // previous draft sitting there for compact layouts. + const user = userEvent.setup(); + const { rerender } = render( {}} />); + + const textarea = await waitFor(() => document.querySelector('textarea')!); + await user.type(textarea, 'half-written report'); + expect(textarea).toHaveValue('half-written report'); + + rerender( {}} />); + rerender( {}} />); + + await waitFor(() => expect(document.querySelector('textarea')).toHaveValue('')); + }); + + it('renders no floating disc when the trigger is hosted elsewhere', () => { + render(); + expect(floatingDisc()).toBeUndefined(); + }); +}); diff --git a/frontend/src/components/BugReportBubble.tsx b/frontend/src/components/BugReportBubble.tsx index ebc3379eb..8a886b296 100644 --- a/frontend/src/components/BugReportBubble.tsx +++ b/frontend/src/components/BugReportBubble.tsx @@ -6,6 +6,7 @@ import { api, bugReportApi, type PrinterDiagnosticResult } from '../api/client'; import { DiagnosticChecklist } from './ConnectionDiagnostic'; import { SystemHealthPanel } from './SystemHealthPanel'; import { Collapsible } from './Collapsible'; +import { useIsMobile } from '../hooks/useIsMobile'; type ViewState = 'form' | 'logging' | 'stopping' | 'submitting' | 'success' | 'error'; @@ -47,9 +48,36 @@ function formatElapsed(seconds: number): string { return `${m.toString().padStart(2, '0')}:${s.toString().padStart(2, '0')}`; } -export function BugReportBubble() { +interface BugReportBubbleProps { + /** + * Render the floating disc in the bottom-right corner. False when the + * trigger lives somewhere else — the compact header does this (#2750), so + * the panel still mounts here while the button that opens it sits in the + * header. The panel deliberately stays at the Layout root rather than + * moving into the header with its button: the header is a ``fixed z-40`` + * element and therefore its own stacking context, so a ``z-50`` panel + * nested inside it would be capped at the header's level and end up + * underneath every ordinary z-50 modal in the app. + */ + showTrigger?: boolean; + /** Controlled open state. Falls back to internal state when omitted. */ + open?: boolean; + onOpenChange?: (open: boolean) => void; +} + +export function BugReportBubble({ showTrigger = true, open, onOpenChange }: BugReportBubbleProps = {}) { const { t } = useTranslation(); - const [isOpen, setIsOpen] = useState(false); + const isMobile = useIsMobile(); + const [internalOpen, setInternalOpen] = useState(false); + const isControlled = open !== undefined; + const isOpen = isControlled ? open : internalOpen; + const setIsOpen = useCallback( + (next: boolean) => { + if (!isControlled) setInternalOpen(next); + onOpenChange?.(next); + }, + [isControlled, onOpenChange], + ); const [viewState, setViewState] = useState('form'); const [description, setDescription] = useState(''); const [email, setEmail] = useState(''); @@ -109,8 +137,12 @@ export function BugReportBubble() { return () => clearTimeout(timer); }, [viewState, elapsedSeconds]); - const handleOpen = () => { - setIsOpen(true); + // Reset on open rather than in the click handler: the panel now has two + // possible triggers (the floating disc here, and the compact header's button + // which only flips the controlled flag), and a stale half-filled form + // reappearing for one of them would be a nasty little inconsistency. + useEffect(() => { + if (!isOpen) return; setViewState('form'); setDescription(''); setEmail(''); @@ -120,7 +152,9 @@ export function BugReportBubble() { setErrorMessage(''); setElapsedSeconds(0); setWasDebug(false); - }; + }, [isOpen]); + + const handleOpen = () => setIsOpen(true); const handleClose = () => { setIsOpen(false); @@ -216,25 +250,47 @@ export function BugReportBubble() { return ( <> - {/* Floating bubble */} - + {/* Floating bubble. Absent below the sidebar-compact breakpoint, where + the compact header carries the trigger instead — see Layout. */} + {showTrigger && ( + + )} - {/* Slide-in panel anchored to bottom-right */} + {/* Slide-in panel anchored to bottom-right; a bottom sheet on phones. + The desktop geometry cannot be reused there: `w-full` resolves against + the viewport for a fixed element, so on a 375px screen the panel was + 375px wide and then pushed 16px in from the right, putting its left + edge at -16px and cutting a strip of the form off-screen. `max-w-md` + hid this on anything above ~464px wide. */} {isOpen && (
{/* Header */}
diff --git a/frontend/src/components/Layout.tsx b/frontend/src/components/Layout.tsx index 2b3584a36..d1bc41ec8 100644 --- a/frontend/src/components/Layout.tsx +++ b/frontend/src/components/Layout.tsx @@ -73,6 +73,17 @@ export function Layout() { const { t } = useTranslation(); const isSidebarCompact = useIsSidebarCompact(); + // Bug-report panel state lives here because the trigger moves (#2750, + // reporter @goodjaltman). Below the sidebar-compact breakpoint the floating + // disc is replaced by a button in the compact header: the bottom-right corner + // is the most contended region in the app — the Profiles scroll-to-top FAB, + // the floating camera window and its resize handle, the Group Edit save bar, + // the bulk-selection toolbars, and the File Manager / Archives per-card + // action buttons all live there — and a fixed 48px disc sits on top of + // whichever of them happens to be underneath. Moving out of the corner is + // the only fix that covers in-flow content as well as fixed overlays. + const [bugReportOpen, setBugReportOpen] = useState(false); + // Theme toggle: mode → icon and tooltip const ThemeIcon = { dark: Sun, light: Monitor, system: Moon }[mode]; const themeSwitchTitle = t({ dark: 'nav.switchToLight', light: 'nav.switchToSystem', system: 'nav.switchToDark' }[mode]); @@ -494,6 +505,15 @@ export function Layout() { alt="Bambuddy" className="h-8 ml-3" /> + {/* Bug report — the compact-layout home of the floating bubble. */} + )} @@ -1108,7 +1128,16 @@ export function Layout() {
)} - + {/* The panel always mounts here, at the Layout root. It must not move + into the header alongside its compact-layout trigger: the header is + `fixed z-40` and so its own stacking context, which would cap the + z-50 panel at the header's level and bury it under every ordinary + modal in the app. */} +
); } diff --git a/frontend/src/pages/ProfilesPage.tsx b/frontend/src/pages/ProfilesPage.tsx index be169b7ce..bc97b0f51 100644 --- a/frontend/src/pages/ProfilesPage.tsx +++ b/frontend/src/pages/ProfilesPage.tsx @@ -378,10 +378,13 @@ function ScrollToTop() { if (!isVisible) return null; + // bottom-24, not bottom-6: at 24px this sat under the bug-report bubble + // (16-64px from both edges), covering ~40x40 of its 44x44. Both are z-40, so + // which one you could actually click came down to DOM order (#2750). return (