From a8a9e64402b555a5aaef28e05c824f2e2169078a Mon Sep 17 00:00:00 2001 From: Joachim Rosskopf Date: Sun, 31 May 2026 06:24:44 +0200 Subject: [PATCH] fix(explore): keep the event-pane chevron live on skill pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once collapsed, the left event pane could not be re-opened on a skill page — the expand chevron was dead. The auto-minimize was modelled as `effective = leftCollapsed || isSkill`, and the chevron wrote `leftCollapsed`; on a skill `isSkill` is true, so the OR stayed true no matter what the chevron wrote (`false || true == true`). Make `leftCollapsedProvider` an explicit choice (`bool?`, null = follow the page-type default) that `effective` prefers over the skill default, and reset it to null in `_SplitBody` when the focused page flips skill↔instance. The chevron is now always authoritative for the current page (it can re-open the event pane on a skill), while skills still auto-minimize by default and instances still show events. Test: crm_workspace_test `the event-pane chevron re-opens an auto-minimized skill page` drives the real chevron on a skill page — verified red under the old OR-logic, green after the fix. Updates left_pane_minimize_test for the explicit-choice semantics; adds a discovered-note. Co-Authored-By: Claude Opus 4.8 --- .../lib/crm/crm_providers.dart | 22 +++++--- .../lib/crm/crm_workspace.dart | 11 +++- .../test/crm/crm_workspace_test.dart | 32 ++++++++++++ .../test/crm/left_pane_minimize_test.dart | 19 ++++--- ...-05-31-dead-event-pane-chevron-on-skill.md | 50 +++++++++++++++++++ 5 files changed, 120 insertions(+), 14 deletions(-) create mode 100644 docs/notes/discovered/2026-05-31-dead-event-pane-chevron-on-skill.md diff --git a/apps/escurel-explore/lib/crm/crm_providers.dart b/apps/escurel-explore/lib/crm/crm_providers.dart index dcf862b..9a13184 100644 --- a/apps/escurel-explore/lib/crm/crm_providers.dart +++ b/apps/escurel-explore/lib/crm/crm_providers.dart @@ -28,7 +28,14 @@ final allInstancesProvider = FutureProvider>((ref) async { /// Layout state for the resizable/collapsible two-pane split: the left /// pane's width fraction, and per-pane collapse flags. final leftPaneFractionProvider = StateProvider((ref) => 0.42); -final leftCollapsedProvider = StateProvider((ref) => false); + +/// The user's *explicit* event-pane collapse choice. `null` means "follow +/// the page-type default" (skills auto-minimize, instances show events); a +/// concrete bool is a chevron toggle that overrides that default. The +/// override is reset to `null` when the focused page flips skill↔instance +/// (see `_SplitBody`), so each context re-derives its default while the +/// chevron stays live — including re-opening the pane on a skill page. +final leftCollapsedProvider = StateProvider((ref) => null); final rightCollapsedProvider = StateProvider((ref) => false); /// Whether the focused page is a skill (skills carry no events). Derived @@ -38,12 +45,15 @@ final currentPageIsSkillProvider = Provider((ref) { return page?.pageType == PageType.skill; }); -/// The left event pane's *effective* collapsed state: auto-minimized when -/// a skill is focused (no events to show), otherwise the user's manual -/// chevron toggle ([leftCollapsedProvider]). The chevron keeps writing -/// the manual flag, so the user's choice is restored on instances. +/// The left event pane's *effective* collapsed state: the user's explicit +/// chevron choice ([leftCollapsedProvider]) when set, otherwise the +/// page-type default — skills carry no events, so they auto-minimize. The +/// override (not an OR over the skill flag) is what lets the chevron +/// re-open the pane on a skill page; `_SplitBody` clears it on a +/// skill↔instance transition so each context falls back to its default. final effectiveLeftCollapsedProvider = Provider((ref) { - return ref.watch(leftCollapsedProvider) || ref.watch(currentPageIsSkillProvider); + final choice = ref.watch(leftCollapsedProvider); + return choice ?? ref.watch(currentPageIsSkillProvider); }); /// The event currently open in the left detail pane. Distinct from the diff --git a/apps/escurel-explore/lib/crm/crm_workspace.dart b/apps/escurel-explore/lib/crm/crm_workspace.dart index da02e66..5b157fc 100644 --- a/apps/escurel-explore/lib/crm/crm_workspace.dart +++ b/apps/escurel-explore/lib/crm/crm_workspace.dart @@ -71,8 +71,17 @@ class _SplitBody extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { + // Clear the explicit collapse choice when the focused page flips + // skill↔instance, so each context re-derives its default (skills + // minimize, instances show events) while the chevron stays live. + ref.listen(currentPageIsSkillProvider, (prev, next) { + if (prev != next) { + ref.read(leftCollapsedProvider.notifier).state = null; + } + }); + // Effective collapse: auto-minimized while a skill is focused (no - // events), else the user's manual toggle. + // events), else the user's explicit chevron choice. final leftCollapsed = ref.watch(effectiveLeftCollapsedProvider); final rightCollapsed = ref.watch(rightCollapsedProvider); final fraction = ref.watch(leftPaneFractionProvider); diff --git a/apps/escurel-explore/test/crm/crm_workspace_test.dart b/apps/escurel-explore/test/crm/crm_workspace_test.dart index 09d19b3..7c7b160 100644 --- a/apps/escurel-explore/test/crm/crm_workspace_test.dart +++ b/apps/escurel-explore/test/crm/crm_workspace_test.dart @@ -5,13 +5,25 @@ import 'package:escurel_explore/client/escurel_client.dart'; import 'package:escurel_explore/client/fixture_escurel_client.dart'; +import 'package:escurel_explore/client/models.dart'; import 'package:escurel_explore/crm/crm_providers.dart'; import 'package:escurel_explore/crm/crm_workspace.dart'; +import 'package:escurel_explore/md/frontmatter.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'; +ExpandResult _skillPage() => const ExpandResult( + pageId: 'markdown/skills/customer.md', + skill: 'customer', + pageType: PageType.skill, + frontmatter: {'type': 'skill', 'id': 'customer'}, + body: '# customer\n', + blocks: [], + wikilinksOut: [], + ); + EscurelClient _corpus() => FixtureEscurelClient.fromSources( skillFiles: const { 'customer.md': '---\ntype: skill\nid: customer\ndescription: A buying org.\n---\n# customer\n', @@ -72,4 +84,24 @@ void main() { // The instance view still renders. expect(find.bySemanticsLabel('instance-pane'), findsOneWidget); }); + + testWidgets('the event-pane chevron re-opens an auto-minimized skill page', (tester) async { + // A skill page auto-minimizes the (event-less) left pane. Regression: + // the chevron used to be dead there — `effective = collapsed || isSkill` + // meant tapping expand could never beat the skill flag, so the pane + // could not be opened again. Now the explicit choice wins. + await _pump(tester, overrides: [ + currentPageProvider.overrideWith((ref) async => _skillPage()), + ]); + + // Auto-minimized: only the expand toggle shows. + expect(find.bySemanticsLabel('region-events-expand'), findsOneWidget); + expect(find.bySemanticsLabel('event-pane'), findsNothing); + + // Tapping expand must re-open it (collapse toggle + event pane back). + await tester.tap(find.bySemanticsLabel('region-events-expand')); + await tester.pumpAndSettle(); + expect(find.bySemanticsLabel('region-events-collapse'), findsOneWidget); + expect(find.bySemanticsLabel('event-pane'), findsOneWidget); + }); } diff --git a/apps/escurel-explore/test/crm/left_pane_minimize_test.dart b/apps/escurel-explore/test/crm/left_pane_minimize_test.dart index 5fa70b4..f837b5b 100644 --- a/apps/escurel-explore/test/crm/left_pane_minimize_test.dart +++ b/apps/escurel-explore/test/crm/left_pane_minimize_test.dart @@ -19,26 +19,31 @@ ExpandResult _page(PageType type) => ExpandResult( ); void main() { - test('effective collapse is forced on a skill, respects the toggle on an instance', () async { - // Skill focused → left auto-minimized even though the user toggle is off. + test('skill auto-minimizes by default, but an explicit choice always wins', () async { + // Skill focused, no explicit choice → auto-minimized. final skillC = ProviderContainer( overrides: [currentPageProvider.overrideWith((ref) async => _page(PageType.skill))], ); addTearDown(skillC.dispose); await skillC.read(currentPageProvider.future); expect(skillC.read(currentPageIsSkillProvider), isTrue); - expect(skillC.read(leftCollapsedProvider), isFalse, reason: 'manual toggle untouched'); - expect(skillC.read(effectiveLeftCollapsedProvider), isTrue, reason: 'skill forces minimize'); + expect(skillC.read(leftCollapsedProvider), isNull, reason: 'no explicit choice yet'); + expect(skillC.read(effectiveLeftCollapsedProvider), isTrue, reason: 'skill default = minimized'); - // Instance focused → effective follows the manual toggle. + // The chevron writes an explicit choice — which must re-open the pane + // even on a skill page (regression: the OR-with-skill made it dead). + skillC.read(leftCollapsedProvider.notifier).state = false; + expect(skillC.read(effectiveLeftCollapsedProvider), isFalse, reason: 'explicit expand wins on a skill'); + + // Instance focused, no explicit choice → events shown. final instC = ProviderContainer( overrides: [currentPageProvider.overrideWith((ref) async => _page(PageType.instance))], ); addTearDown(instC.dispose); await instC.read(currentPageProvider.future); expect(instC.read(currentPageIsSkillProvider), isFalse); - expect(instC.read(effectiveLeftCollapsedProvider), isFalse, reason: 'instance, toggle off'); + expect(instC.read(effectiveLeftCollapsedProvider), isFalse, reason: 'instance default = shown'); instC.read(leftCollapsedProvider.notifier).state = true; - expect(instC.read(effectiveLeftCollapsedProvider), isTrue, reason: 'instance, toggle on'); + expect(instC.read(effectiveLeftCollapsedProvider), isTrue, reason: 'instance, explicit collapse'); }); } diff --git a/docs/notes/discovered/2026-05-31-dead-event-pane-chevron-on-skill.md b/docs/notes/discovered/2026-05-31-dead-event-pane-chevron-on-skill.md new file mode 100644 index 0000000..80ce0dc --- /dev/null +++ b/docs/notes/discovered/2026-05-31-dead-event-pane-chevron-on-skill.md @@ -0,0 +1,50 @@ +# A derived OR made the event-pane chevron dead on skill pages + +**Symptom.** In the escurel-explore CRM workspace, once the left event +pane was collapsed it could not be re-opened — on a **skill** page the +expand chevron did nothing at all. (On instance pages collapse/expand +worked fine, which is why it looked intermittent.) + +**Cause.** The event pane auto-minimizes on skill pages (skills carry no +events). That was modelled as a derived OR: + +```dart +final effectiveLeftCollapsedProvider = Provider((ref) => + ref.watch(leftCollapsedProvider) || ref.watch(currentPageIsSkillProvider)); +``` + +The chevron wrote `leftCollapsedProvider`. On a skill page +`currentPageIsSkill` is `true`, so the OR is stuck `true` no matter what +the chevron writes — the toggle is **dead**. Clicking expand set +`leftCollapsedProvider = false`, but `false || true` is still `true`. + +**Fix.** Make `leftCollapsedProvider` an *explicit choice* (`bool?`, +`null` = follow the page-type default) and have `effective` prefer it +over the skill default: + +```dart +final leftCollapsedProvider = StateProvider((ref) => null); +final effectiveLeftCollapsedProvider = Provider((ref) { + final choice = ref.watch(leftCollapsedProvider); + return choice ?? ref.watch(currentPageIsSkillProvider); +}); +``` + +`_SplitBody` resets the choice to `null` when the focused page flips +skill↔instance (`ref.listen(currentPageIsSkillProvider, …)`), so each +context falls back to its sensible default (skills minimize, instances +show events) while the chevron is always authoritative for the current +page. + +**How to recognise it.** A user-toggleable boolean that is OR-ed (or +AND-ed) with a derived condition in its *effective* value. The derived +term silently wins, so the control appears inert in exactly the state +where the derived term is active. If a toggle "does nothing" only in a +particular context, look for a derived value masking the user's flag — +prefer "explicit choice overrides default" (nullable override) over +"flag OR condition." + +**Test.** `crm_workspace_test.dart` → +`the event-pane chevron re-opens an auto-minimized skill page` drives the +real chevron on a skill page and asserts it expands. Verified red under +the old OR-logic, green after the fix.