diff --git a/desktop/src/components/chat/ImageGalleryModal.test.tsx b/desktop/src/components/chat/ImageGalleryModal.test.tsx index 388494de..eb899ae4 100644 --- a/desktop/src/components/chat/ImageGalleryModal.test.tsx +++ b/desktop/src/components/chat/ImageGalleryModal.test.tsx @@ -203,11 +203,11 @@ describe('ImageGalleryModal · closer look', () => { it('starts each picture fitted: the zoom of the last one is not the next one\'s', () => { const { rerender } = render( {}} onSelect={() => {}} />) fireEvent.click(screen.getByRole('button', { name: 'Zoom in' })) - expect(screen.getByRole('button', { name: 'Fit to window' })).not.toHaveAttribute('aria-pressed', 'true') + expect(screen.getByRole('button', { name: 'Fit to window' })).toBeEnabled() rerender( {}} onSelect={() => {}} />) - expect(screen.getByRole('button', { name: 'Fit to window' })).toHaveAttribute('aria-pressed', 'true') + expect(screen.getByRole('button', { name: 'Fit to window' })).toBeDisabled() }) }) diff --git a/desktop/src/components/ui/ZoomControls.test.tsx b/desktop/src/components/ui/ZoomControls.test.tsx index b9805a2d..aedcb8e4 100644 --- a/desktop/src/components/ui/ZoomControls.test.tsx +++ b/desktop/src/components/ui/ZoomControls.test.tsx @@ -59,14 +59,32 @@ describe('ZoomControls', () => { expect(screen.getByRole('button', { name: 'Fit to window' })).toBeEnabled() }) - it('marks fit as pressed while the viewer is fitting', () => { - const { rerender } = renderControls({ fitActive: true }) - expect(screen.getByRole('button', { name: 'Fit to window' })).toHaveAttribute('aria-pressed', 'true') + it('offers fit only once the viewer has left it, as an action rather than a toggle', () => { + const { rerender, onFit } = renderControls({ fitActive: true }) + const fit = () => screen.getByRole('button', { name: 'Fit to window' }) + // Already fitted: clicking would change nothing, so it cannot be clicked. + expect(fit()).toBeDisabled() + expect(fit()).not.toHaveAttribute('aria-pressed') + fireEvent.click(fit()) + expect(onFit).not.toHaveBeenCalled() rerender( - , + , ) - expect(screen.getByRole('button', { name: 'Fit to window' })).toHaveAttribute('aria-pressed', 'false') + expect(fit()).toBeEnabled() + expect(fit()).not.toHaveAttribute('aria-pressed') + }) + + it('draws fit with an icon of its own, never the maximize arrows of the panel control', () => { + const { container, rerender } = renderControls() + const fitIcon = () => screen.getByRole('button', { name: 'Fit to window' }).querySelector('svg') + expect(fitIcon()).toHaveClass('lucide-scan') + + rerender( + , + ) + expect(fitIcon()).toHaveClass('lucide-move-horizontal') + expect(container.querySelector('.lucide-maximize2, .lucide-maximize-2')).toBeNull() }) it('floats with a shadow by default and sits flat in a toolbar on request', () => { diff --git a/desktop/src/components/ui/ZoomControls.tsx b/desktop/src/components/ui/ZoomControls.tsx index 35838773..9d89d6c1 100644 --- a/desktop/src/components/ui/ZoomControls.tsx +++ b/desktop/src/components/ui/ZoomControls.tsx @@ -1,4 +1,4 @@ -import { Maximize2, Minus, Plus } from 'lucide-react' +import { Minus, MoveHorizontal, Plus, Scan } from 'lucide-react' import { cx } from '@/lib/cx' import { IconButton } from './IconButton' @@ -12,6 +12,12 @@ export type ZoomControlsLabels = { fit: string } +/** + * What "fit" means for a viewer. Picks the fit button's icon, which must not read as + * "maximize": the workspace panel's own maximize control sits in the same view. + */ +export type ZoomFitMode = 'window' | 'width' + /** * The look of a control floating over a viewer: a hairline-bordered pill. Shared * so an extra action placed beside the zoom cluster reads as part of it. @@ -38,6 +44,8 @@ export type ZoomControlsProps = { onZoomIn: () => void onZoomOut: () => void onFit: () => void + /** Defaults to `window`. */ + fitMode?: ZoomFitMode /** Caller-supplied so this primitive never carries user-visible text of its own. */ labels: ZoomControlsLabels /** @@ -57,6 +65,9 @@ export type ZoomControlsProps = { * ladder passing through 100%, by a double click on the content, and by the * keyboard, so it does not need a control of its own competing for room in a * narrow panel. + * + * Fit is an action, not a toggle: it is only available once the reader has zoomed + * away from the fitted scale, so a button that can be clicked always does something. */ export function ZoomControls({ percent, @@ -66,6 +77,7 @@ export function ZoomControls({ onZoomIn, onZoomOut, onFit, + fitMode = 'window', labels, surface = 'default', flat = false, @@ -98,12 +110,14 @@ export function ZoomControls({ onClick={onZoomIn} /> } + icon={fitMode === 'width' + ? + : } label={labels.fit} size="md" tone="secondary" surface={media ? 'media' : 'default'} - pressed={fitActive} + disabled={fitActive} onClick={onFit} /> diff --git a/desktop/src/components/ui/ZoomableImage.test.tsx b/desktop/src/components/ui/ZoomableImage.test.tsx index 156d6d4f..047e1f27 100644 --- a/desktop/src/components/ui/ZoomableImage.test.tsx +++ b/desktop/src/components/ui/ZoomableImage.test.tsx @@ -63,7 +63,7 @@ describe('ZoomableImage', () => { // 800×600 area, 16px padding each side → 768×568 available; 1600×1200 → min(0.48, 0.4733) expect(image.style.width).toBe(`${1600 * (568 / 1200)}px`) expect(screen.getByText('47%')).toBeInTheDocument() - expect(screen.getByRole('button', { name: 'Fit to window' })).toHaveAttribute('aria-pressed', 'true') + expect(screen.getByRole('button', { name: 'Fit to window' })).toBeDisabled() }) it('does not enlarge a small picture beyond 100% when fitting', () => { @@ -80,7 +80,7 @@ describe('ZoomableImage', () => { fireEvent.click(screen.getByRole('button', { name: 'Zoom in' })) expect(screen.getByText('50%')).toBeInTheDocument() expect(image.style.width).toBe('800px') - expect(screen.getByRole('button', { name: 'Fit to window' })).toHaveAttribute('aria-pressed', 'false') + expect(screen.getByRole('button', { name: 'Fit to window' })).toBeEnabled() fireEvent.click(screen.getByRole('button', { name: 'Zoom out' })) expect(screen.getByText('33%')).toBeInTheDocument() diff --git a/desktop/src/components/workspace/surfaces/ImagePreview.test.tsx b/desktop/src/components/workspace/surfaces/ImagePreview.test.tsx index db1ec247..a304ddd8 100644 --- a/desktop/src/components/workspace/surfaces/ImagePreview.test.tsx +++ b/desktop/src/components/workspace/surfaces/ImagePreview.test.tsx @@ -170,7 +170,7 @@ describe('ImagePreview', () => { loaded(screen.getByRole('img')) expect(screen.getByText('50%')).toBeInTheDocument() - expect(screen.getByRole('button', { name: 'Fit to window' })).toHaveAttribute('aria-pressed', 'false') + expect(screen.getByRole('button', { name: 'Fit to window' })).toBeEnabled() }) it('stores fit as the absence of a zoom, so the default keeps following the panel size', () => { @@ -181,7 +181,7 @@ describe('ImagePreview', () => { fireEvent.click(screen.getByRole('button', { name: 'Fit to window' })) expect(useWorkspaceContentStore.getState().fileViewByKey[workspaceFileKey('s1', 'assets/logo.png')]?.zoom).toBeUndefined() - expect(screen.getByRole('button', { name: 'Fit to window' })).toHaveAttribute('aria-pressed', 'true') + expect(screen.getByRole('button', { name: 'Fit to window' })).toBeDisabled() }) it('keeps the scroll position stored beside the zoom', () => { diff --git a/desktop/src/components/workspace/surfaces/document/DocumentToolbar.test.tsx b/desktop/src/components/workspace/surfaces/document/DocumentToolbar.test.tsx index dcf844b3..f89c9025 100644 --- a/desktop/src/components/workspace/surfaces/document/DocumentToolbar.test.tsx +++ b/desktop/src/components/workspace/surfaces/document/DocumentToolbar.test.tsx @@ -44,7 +44,9 @@ describe('DocumentToolbar', () => { fireEvent.click(screen.getByRole('button', { name: 'Zoom in' })) fireEvent.click(screen.getByRole('button', { name: 'Zoom out' })) // A page reads top to bottom: "fit" means the width, not the whole page. - fireEvent.click(screen.getByRole('button', { name: 'Fit to width' })) + const fit = screen.getByRole('button', { name: 'Fit to width' }) + expect(fit.querySelector('svg')).toHaveClass('lucide-move-horizontal') + fireEvent.click(fit) expect(zoom.onZoomIn).toHaveBeenCalledTimes(1) expect(zoom.onZoomOut).toHaveBeenCalledTimes(1) diff --git a/desktop/src/components/workspace/surfaces/document/DocumentToolbar.tsx b/desktop/src/components/workspace/surfaces/document/DocumentToolbar.tsx index 64aeb4d5..0f7dca5d 100644 --- a/desktop/src/components/workspace/surfaces/document/DocumentToolbar.tsx +++ b/desktop/src/components/workspace/surfaces/document/DocumentToolbar.tsx @@ -6,7 +6,7 @@ import { useTranslation } from '@/i18n' import { isRootedLocalPath } from '@/lib/handlePreviewLink' import { openLocalFileWithSystem, reportOpenFailure } from '@/lib/systemFileOpen' -export type DocumentZoomState = Omit +export type DocumentZoomState = Omit /** * The bar above a rendered document: whatever is specific to the kind of document @@ -40,6 +40,7 @@ export function DocumentToolbar({ { expect(screen.getByText('100%')).toBeInTheDocument() expect(zoomOf(frames()[0]!)).toBe('1') - expect(fitButton()).toHaveAttribute('aria-pressed', 'true') + expect(fitButton()).toBeDisabled() }) it('shrinks a page that does not fit a narrow panel to fit it', async () => { @@ -276,7 +276,7 @@ describe('DocxSurface', () => { expect(zoomOf(frames()[0]!)).toBe('1.5') expect(screen.getByText('150%')).toBeInTheDocument() - expect(fitButton()).toHaveAttribute('aria-pressed', 'false') + expect(fitButton()).toBeEnabled() }) it('makes the frame as wide as the pages need when they are wider than the panel, so that this area scrolls', async () => { @@ -301,11 +301,11 @@ describe('DocxSurface', () => { await shown({ onZoomChange }) fireEvent.click(zoomOut()) - expect(fitButton()).toHaveAttribute('aria-pressed', 'false') + expect(fitButton()).toBeEnabled() fireEvent.click(fitButton()) expect(onZoomChange).toHaveBeenLastCalledWith(undefined) - expect(fitButton()).toHaveAttribute('aria-pressed', 'true') + expect(fitButton()).toBeDisabled() }) it('cannot go past either end of the ladder', async () => { diff --git a/desktop/src/components/workspace/surfaces/document/PdfSurface.test.tsx b/desktop/src/components/workspace/surfaces/document/PdfSurface.test.tsx index affe7cbb..d263179e 100644 --- a/desktop/src/components/workspace/surfaces/document/PdfSurface.test.tsx +++ b/desktop/src/components/workspace/surfaces/document/PdfSurface.test.tsx @@ -288,14 +288,14 @@ describe('PdfSurface', () => { it('leaves fit mode when the reader chooses a zoom, and returns to it on request', async () => { const onZoomChange = vi.fn() await show({ onZoomChange }) - expect(fitButton()).toHaveAttribute('aria-pressed', 'true') + expect(fitButton()).toBeDisabled() fireEvent.click(zoomIn()) - expect(fitButton()).toHaveAttribute('aria-pressed', 'false') + expect(fitButton()).toBeEnabled() fireEvent.click(fitButton()) expect(onZoomChange).toHaveBeenLastCalledWith(undefined) - expect(fitButton()).toHaveAttribute('aria-pressed', 'true') + expect(fitButton()).toBeDisabled() expect(screen.getByText('82%')).toBeInTheDocument() }) @@ -304,7 +304,7 @@ describe('PdfSurface', () => { expect(parseFloat(placed(sheets()[0]!).style.width)).toBeCloseTo(LETTER.width * 1.5, 3) expect(screen.getByText('150%')).toBeInTheDocument() - expect(fitButton()).toHaveAttribute('aria-pressed', 'false') + expect(fitButton()).toBeEnabled() }) it('cannot go past either end of the ladder', async () => { @@ -564,7 +564,9 @@ describe('PdfSurface', () => { const top = boxesAt(FIT)[4]!.top + 200 scrollTo(top) - fireEvent.click(fitButton()) // already fitted: no layout change, so no anchor should be left behind + // The fit button is disabled while fitted, but the keyboard still asks for fit: + // no layout change, so no anchor should be left behind. + fireEvent.keyDown(scroller(), { key: '0' }) resizePanel(760, 800) const wider = (760 - 2 * PDF_PAGE_PADDING) / LETTER.width