diff --git a/.claude/skills/release-notes/SKILL.md b/.claude/skills/release-notes/SKILL.md index 755e9c21..c64f97c5 100644 --- a/.claude/skills/release-notes/SKILL.md +++ b/.claude/skills/release-notes/SKILL.md @@ -25,7 +25,7 @@ In the pull request's body, the section `## Release note` holds exactly one note Add the tools you use every day, like an editor, a script or a build, in **Settings › Tools**. They show up in the menu of files, changesets, branches, labels and shelves, and open on what you right-clicked. -![Settings › Tools](./tools.png) +![The Tools page in Settings](./tools.png) ``` - `### : ` on the first line. The kind is one of: @@ -35,8 +35,7 @@ menu of files, changesets, branches, labels and shelves, and open on what you ri Choose by what the user gets, not by the branch name: a `feature/` branch that makes an existing view better is **Improved**; a `perf/` branch users will feel is **Improved**. - The description under it: Markdown. Headings inside it are `####`, never `###` (that starts a note). -- A picture is the pull request's own screenshot: reference the same file (`![Settings › Tools](./tools.png)`) and - `gh pr create --attach ./tools.png` uploads it once for both. +- A picture or a GIF goes in only when it adds to what the note says, cropped to what it shows: see "Pictures and GIFs". A pull request with nothing for users writes `None:` and the reason, in one line: @@ -71,6 +70,54 @@ anything at all (faster, a different word on screen, a crash gone), it gets a no - **A picture, a table or a snippet only when words aren't enough**: a new view or dialog, a before and after, a format the user types (`cs:4821` in a comment). Never a picture of something a sentence says as well. +## Pictures and GIFs + +A picture earns its place only when it shows what the note delivers better than words: a new dialog, menu or panel, +a before and after, a layout the user has to see. Most fixes and many improvements need none. When in doubt, leave it +out: a note without a picture reads well; a picture that doesn't help makes the note harder to read. + +What's New shows a picture inside the note, as wide as the text (about 600 pixels) and at most 360 pixels tall, and +a click shows it larger. So a picture must read at that size: + +- **Crop it to what the note is about.** A feature in a popup: the popup. A part of a panel: that part. A menu: the + menu open, with the row it opened from. Never the whole window, unless the note is about the whole window. +- **Keep just enough around it to place it**: a strip of the panel or toolbar it belongs to, so the user knows where + to look in the app. Not so much that the feature becomes small. +- **Wider than tall**, or about square: a tall crop is shrunk to 360 pixels high and its text gets hard to read. +- **Real-looking content**: a sandbox (`scripts/sandboxes/`) with sensible names, the light theme, the pointer out + of the way, nothing personal or internal on screen. +- **Take it with `playwright-cli`** (CLAUDE.md "Seeing the app"): `playwright-cli screenshot <ref>` takes one element, + the popup or the panel, already cropped. For a zone that no element bounds, take the window and crop the file. On a + high-density screen (a Retina Mac) the file is twice the size on screen: that's expected and fine, What's New fits it + to the note and shows it larger on a click. +- **Name it**: the alt text says what the picture shows (`![The Tools page in Settings](./tools.png)`); it is what a + screen reader says and what shows if the picture can't load. +- **Reuse the pull request's screenshot** when it already is that crop: reference the same file in the note and + `gh pr create --attach ./tools.png` uploads it once for both. The pull request's own picture may show more for + reviewers; the note's is cropped for users. + +### GIFs + +When what the note delivers is a movement, a still can't show it: something that follows the pointer, a drag, an +animation, a flow of two or three steps, something that now stays in place. Then a short GIF says it at a glance. +GIFs are welcome there; elsewhere a still reads better, and costs the reader less. + +- **The same crop as a picture**, to the zone where it happens, with its context around it. +- **Short and looping**: three to eight seconds, one action, starting and ending where the user would; no waiting + at the start, the pointer moving as a person moves it. +- **Light**: about 12 frames a second, no wider than about 900 pixels, a few megabytes at most (GitHub takes up to + 10 MB, and What's New downloads it). +- **A GIF, never a video file**: What's New shows a GIF as a picture; a video in the note doesn't show there at all. +- **How**: record with `playwright-cli video-start clip.webm` … `video-stop` (CLAUDE.md "Seeing the app"), then make + the GIF with ffmpeg, cropping as you go (`crop=width:height:x:y`, in the recording's pixels): + + ```bash + ffmpeg -i clip.webm -vf "crop=900:560:180:120,fps=12,scale='min(900,iw)':-1:flags=lanczos,split[a][b];[a]palettegen[p];[b][p]paletteuse" -loop 0 clip.gif + ``` + + Reference it as a picture, `![Dragging a branch onto another to merge it](./merge-drag.gif)`, and attach it with + `gh pr create --attach ./merge-drag.gif`. + ## The words The note uses the app's words, the ones on screen and the ones users of UVCS already use: workspace, branch, diff --git a/docs/features/updates.md b/docs/features/updates.md index 2ce0287f..80bc57ae 100644 --- a/docs/features/updates.md +++ b/docs/features/updates.md @@ -145,11 +145,15 @@ address whose token expires five minutes after the feed is read, long before Wha attachment's lasting address, `https://github.com/user-attachments/assets/<id>` (`lastingPictureUrl`). What's New shows the pictures themselves (`MarkdownBlocks` with `images="inline"`, `InlinePicture`): as wide as the -note at most and never wider than they are, at their own proportions, framed by a subtle border, opening at full size -in the browser on a click. While one loads it holds a quiet place of a stable height; one that can't load falls back -to the chip comments show (`inlinePictureView`), so its description and link stay. Comments keep their chips -(`CommentImage`, fetched only on hover): anyone may link anything there. Showing the HTML as it comes -(`dangerouslySetInnerHTML`) was rejected: the renderer is untrusted and must never run markup from the network. +note and 360 pixels tall at most, never larger than they are, at their own proportions, framed by a subtle border. A +click on one shown smaller than it is (`pictureEnlarges`, measured again as the window resizes: `usePictureEnlarges`) +shows it larger over the dialog, as large as the window allows (`PictureViewer`): a click, Esc or its close button +returns, and "Open in Browser" shows it at full size there. How to take a picture that reads at that size, cropped to +what it shows: the `release-notes` skill, "Pictures and GIFs" (a GIF plays in place, as a picture). While one loads it +holds a quiet place of a stable height; one that can't load falls back to the chip comments show +(`inlinePictureView`), so its description and link stay. Comments keep their chips (`CommentImage`, fetched only on +hover): anyone may link anything there. Showing the HTML as it comes (`dangerouslySetInnerHTML`) was rejected: the +renderer is untrusted and must never run markup from the network. **Seeing it while developing**: an unpackaged app has no feed, so What's New never opens. `UVCS_PREVIEW_UPDATE=<file>` (`previewUpdateFile`, `previewUpdateFeed`; ignored by a packaged app, and when blank) makes it find the update a JSON diff --git a/src/renderer/src/components/InlinePicture.module.css b/src/renderer/src/components/InlinePicture.module.css index fce53632..35027f4c 100644 --- a/src/renderer/src/components/InlinePicture.module.css +++ b/src/renderer/src/components/InlinePicture.module.css @@ -1,6 +1,12 @@ -/* The picture on its own line, as wide as the text at most: `max-width` alone, so a small one is never blown up. */ +/* The picture on its own line, as wide as the text at most: `max-width` alone, so a small one is never blown up. A + button, so a click enlarges it (`PictureViewer`), and disabled when it already shows at its own size. */ .frame { display: block; + padding: 0; + background: none; + font: inherit; + color: inherit; + text-align: inherit; width: fit-content; max-width: 100%; margin: var(--space-1) 0; @@ -11,14 +17,22 @@ cursor: zoom-in; } +.frame:disabled { + cursor: default; +} + .frame:focus-visible { outline: none; box-shadow: var(--focus-ring-visible); } +/* No taller than a screenshot should stand in the text, so a tall one doesn't push the notes after it away: its + width follows, at its own proportions, and a click shows it whole. */ .picture { display: block; max-width: 100%; + max-height: 360px; + width: auto; height: auto; } diff --git a/src/renderer/src/components/InlinePicture.tsx b/src/renderer/src/components/InlinePicture.tsx index 2ff2ce87..ea5bc3a3 100644 --- a/src/renderer/src/components/InlinePicture.tsx +++ b/src/renderer/src/components/InlinePicture.tsx @@ -1,7 +1,8 @@ -import { useState, type MouseEvent } from 'react'; -import { api } from '../api/client'; +import { useRef, useState, type MouseEvent } from 'react'; import { CommentImage } from './comments/CommentImage'; -import { inlinePictureView, pictureLoadAfter, type PictureLoad } from './inlinePictureView'; +import { inlinePictureView, pictureLabel, pictureLoadAfter, type PictureLoad } from './inlinePictureView'; +import { openPictureViewer } from './PictureViewer'; +import { usePictureEnlarges } from './usePictureEnlarges'; import styles from './InlinePicture.module.css'; interface InlinePictureProps { @@ -12,24 +13,36 @@ interface InlinePictureProps { } /** - * A picture shown in the text (`MarkdownImages` `inline`): as wide as the text at most, never wider than it is, at its - * own proportions, without telling its host which page asked (`no-referrer`). A click opens it in the browser, at its - * full size. While it loads it holds a quiet place; if it can't load, it's the chip comments show (`inlinePictureView`). + * A picture shown in the text (`MarkdownImages` `inline`): as wide as the text at most and no taller than a + * screenshot should stand in it, never larger than it is, at its own proportions, without telling its host which page + * asked (`no-referrer`). A click shows it larger (`PictureViewer`) when it shows smaller than it is + * (`pictureEnlarges`). While it loads it holds a quiet place; if it can't load, it's the chip comments show + * (`inlinePictureView`). */ export function InlinePicture({ url, alt, source }: InlinePictureProps) { const [load, setLoad] = useState<PictureLoad>('loading'); + const pictureRef = useRef<HTMLImageElement>(null); + const enlarges = usePictureEnlarges(pictureRef, load === 'loaded'); const view = inlinePictureView(load); if (view === 'chip') return <CommentImage url={url} alt={alt} source={source} />; - const openInBrowser = (event: MouseEvent): void => { - event.preventDefault(); + const enlarge = (event: MouseEvent): void => { event.stopPropagation(); - void api.system.openExternal(url); + openPictureViewer(url, alt); }; return ( - <a href={url} className={styles.frame} data-loading={view === 'placeholder' || undefined} data-tip={alt || undefined} onClick={openInBrowser}> + <button + type="button" + className={styles.frame} + data-loading={view === 'placeholder' || undefined} + data-tip={alt || undefined} + aria-label={pictureLabel(alt, enlarges)} + disabled={!enlarges} + onClick={enlarge} + > <img + ref={pictureRef} className={styles.picture} src={url} alt={alt} @@ -37,6 +50,6 @@ export function InlinePicture({ url, alt, source }: InlinePictureProps) { onLoad={(event) => setLoad(pictureLoadAfter('load', event.currentTarget.naturalWidth))} onError={() => setLoad(pictureLoadAfter('error', 0))} /> - </a> + </button> ); } diff --git a/src/renderer/src/components/PictureViewer.module.css b/src/renderer/src/components/PictureViewer.module.css new file mode 100644 index 00000000..b5e19fb6 --- /dev/null +++ b/src/renderer/src/components/PictureViewer.module.css @@ -0,0 +1,46 @@ +/* Over the dialog the picture came from, dimming it as a dialog dims the window: the dialogs' own backdrop, a layer + above their content (`Dialog.module.css`, 800 and 801). */ +.overlay { + composes: overlay from '../ui/dialog/Dialog.module.css'; + z-index: 802; +} + +/* The whole window: a click anywhere outside the buttons returns to the dialog. */ +.content { + position: fixed; + inset: 0; + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + gap: var(--space-3); + padding: var(--space-6); + cursor: zoom-out; + animation: var(--keyframes-fade-in) var(--duration-normal) var(--ease-out); + z-index: 803; + outline: none; +} + +/* As large as the window allows, leaving room for the buttons, and never larger than the picture is. */ +.picture { + display: block; + min-height: 0; + max-width: 100%; + max-height: calc(100% - var(--space-8) - var(--space-3)); + object-fit: contain; + border: 1px solid var(--border-subtle); + border-radius: var(--radius-md); + background: var(--bg-surface); + box-shadow: var(--shadow-lg); +} + +.actions { + display: flex; + align-items: center; + gap: var(--space-2); + padding: var(--space-1); + border-radius: var(--radius-lg); + background: var(--bg-surface-raised); + box-shadow: var(--shadow-lg); + cursor: default; +} diff --git a/src/renderer/src/components/PictureViewer.tsx b/src/renderer/src/components/PictureViewer.tsx new file mode 100644 index 00000000..709c579c --- /dev/null +++ b/src/renderer/src/components/PictureViewer.tsx @@ -0,0 +1,54 @@ +import * as RadixDialog from '@radix-ui/react-dialog'; +import { ArrowUpRight, X } from 'lucide-react'; +import { useRef, type MouseEvent } from 'react'; +import { api } from '../api/client'; +import { MODAL_DIALOG } from '../lib/modalDialog'; +import { Button } from '../ui/Button'; +import { IconButton } from '../ui/IconButton'; +import { openDialog } from '../ui/dialog/dialogStore'; +import { pictureTitle } from './inlinePictureView'; +import styles from './PictureViewer.module.css'; + +/** + * Shows a picture larger, over whatever dialog it came from: as large as the window allows and never larger than it + * is. A click anywhere, Esc or the close button, which it focuses, returns; "Open in Browser" shows it at its full + * size there. + */ +export function openPictureViewer(url: string, alt: string): void { + openDialog((close) => <PictureViewer url={url} alt={alt} onClose={close} />); +} + +function PictureViewer({ url, alt, onClose }: { url: string; alt: string; onClose: () => void }) { + const keepOpen = (event: MouseEvent): void => event.stopPropagation(); + const closeRef = useRef<HTMLButtonElement>(null); + + return ( + <RadixDialog.Root open onOpenChange={(open) => !open && onClose()}> + <RadixDialog.Portal> + <RadixDialog.Overlay className={styles.overlay} /> + <RadixDialog.Content + className={styles.content} + aria-describedby={undefined} + onClick={onClose} + // Close takes focus, not "Open in Browser": Enter, which may have opened the picture, then brings the user back. + onOpenAutoFocus={(event) => { + event.preventDefault(); + closeRef.current?.focus(); + }} + {...MODAL_DIALOG} + > + <RadixDialog.Title className="visually-hidden">{pictureTitle(alt)}</RadixDialog.Title> + <img className={styles.picture} src={url} alt={alt} referrerPolicy="no-referrer" draggable={false} /> + <div className={styles.actions} onClick={keepOpen}> + <Button size="small" icon={<ArrowUpRight size={14} />} onClick={() => void api.system.openExternal(url)}> + Open in Browser + </Button> + <RadixDialog.Close asChild> + <IconButton ref={closeRef} icon={<X size={15} />} label="Close" size="small" /> + </RadixDialog.Close> + </div> + </RadixDialog.Content> + </RadixDialog.Portal> + </RadixDialog.Root> + ); +} diff --git a/src/renderer/src/components/inlinePictureView.test.ts b/src/renderer/src/components/inlinePictureView.test.ts index 5fb33fbb..33c057d4 100644 --- a/src/renderer/src/components/inlinePictureView.test.ts +++ b/src/renderer/src/components/inlinePictureView.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { inlinePictureView, pictureLoadAfter } from './inlinePictureView'; +import { inlinePictureView, pictureEnlarges, pictureLabel, pictureLoadAfter, pictureTitle } from './inlinePictureView'; describe('a picture shown in the text', () => { it('holds its place quietly while it loads, then shows itself', () => { @@ -17,3 +17,31 @@ describe('a picture shown in the text', () => { expect(pictureLoadAfter('error', 0)).toBe('failed'); }); }); + +describe('clicking a picture shown in the text', () => { + it('enlarges it when the text shows it smaller than it is', () => { + expect(pictureEnlarges(1280, 586)).toBe(true); + }); + + it('does nothing when it already shows at its own size, a pixel of rounding aside', () => { + expect(pictureEnlarges(420, 420)).toBe(false); + expect(pictureEnlarges(420.4, 420)).toBe(false); + }); +}); + +describe("a picture's name", () => { + it('offers to enlarge it, by its description, when a click does', () => { + expect(pictureLabel('The Tools page in Settings', true)).toBe('Enlarge The Tools page in Settings'); + expect(pictureLabel('', true)).toBe('Enlarge picture'); + }); + + it('is only its description when a click does nothing', () => { + expect(pictureLabel('The Tools page in Settings', false)).toBe('The Tools page in Settings'); + expect(pictureLabel('', false)).toBe('Picture'); + }); + + it('titles the larger view by its description', () => { + expect(pictureTitle('The Tools page in Settings')).toBe('The Tools page in Settings'); + expect(pictureTitle('')).toBe('Picture'); + }); +}); diff --git a/src/renderer/src/components/inlinePictureView.ts b/src/renderer/src/components/inlinePictureView.ts index 8dba958d..582faebd 100644 --- a/src/renderer/src/components/inlinePictureView.ts +++ b/src/renderer/src/components/inlinePictureView.ts @@ -27,3 +27,26 @@ export function inlinePictureView(load: PictureLoad): 'placeholder' | 'picture' export function pictureLoadAfter(event: 'load' | 'error', naturalWidth: number): PictureLoad { return event === 'load' && naturalWidth > 0 ? 'loaded' : 'failed'; } + +/** + * Whether a click enlarges a picture (`PictureViewer`): only when the text shows it smaller than it is, held to the + * text's width and a height that keeps a screenshot from filling the dialog (`InlinePicture.module.css`). A picture + * already at its own size has nothing more to show; a pixel is left for rounding. + */ +export function pictureEnlarges(naturalWidth: number, shownWidth: number): boolean { + return naturalWidth - shownWidth > 1; +} + +/** What a picture with no description is called. */ +const UNNAMED_PICTURE = 'Picture'; + +/** A picture's accessible name: an offer to enlarge it when a click does (`pictureEnlarges`), else what it shows. */ +export function pictureLabel(alt: string, enlarges: boolean): string { + if (!enlarges) return alt || UNNAMED_PICTURE; + return alt ? `Enlarge ${alt}` : `Enlarge ${UNNAMED_PICTURE.toLowerCase()}`; +} + +/** What the larger view of a picture (`PictureViewer`) is titled for screen readers: what it shows. */ +export function pictureTitle(alt: string): string { + return alt || UNNAMED_PICTURE; +} diff --git a/src/renderer/src/components/usePictureEnlarges.ts b/src/renderer/src/components/usePictureEnlarges.ts new file mode 100644 index 00000000..0f87653a --- /dev/null +++ b/src/renderer/src/components/usePictureEnlarges.ts @@ -0,0 +1,22 @@ +import { useEffect, useState, type RefObject } from 'react'; +import { pictureEnlarges } from './inlinePictureView'; + +/** + * Whether a click on a loaded picture enlarges it (`pictureEnlarges`), measured again whenever the width it is shown + * at changes: a narrower window shrinks a dialog, and a picture shown whole can become one shown smaller. + */ +export function usePictureEnlarges(picture: RefObject<HTMLImageElement | null>, loaded: boolean): boolean { + const [enlarges, setEnlarges] = useState(false); + + useEffect(() => { + const element = picture.current; + if (!loaded || !element) return; + const measure = (): void => setEnlarges(pictureEnlarges(element.naturalWidth, element.clientWidth)); + measure(); + const observer = new ResizeObserver(measure); + observer.observe(element); + return () => observer.disconnect(); + }, [picture, loaded]); + + return enlarges; +}