From 597b2f762278f8099896407ae2966fb2f4df60cd Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 10 Jun 2026 15:18:00 +0200 Subject: [PATCH] dismiss the lightbox on a tap outside the image (T-309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The image fills the 94% viewer with a transparent letterbox, so taps on the dimmed canvas beside the image hit the viewer and did nothing — only the 6% margin dismissed. Add onTapUp that dismisses when the tap lands outside the actual painted-image rect (computed from the RenderImage + applyBoxFit, transformed for any zoom/pan). Double-tap reset, scroll zoom, pan, Esc, and the close button are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 7 +++++ CHANGELOG.md | 5 +++ lib/widgets/src/clide_lightbox.dart | 37 +++++++++++++++++++++++ test/widgets/clide_lightbox_test.dart | 26 ++++++++++++++++ 5 files changed, 76 insertions(+) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index e9d503c5..47d57fb5 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3178,3 +3178,4 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB1S7613SYF0M9XQT5JNWM40', 'status', 'backlog', 'ready', NULL, '2026-06-10 12:56:38', '2026-06-10 12:56:38', '2026-06-10 12:56:38', NULL, '0c7b9f022143b733461ca7b1b715c435', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB37JZSFZKWPK9PDFYJY2YC0', 'status', 'backlog', 'ready', NULL, '2026-06-10 13:00:19', '2026-06-10 13:00:19', '2026-06-10 13:00:19', NULL, 'a394b4a8b72a7bc99c7fc1a1ac6a2f19', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB2TY91VHK7TPKPMZ11EG3TM', 'status', 'ready', 'done', NULL, '2026-06-10 13:00:29', '2026-06-10 13:00:29', '2026-06-10 13:00:29', NULL, '25246286c46b36a114817316658049c7', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB1S7613SYF0M9XQT5JNWM40', 'status', 'ready', 'done', NULL, '2026-06-10 13:15:15', '2026-06-10 13:15:15', '2026-06-10 13:15:15', NULL, '9cea01c55347a9c347bd195d322dd004', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 2ba888eb..51573686 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -2441,3 +2441,10 @@ Root cause: each status maps to a child with a fixed ValueKey (''running'' / ''s Repro: observed live during `make run` with two Claude panes bound (primary + secondary-1); status indicators flipping quickly trigger it. Fix direction: the ValueKey must be unique per indicator instance, not just per status, so two instances (or an in-flight transition) never share a key. Options: key by status combined with a stable per-widget id, or drop the const keys and let AnimatedSwitcher key on child type. Add a widget test that rapidly toggles status within the switch duration and pumps mid-transition to guard the regression.', 'done', 'high', NULL, NULL, NULL, '2026-06-10 12:04:51', '2026-06-10 13:00:29', NULL, '8e5bd637d25313c220c5565e54ee657f', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB1S7613SYF0M9XQT5JNWM40', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'Lightbox: click anywhere outside the image should dismiss', 'When the image lightbox is open, clicking outside the image should close it, matching Esc and the close (x) button. + +Today only the thin margin around the lightbox dismisses. The DialogRouter host (lib/kernel/src/dialog.dart:95-103) renders a backdrop that dismisses on tap, but wraps the dialog content in a tap-swallowing GestureDetector (onTap: () {}). ClideLightbox fills a 94%-of-screen SizedBox whose Positioned.fill InteractiveViewer (lib/widgets/src/clide_lightbox.dart:93-113) covers the entire box. So taps on the dark area beside a smaller-than-94% image hit the swallower, not the backdrop, and nothing happens. + +Fix: make taps that land outside the actual image bounds dismiss — e.g. add an onTap to the lightbox''s outer GestureDetector that calls onDismiss when the tap is not on the image (InteractiveViewer child), rather than relying on the host backdrop only reaching the 6% margin. Preserve double-tap-to-reset, scroll-to-zoom, and pan; a single tap on empty canvas closes, a tap/drag on the image does not. + +Acceptance: single click on the dimmed area surrounding the image closes the lightbox; clicking/dragging/zooming the image does not; Esc and the x still close.', 'done', 'low', NULL, NULL, NULL, '2026-06-10 09:37:31', '2026-06-10 13:15:15', NULL, '2c16b97db9098d73e122bfbaba2b74eb', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index da6fbec8..9263bd79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- **Clicking outside the image in the lightbox now closes it.** Previously only + the thin margin dismissed — a click on the dimmed canvas beside a letterboxed + image hit the viewer and did nothing. A single tap outside the painted image + now closes it (matching Esc / the × button); tapping, dragging, or zooming the + image still doesn't. (T-309) - **Run-status indicators no longer crash on rapid flips.** Switching status back and forth within the 200ms cross-fade (e.g. running → success → running across two bound Claude panes) tripped an AnimatedSwitcher duplicate-key diff --git a/lib/widgets/src/clide_lightbox.dart b/lib/widgets/src/clide_lightbox.dart index 8bf48788..3eb58781 100644 --- a/lib/widgets/src/clide_lightbox.dart +++ b/lib/widgets/src/clide_lightbox.dart @@ -17,6 +17,7 @@ import 'package:clide/widgets/src/clide_text.dart'; import 'package:clide/widgets/src/icons/phosphor.dart'; import 'package:clide/widgets/src/typography.dart'; import 'package:flutter/gestures.dart'; +import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; @@ -61,6 +62,41 @@ class _ClideLightboxState extends State { void _reset() => _tc.value = Matrix4.identity(); + /// A single tap that lands on the dimmed canvas around the image dismisses, + /// matching Esc / the close button (T-309). A tap on the actual image pixels + /// (not the transparent letterbox that fills the viewer) does not — nor does a + /// drag/zoom, which the [InteractiveViewer] consumes so no tap fires. + void _onTapUp(TapUpDetails d) { + final rect = _imageRect(); + if (rect != null && rect.contains(d.globalPosition)) return; + widget.onDismiss(); + } + + /// On-screen rect of the painted image (fit:contain, after any zoom/pan), or + /// null if there's no image to find (a non-image child → any tap dismisses). + Rect? _imageRect() { + final root = context.findRenderObject(); + if (root == null) return null; + RenderImage? image; + void find(RenderObject o) { + if (image != null) return; + if (o is RenderImage) { + image = o; + return; + } + o.visitChildren(find); + } + + root.visitChildren(find); + final ri = image; + final pixels = ri?.image; + if (ri == null || pixels == null) return null; + final natural = Size(pixels.width.toDouble(), pixels.height.toDouble()); + final painted = applyBoxFit(BoxFit.contain, natural, ri.size).destination; + final local = Alignment.center.inscribe(painted, Offset.zero & ri.size); + return MatrixUtils.transformRect(ri.getTransformTo(null), local); + } + void _onScroll(PointerSignalEvent e) { if (e is! PointerScrollEvent) return; final current = _tc.value.getMaxScaleOnAxis(); @@ -101,6 +137,7 @@ class _ClideLightboxState extends State { onPointerSignal: _onScroll, child: GestureDetector( onDoubleTap: _reset, + onTapUp: _onTapUp, child: InteractiveViewer( transformationController: _tc, minScale: widget.minScale, diff --git a/test/widgets/clide_lightbox_test.dart b/test/widgets/clide_lightbox_test.dart index 53057abb..f4450e59 100644 --- a/test/widgets/clide_lightbox_test.dart +++ b/test/widgets/clide_lightbox_test.dart @@ -60,6 +60,32 @@ void main() { expect(find.byType(InteractiveViewer), findsOneWidget); }); + testWidgets('a tap on the dimmed canvas dismisses; a tap on the image does not (T-309)', (tester) async { + final img = (await tester.runAsync(() => createTestImage(width: 100, height: 100)))!; // 1:1; real async + var dismissed = false; + await tester.pumpWidget(harness( + f, + MediaQuery( + data: const MediaQueryData(size: Size(800, 600)), + child: ClideLightbox(onDismiss: () => dismissed = true, child: RawImage(image: img, fit: BoxFit.contain)), + ), + )); + await tester.pumpAndSettle(); + + // The 94% box is ~752×564; a 1:1 image fits to 564×564 centred, leaving + // ~94px side margins. A tap in the left margin is dimmed canvas → dismiss. + final r = tester.getRect(find.byType(InteractiveViewer)); + await tester.tapAt(Offset(r.left + 10, r.center.dy)); + await tester.pump(const Duration(milliseconds: 350)); // past the double-tap delay + expect(dismissed, isTrue); + + // A tap on the image itself does not dismiss. + dismissed = false; + await tester.tapAt(r.center); + await tester.pump(const Duration(milliseconds: 350)); + expect(dismissed, isFalse); + }); + testWidgets('scroll wheel zooms in and out', (tester) async { await tester.pumpWidget(harness(f, box(() {}))); await tester.pumpAndSettle();