From 1ae8ad078451f268ebfd0f616808f31e483ee461 Mon Sep 17 00:00:00 2001 From: Joachim Rosskopf Date: Sun, 31 May 2026 06:56:13 +0200 Subject: [PATCH] fix(explore): make the whole collapsed rail expand on tap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A collapsed pane could not be re-opened by a real mouse: the expand chevron was a centered 16px IconButton, so only that tiny icon was tappable. Collapsing (a full-width 28px header bar) was easy; expanding (the small rail icon) usually missed — and when both panes collapsed, the right rail ballooned to a wide blank region that ignored clicks everywhere but the icon. Widget tests missed it because `tester.tap` hits the widget centre — exactly where the icon is. Make the entire collapsed rail an `InkWell(onTap: onToggle)` so a tap anywhere re-expands it. Tests (both no-mock, real fixture, pointer path): - collapse both panes, then both expand chevrons work; - a tap *off* the icon (top of the rail, via tapAt) still expands — verified red under the old icon-only target, green after. Adds a discovered-note on small-hit-target bugs that `tester.tap` hides. Co-Authored-By: Claude Opus 4.8 --- .../lib/crm/crm_workspace.dart | 24 +++++- .../test/crm/both_panes_collapse_test.dart | 83 +++++++++++++++++++ ...26-05-31-collapsed-rail-tiny-hit-target.md | 33 ++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 apps/escurel-explore/test/crm/both_panes_collapse_test.dart create mode 100644 docs/notes/discovered/2026-05-31-collapsed-rail-tiny-hit-target.md diff --git a/apps/escurel-explore/lib/crm/crm_workspace.dart b/apps/escurel-explore/lib/crm/crm_workspace.dart index 5b157fc..36f8c70 100644 --- a/apps/escurel-explore/lib/crm/crm_workspace.dart +++ b/apps/escurel-explore/lib/crm/crm_workspace.dart @@ -176,11 +176,33 @@ class _CollapsibleRegion extends StatelessWidget { ); if (collapsed) { + // The whole collapsed rail is the expand target. A centered 16px icon + // is far too small to reliably hit — and when *both* panes collapse + // the right rail balloons to a wide blank area — so an InkWell fills + // the region and re-expands on a tap anywhere within it. return Semantics( label: label, container: true, explicitChildNodes: true, - child: Center(child: toggle), + child: Semantics( + label: '$label-expand', + button: true, + onTap: onToggle, + excludeSemantics: true, + child: Tooltip( + message: 'Expand', + child: InkWell( + onTap: onToggle, + child: Center( + child: Icon( + edge == _Edge.right ? Icons.chevron_right : Icons.chevron_left, + size: 16, + color: kOnSurfaceVariant, + ), + ), + ), + ), + ), ); } return Semantics( diff --git a/apps/escurel-explore/test/crm/both_panes_collapse_test.dart b/apps/escurel-explore/test/crm/both_panes_collapse_test.dart new file mode 100644 index 0000000..cb2b628 --- /dev/null +++ b/apps/escurel-explore/test/crm/both_panes_collapse_test.dart @@ -0,0 +1,83 @@ +// Reproduction for the live report: on an instance, collapsing BOTH panes +// leaves the expand chevrons unresponsive. Drives the real chevrons via +// the pointer path over the real fixture. + +import 'package:escurel_explore/client/escurel_client.dart'; +import 'package:escurel_explore/client/fixture_escurel_client.dart'; +import 'package:escurel_explore/crm/crm_workspace.dart'; +import 'package:escurel_explore/state/providers.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +EscurelClient _corpus() => FixtureEscurelClient.fromSources( + skillFiles: const { + 'customer.md': '---\ntype: skill\nid: customer\ndescription: A buying org.\n---\n# customer\n', + }, + instanceFiles: const { + 'customer__acme.md': + '---\ntype: instance\nskill: customer\nid: acme\nname: Acme Ltd\n---\n# Acme Ltd\n', + }, + ); + +Future _pump(WidgetTester tester) async { + tester.view.physicalSize = const Size(1400, 900); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + await tester.pumpWidget( + ProviderScope( + overrides: [escurelClientProvider.overrideWithValue(_corpus())], + child: const MaterialApp(home: CrmWorkspace()), + ), + ); + await tester.pumpAndSettle(); +} + +void main() { + testWidgets('collapse both panes, then both expand chevrons work', (tester) async { + await _pump(tester); + + // Collapse the left (events) pane. + await tester.tap(find.bySemanticsLabel('region-events-collapse')); + await tester.pumpAndSettle(); + // Collapse the right (instance) pane. + await tester.tap(find.bySemanticsLabel('region-instance-collapse')); + await tester.pumpAndSettle(); + + // Both collapsed: two expand chevrons, no panes. + expect(find.bySemanticsLabel('region-events-expand'), findsOneWidget); + expect(find.bySemanticsLabel('region-instance-expand'), findsOneWidget); + expect(find.bySemanticsLabel('event-pane'), findsNothing); + expect(find.bySemanticsLabel('instance-pane'), findsNothing); + + // Expand the right pane. + await tester.tap(find.bySemanticsLabel('region-instance-expand')); + await tester.pumpAndSettle(); + expect(find.bySemanticsLabel('instance-pane'), findsOneWidget, + reason: 'right pane must re-expand'); + + // Expand the left pane. + await tester.tap(find.bySemanticsLabel('region-events-expand')); + await tester.pumpAndSettle(); + expect(find.bySemanticsLabel('event-pane'), findsOneWidget, + reason: 'left pane must re-expand'); + }); + + testWidgets('a tap anywhere in the collapsed rail re-expands it (not just the icon)', (tester) async { + await _pump(tester); + + await tester.tap(find.bySemanticsLabel('region-events-collapse')); + await tester.pumpAndSettle(); + expect(find.bySemanticsLabel('event-pane'), findsNothing); + + // Tap near the TOP of the collapsed rail — far from the vertically + // centered chevron icon. Before the full-rail hit target, only the + // ~16px icon responded, so a real click that missed it did nothing. + final rail = tester.getRect(find.bySemanticsLabel('region-events')); + await tester.tapAt(Offset(rail.center.dx, rail.top + 24)); + await tester.pumpAndSettle(); + expect(find.bySemanticsLabel('event-pane'), findsOneWidget, + reason: 'tapping the rail (off the icon) must still expand it'); + }); +} diff --git a/docs/notes/discovered/2026-05-31-collapsed-rail-tiny-hit-target.md b/docs/notes/discovered/2026-05-31-collapsed-rail-tiny-hit-target.md new file mode 100644 index 0000000..ba82060 --- /dev/null +++ b/docs/notes/discovered/2026-05-31-collapsed-rail-tiny-hit-target.md @@ -0,0 +1,33 @@ +# A collapsed pane's expand chevron was a too-small hit target + +**Symptom.** On the CRM workspace, collapsing a pane worked, but the +**expand** chevron in the resulting rail often did nothing for a real +mouse — "I can collapse, but afterwards expand is not possible." Worst +when *both* panes were collapsed (a near-blank screen with two tiny +chevrons that wouldn't respond). + +**Cause.** The collapsed rail rendered `Center(child: toggle)` where +`toggle` was a 16px `IconButton`. Only that ~16–28px icon was +tappable. The collapse button, by contrast, lives in a full-width 28px +header bar (easy to hit) — hence "collapse works, expand doesn't." When +both panes collapse, the *right* rail also balloons to most of the +width (`leftW = rail`, `rightW = w - rail - divW`), so the chevron sits +alone in a large blank region that ignores clicks everywhere except the +icon. + +**Why tests missed it.** `tester.tap(find.bySemanticsLabel(...))` taps +the widget's **center**, which is exactly where the icon is — so every +collapse/expand test passed while real off-icon clicks failed. The +regression test now uses `tester.tapAt(...)` at an offset *away* from the +centered icon (near the top of the rail) to exercise the real hit area. + +**Fix.** Make the whole collapsed rail the tap target: wrap the rail in +an `InkWell(onTap: onToggle)` filling the region, with the chevron +centered for affordance. A tap anywhere in the rail re-expands it. + +**How to recognise it.** A control that "doesn't work" for real users +but passes every widget test. Check whether the tappable area is a small +centered child while the test taps dead-center — `tester.tap` hits the +center regardless of size, so it cannot catch an undersized hit target. +Use `tapAt` with an offset, and prefer generous, full-region hit targets +for rails/toggles.