Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions apps/escurel-explore/lib/crm/crm_providers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ final allInstancesProvider = FutureProvider<List<InstanceSummary>>((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<double>((ref) => 0.42);
final leftCollapsedProvider = StateProvider<bool>((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<bool?>((ref) => null);
final rightCollapsedProvider = StateProvider<bool>((ref) => false);

/// Whether the focused page is a skill (skills carry no events). Derived
Expand All @@ -38,12 +45,15 @@ final currentPageIsSkillProvider = Provider<bool>((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<bool>((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
Expand Down
11 changes: 10 additions & 1 deletion apps/escurel-explore/lib/crm/crm_workspace.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>(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);
Expand Down
32 changes: 32 additions & 0 deletions apps/escurel-explore/test/crm/crm_workspace_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
});
}
19 changes: 12 additions & 7 deletions apps/escurel-explore/test/crm/left_pane_minimize_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
}
Original file line number Diff line number Diff line change
@@ -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<bool>((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<bool?>((ref) => null);
final effectiveLeftCollapsedProvider = Provider<bool>((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.
Loading