fix(desktop): make zoom fit a distinct, action-only control

The fit button used the same Maximize2 icon as the workspace panel's
maximize control and stayed pressed while already fitted, where clicking
it did nothing. Give it its own icon (fit width: MoveHorizontal, fit
window: Scan) and disable it while the viewer is fitted, so it is only
clickable when there is a manual zoom to undo.
This commit is contained in:
程序员阿江(Relakkes)
2026-10-02 00:23:17 +08:00
parent 7c7a977e15
commit 98fab14a8d
9 changed files with 62 additions and 25 deletions
@@ -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(<ImageGalleryModal open images={gallery} activeIndex={0} onClose={() => {}} 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(<ImageGalleryModal open images={gallery} activeIndex={1} onClose={() => {}} onSelect={() => {}} />)
expect(screen.getByRole('button', { name: 'Fit to window' })).toHaveAttribute('aria-pressed', 'true')
expect(screen.getByRole('button', { name: 'Fit to window' })).toBeDisabled()
})
})
@@ -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(
<ZoomControls percent={100} fitActive={false} canZoomIn canZoomOut labels={LABELS} onZoomIn={vi.fn()} onZoomOut={vi.fn()} onFit={vi.fn()} />,
<ZoomControls percent={100} fitActive={false} canZoomIn canZoomOut labels={LABELS} onZoomIn={vi.fn()} onZoomOut={vi.fn()} onFit={onFit} />,
)
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(
<ZoomControls percent={100} fitActive={false} fitMode="width" canZoomIn canZoomOut labels={LABELS} onZoomIn={vi.fn()} onZoomOut={vi.fn()} onFit={vi.fn()} />,
)
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', () => {
+17 -3
View File
@@ -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}
/>
<IconButton
icon={<Maximize2 size={15} strokeWidth={1.9} />}
icon={fitMode === 'width'
? <MoveHorizontal size={16} strokeWidth={1.9} />
: <Scan size={15} strokeWidth={1.9} />}
label={labels.fit}
size="md"
tone="secondary"
surface={media ? 'media' : 'default'}
pressed={fitActive}
disabled={fitActive}
onClick={onFit}
/>
</div>
@@ -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()
@@ -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', () => {
@@ -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)
@@ -6,7 +6,7 @@ import { useTranslation } from '@/i18n'
import { isRootedLocalPath } from '@/lib/handlePreviewLink'
import { openLocalFileWithSystem, reportOpenFailure } from '@/lib/systemFileOpen'
export type DocumentZoomState = Omit<ZoomControlsProps, 'labels' | 'surface' | 'flat' | 'className'>
export type DocumentZoomState = Omit<ZoomControlsProps, 'labels' | 'surface' | 'flat' | 'fitMode' | 'className'>
/**
* The bar above a rendered document: whatever is specific to the kind of document
@@ -40,6 +40,7 @@ export function DocumentToolbar({
<ZoomControls
{...zoom}
flat
fitMode="width"
labels={{
group: t('workspace.zoom.group'),
zoomIn: t('workspace.zoom.in'),
@@ -249,7 +249,7 @@ describe('DocxSurface', () => {
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 () => {
@@ -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