From dc732754bd32c1456fe41393da8b795ff7cd79d9 Mon Sep 17 00:00:00 2001 From: TouwfiQ Meghlaoui Date: Thu, 22 Oct 2026 02:21:04 +0100 Subject: [PATCH 1/4] feat: preview draft images in lightbox before posting (#875) --- .../js/components/commit-image-picker.tsx | 47 +++- tests/Unit/DraftImagePreview.test.tsx | 241 ++++++++++++++++++ tests/draft-images.config.ts | 1 + 3 files changed, 284 insertions(+), 5 deletions(-) create mode 100644 tests/Unit/DraftImagePreview.test.tsx diff --git a/resources/js/components/commit-image-picker.tsx b/resources/js/components/commit-image-picker.tsx index 10ddb1b7a..06a8345ef 100644 --- a/resources/js/components/commit-image-picker.tsx +++ b/resources/js/components/commit-image-picker.tsx @@ -1,6 +1,8 @@ import { ImagePlus, X } from 'lucide-react'; import { useId, useRef, useState } from 'react'; import { ActionTooltip } from '@/components/action-tooltip'; +import type { ShownImage } from '@/components/commit-images'; +import { ImageViewer } from '@/components/image-viewer'; import { Button } from '@/components/ui/button'; import { acceptedImageTypes, @@ -164,6 +166,7 @@ export function ImagePickerPreviews({ }) { const id = useId(); const [describedIndex, setDescribedIndex] = useState(null); + const [viewedIndex, setViewedIndex] = useState(null); const urls = images.map(objectUrlFor); if (images.length === 0) { @@ -174,6 +177,10 @@ export function ImagePickerPreviews({ describedIndex !== null && describedIndex < images.length ? describedIndex : null; + const activeViewedIndex = + viewedIndex !== null && viewedIndex < images.length + ? viewedIndex + : null; const altButtonId = (index: number) => `${id}-alt-${index}`; const finishDescribing = () => { @@ -184,6 +191,12 @@ export function ImagePickerPreviews({ setDescribedIndex(null); }; + const shownImages: ShownImage[] = images.map((image, index) => ({ + id: image.name ? image.name.replace(/\.[^/.]+$/, '') : `${index + 1}`, + url: urls[index], + description: descriptions[index]?.trim() || null, + })); + return (
    @@ -192,15 +205,31 @@ export function ImagePickerPreviews({ return (
  • - {`Attached +
+ setViewedIndex(null)} + downloadUrl={(image) => image.url} + /> + {editing !== null && (
+import assert from 'node:assert/strict'; +import * as React from 'react'; +import type { ReactElement, ReactNode } from 'react'; +import { beforeEach, test, vi } from 'vite-plus/test'; +import type { ShownImage } from '../../resources/js/components/commit-images'; + +let state: unknown[] = []; +let cursor = 0; + +vi.doMock('react', () => ({ + ...React, + useId: () => 'preview-test-id', + useRef: (initial: unknown) => ({ current: initial }), + useState: (initial: unknown) => { + const index = cursor++; + if (!(index in state)) { + state[index] = typeof initial === 'function' ? (initial as () => unknown)() : initial; + } + + return [ + state[index], + (next: unknown) => { + state[index] = typeof next === 'function' ? (next as (current: unknown) => unknown)(state[index]) : next; + }, + ]; + }, +})); + +const { ImagePickerPreviews } = await import('../../resources/js/components/commit-image-picker'); +const { ImageViewer } = await import('../../resources/js/components/image-viewer'); + +function elements(node: ReactNode): ReactElement>[] { + if (!React.isValidElement(node)) { + return []; + } + + const element = node as ReactElement>; + + return [ + element, + ...React.Children.toArray(element.props.children as ReactNode).flatMap(elements), + ]; +} + +beforeEach(() => { + state = []; + cursor = 0; +}); + +test('renders preview buttons for draft images and opens them in the image viewer when clicked', () => { + const file1 = new File(['image-1'], 'photo1.png', { type: 'image/png' }); + const file2 = new File(['image-2'], 'photo2.jpg', { type: 'image/jpeg' }); + const images = [file1, file2]; + const descriptions = ['First caption', '']; + const removed: number[] = []; + + // Initial render: no image is being previewed in lightbox + cursor = 0; + let tree = elements( + ImagePickerPreviews({ + images, + descriptions, + onDescribe: () => {}, + onRemove: (index) => removed.push(index), + }), + ); + + const previewButtons = tree.filter((el) => el.props['data-test'] === 'preview-image-button'); + assert.equal(previewButtons.length, 2); + assert.equal(previewButtons[0].props['aria-label'], 'Preview attached image 1'); + assert.equal(previewButtons[1].props['aria-label'], 'Preview attached image 2'); + + let viewer = tree.find((el) => el.type === ImageViewer); + assert.ok(viewer); + assert.equal(viewer.props.index, null); + const shownImages = viewer.props.images as ShownImage[]; + assert.equal(shownImages.length, 2); + assert.equal(shownImages[0].id, 'photo1'); + assert.equal(shownImages[0].description, 'First caption'); + assert.equal(shownImages[1].id, 'photo2'); + assert.equal(shownImages[1].description, null); + + // Click first image to preview it + (previewButtons[0].props.onClick as () => void)(); + + // Re-render after state change + cursor = 0; + tree = elements( + ImagePickerPreviews({ + images, + descriptions, + onDescribe: () => {}, + onRemove: (index) => removed.push(index), + }), + ); + + viewer = tree.find((el) => el.type === ImageViewer); + assert.ok(viewer); + assert.equal(viewer.props.index, 0); + + // Navigate to second image via ImageViewer + (viewer.props.onView as (index: number) => void)(1); + + cursor = 0; + tree = elements( + ImagePickerPreviews({ + images, + descriptions, + onDescribe: () => {}, + onRemove: (index) => removed.push(index), + }), + ); + + viewer = tree.find((el) => el.type === ImageViewer); + assert.ok(viewer); + assert.equal(viewer.props.index, 1); + + // Close the viewer + (viewer.props.onClose as () => void)(); + + cursor = 0; + tree = elements( + ImagePickerPreviews({ + images, + descriptions, + onDescribe: () => {}, + onRemove: (index) => removed.push(index), + }), + ); + + viewer = tree.find((el) => el.type === ImageViewer); + assert.ok(viewer); + assert.equal(viewer.props.index, null); +}); + +test('removing the currently previewed image closes the viewer', () => { + const file1 = new File(['image-1'], 'first.png', { type: 'image/png' }); + const file2 = new File(['image-2'], 'second.png', { type: 'image/png' }); + const images = [file1, file2]; + let removedIndex: number | null = null; + + cursor = 0; + let tree = elements( + ImagePickerPreviews({ + images, + descriptions: ['', ''], + onDescribe: () => {}, + onRemove: (index) => { + removedIndex = index; + }, + }), + ); + + // Preview the first image + const previewButtons = tree.filter((el) => el.props['data-test'] === 'preview-image-button'); + (previewButtons[0].props.onClick as () => void)(); + + // Verify it is open at index 0 + cursor = 0; + tree = elements( + ImagePickerPreviews({ + images, + descriptions: ['', ''], + onDescribe: () => {}, + onRemove: (index) => { + removedIndex = index; + }, + }), + ); + let viewer = tree.find((el) => el.type === ImageViewer); + assert.equal(viewer?.props.index, 0); + + // Click remove button on the first image + const removeButtons = tree.filter((el) => typeof el.props['aria-label'] === 'string' && el.props['aria-label'].startsWith('Remove image')); + (removeButtons[0].props.onClick as () => void)(); + assert.equal(removedIndex, 0); + + // Re-render: viewer should be closed (index null) + cursor = 0; + tree = elements( + ImagePickerPreviews({ + images: [file2], + descriptions: [''], + onDescribe: () => {}, + onRemove: () => {}, + }), + ); + viewer = tree.find((el) => el.type === ImageViewer); + assert.equal(viewer?.props.index, null); +}); + +test('removing an image preceding the currently previewed image shifts the index down', () => { + const file1 = new File(['1'], '1.png', { type: 'image/png' }); + const file2 = new File(['2'], '2.png', { type: 'image/png' }); + const file3 = new File(['3'], '3.png', { type: 'image/png' }); + const images = [file1, file2, file3]; + + cursor = 0; + let tree = elements( + ImagePickerPreviews({ + images, + descriptions: ['', '', ''], + onDescribe: () => {}, + onRemove: () => {}, + }), + ); + + // Preview image 3 (index 2) + const previewButtons = tree.filter((el) => el.props['data-test'] === 'preview-image-button'); + (previewButtons[2].props.onClick as () => void)(); + + cursor = 0; + tree = elements( + ImagePickerPreviews({ + images, + descriptions: ['', '', ''], + onDescribe: () => {}, + onRemove: () => {}, + }), + ); + let viewer = tree.find((el) => el.type === ImageViewer); + assert.equal(viewer?.props.index, 2); + + // Remove image 1 (index 0) + const removeButtons = tree.filter((el) => typeof el.props['aria-label'] === 'string' && el.props['aria-label'].startsWith('Remove image')); + (removeButtons[0].props.onClick as () => void)(); + + // Re-render: viewer index should now be shifted to 1 + cursor = 0; + tree = elements( + ImagePickerPreviews({ + images: [file2, file3], + descriptions: ['', ''], + onDescribe: () => {}, + onRemove: () => {}, + }), + ); + viewer = tree.find((el) => el.type === ImageViewer); + assert.equal(viewer?.props.index, 1); +}); diff --git a/tests/draft-images.config.ts b/tests/draft-images.config.ts index 2ba4230e1..e3e3bd666 100644 --- a/tests/draft-images.config.ts +++ b/tests/draft-images.config.ts @@ -20,6 +20,7 @@ export default defineConfig({ 'tests/Unit/GuestFollow.test.tsx', 'tests/Unit/DraftStash.test.tsx', 'tests/Unit/DraftImageReads.test.tsx', + 'tests/Unit/DraftImagePreview.test.tsx', 'tests/Unit/ImageAttachments.test.tsx', 'tests/Unit/ComposerDraft.test.tsx', 'tests/Unit/QuoteDraft.test.tsx', From a0e6df5cf960d74e9743f19aa5b591a5bfa17613 Mon Sep 17 00:00:00 2001 From: TouwfiQ Meghlaoui Date: Thu, 22 Oct 2026 23:51:30 +0100 Subject: [PATCH 2/4] fix: coordinate escape handling and preserve download format for draft previews --- .../js/components/commit-image-picker.tsx | 57 ++++- resources/js/components/image-viewer.tsx | 34 ++- tests/Unit/DraftImagePreview.test.tsx | 241 ++++++++++++++++-- 3 files changed, 296 insertions(+), 36 deletions(-) diff --git a/resources/js/components/commit-image-picker.tsx b/resources/js/components/commit-image-picker.tsx index 06a8345ef..5aa704598 100644 --- a/resources/js/components/commit-image-picker.tsx +++ b/resources/js/components/commit-image-picker.tsx @@ -1,5 +1,5 @@ import { ImagePlus, X } from 'lucide-react'; -import { useId, useRef, useState } from 'react'; +import { useCallback, useEffect, useId, useRef, useState } from 'react'; import { ActionTooltip } from '@/components/action-tooltip'; import type { ShownImage } from '@/components/commit-images'; import { ImageViewer } from '@/components/image-viewer'; @@ -182,15 +182,64 @@ export function ImagePickerPreviews({ ? viewedIndex : null; const altButtonId = (index: number) => `${id}-alt-${index}`; + const previewButtonId = (index: number) => `${id}-preview-${index}`; + + const closeViewer = useCallback(() => { + if (activeViewedIndex !== null && typeof document !== 'undefined') { + document + .getElementById(`${id}-preview-${activeViewedIndex}`) + ?.focus(); + } + + setViewedIndex(null); + }, [activeViewedIndex, id]); + + useEffect(() => { + if (activeViewedIndex === null || typeof window === 'undefined') { + return; + } + + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + event.preventDefault(); + event.stopPropagation(); + closeViewer(); + } + }; + + window.addEventListener('keydown', onKeyDown, { capture: true }); + + return () => + window.removeEventListener('keydown', onKeyDown, { capture: true }); + }, [activeViewedIndex, closeViewer]); const finishDescribing = () => { - if (editing !== null) { + if (editing !== null && typeof document !== 'undefined') { document.getElementById(altButtonId(editing))?.focus(); } setDescribedIndex(null); }; + const downloadFilename = (_image: ShownImage, index: number): string => { + const file = images[index]; + if (!file) { + return `image-${index + 1}.png`; + } + + if (file.name && file.name.includes('.')) { + return file.name; + } + + const subtype = file.type + ? file.type.replace(/^image\//, '').toLowerCase() + : ''; + const ext = subtype === 'jpeg' ? 'jpg' : subtype || 'png'; + const baseName = file.name || `image-${index + 1}`; + + return `${baseName}.${ext}`; + }; + const shownImages: ShownImage[] = images.map((image, index) => ({ id: image.name ? image.name.replace(/\.[^/.]+$/, '') : `${index + 1}`, url: urls[index], @@ -206,6 +255,7 @@ export function ImagePickerPreviews({ return (