From eafee1b5008d5265a3778ea5c2d740c9163d73de Mon Sep 17 00:00:00 2001 From: Illia Obukhau <8282906+iobuhov@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:56:07 +0200 Subject: [PATCH] fix(widget-plugin-grid): clear selection wins over keep, re-arms on next reconciliation Co-Authored-By: Claude Opus 4.8 --- .../datagrid-web/CHANGELOG.md | 2 + .../pluggableWidgets/gallery-web/CHANGELOG.md | 2 + .../.openspec.yaml | 2 + .../design.md | 84 ++++++++ .../proposal.md | 61 ++++++ .../tasks.md | 67 ++++++ .../openspec/schemas/tdd-refactor/schema.yaml | 106 ++++++++++ .../schemas/tdd-refactor/templates/design.md | 28 +++ .../tdd-refactor/templates/proposal.md | 15 ++ .../schemas/tdd-refactor/templates/tasks.md | 33 +++ .../selection/__tests__/keepSelection.spec.ts | 193 ++++++++++++++++++ .../src/selection/createSelectionHelper.ts | 9 +- .../src/selection/helpers.ts | 100 ++++++++- .../builders/SelectionMultiValueBuilder.ts | 13 +- .../builders/SelectionSingleValueBuilder.ts | 12 ++ 15 files changed, 716 insertions(+), 11 deletions(-) create mode 100644 packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/.openspec.yaml create mode 100644 packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/design.md create mode 100644 packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/proposal.md create mode 100644 packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/tasks.md create mode 100644 packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/schema.yaml create mode 100644 packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/design.md create mode 100644 packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/proposal.md create mode 100644 packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/tasks.md create mode 100644 packages/shared/widget-plugin-grid/src/selection/__tests__/keepSelection.spec.ts diff --git a/packages/pluggableWidgets/datagrid-web/CHANGELOG.md b/packages/pluggableWidgets/datagrid-web/CHANGELOG.md index 886dd05c12..7ed624005e 100644 --- a/packages/pluggableWidgets/datagrid-web/CHANGELOG.md +++ b/packages/pluggableWidgets/datagrid-web/CHANGELOG.md @@ -14,6 +14,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - We fixed an issue where export type and format properties were visible in Studio Pro for dynamic text columns, even though they have no effect. +- We fixed an issue where the "Clear Selection" JavaScript action did not clear the selection when the "Keep selection" property was enabled. The selection now clears, and keep selection resumes on subsequent paging or refresh. + ## [3.11.1] - 2026-06-18 ### Fixed diff --git a/packages/pluggableWidgets/gallery-web/CHANGELOG.md b/packages/pluggableWidgets/gallery-web/CHANGELOG.md index 7acb316617..be9acb9f53 100644 --- a/packages/pluggableWidgets/gallery-web/CHANGELOG.md +++ b/packages/pluggableWidgets/gallery-web/CHANGELOG.md @@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - We fixed an issue where "Reset All Filters" and "Clear Selection" JavaScript actions did not work with the Gallery widget. Event listeners were not registered despite the required infrastructure being in place. +- We fixed an issue where the "Clear Selection" JavaScript action did not clear the selection when the "Keep selection" property was enabled. The selection now clears, and keep selection resumes on subsequent paging or refresh. + ## [3.11.0] - 2026-05-27 ### Added diff --git a/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/.openspec.yaml b/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/.openspec.yaml new file mode 100644 index 0000000000..611d90afcd --- /dev/null +++ b/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/.openspec.yaml @@ -0,0 +1,2 @@ +schema: tdd-refactor +created: 2026-07-21 diff --git a/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/design.md b/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/design.md new file mode 100644 index 0000000000..6545318acb --- /dev/null +++ b/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/design.md @@ -0,0 +1,84 @@ +## Test Cases + +Tests live in `packages/shared/widget-plugin-grid/src/selection/__tests__/` (Jest). The keep +predicate is exercised via the updated `SelectionMultiValueBuilder`, which must capture the +function passed to `setKeepSelection` and expose it (e.g. `getKeepSelectionPredicate()`). + +### Reproduction Tests + +- clear-beats-keep (multi) - Clear empties selection even when keep is enabled (unit) + - **Given**: A `MultiSelectionHelper` created with `keepSelection: true`, `setup()` run, and + two items selected. + - **When**: `clearSelection()` is called. + - **Then**: `selection.setSelection([])` was called AND, at the moment of the + reconciliation, the captured keep predicate returns `false` (i.e. the runtime would not + restore). Selection ends empty. + +- clear-beats-keep (single) - Clear/remove empties single selection when keep is enabled (unit) + - **Given**: A `SingleSelectionHelper` created with `keepSelection: true`, `setup()` run, one + item selected. + - **When**: the clear path (`remove()`) is called. + - **Then**: `setSelection(undefined)` called and the keep predicate returns `false` during + the clear cycle. + +### Edge Cases + +- keep-resumes-after-clear - Keep selection re-arms after a clear completes (unit) + - **Given**: keep enabled, items selected, `clearSelection()` called. + - **When**: a datasource reconciliation delivers a new selection ref (the `when` guard fires + on ref change). + - **Then**: the keep predicate returns `true` again, so a subsequent datasource update would + retain selection. + +- rearm-on-reselect - Keep re-arms even if the user re-selects before the clear lands (unit) + - **Given**: keep enabled, items selected, `clearSelection()` called (keep now yields). + - **When**: the next reconciliation delivers a _non-empty_ new selection (the user picked a + different item before the empty selection ever arrived). + - **Then**: the keep predicate returns `true` — re-arm keys off the reconciliation (ref + change), not off emptiness, so the new selection is retained on later paging/refresh. + +- double-clear-reentrancy - Two clears before the first reconciliation lands do not leave keep off (unit) + - **Given**: keep enabled, `clearSelection()` called twice in succession. + - **When**: the next reconciliation delivers a new selection ref. + - **Then**: the earlier pending `when` is disposed, exactly one re-arm occurs, and the + predicate returns `true` at the end. + +- clear-without-keep - Clear behaves as today when keep is disabled (unit) + - **Given**: helper created with `keepSelection: false`. + - **When**: `clearSelection()` is called. + - **Then**: `setSelection([])` called, no keep predicate installed, no `when` watcher created. + +- dispose-mid-clear - Pending re-arm watcher is cleaned up on teardown (unit) + - **Given**: keep enabled, `clearSelection()` called, next reconciliation not yet observed. + - **When**: the disposer returned by `setup()` runs. + - **Then**: the pending `when` is disposed; no reaction leaks (no re-arm fires afterwards). + +### Regression Tests + +- existing selection ops unchanged - `selectAll`, `selectNone`, `add`, `remove`, range select, + `selectionStatus` keep current behavior (unit) — existing `helpers.spec.ts` cases remain green. + - **Given**: existing selection helper specs. + - **When**: run after the change. + - **Then**: all pass unchanged. + +- predicate installed once - keep predicate is installed via `setup()`, not the constructor (unit) + - **Given**: helper created with `keepSelection: true` but `setup()` not yet called. + - **When**: constructor completes. + - **Then**: `setKeepSelection` has not been called; calling `setup()` installs it. + +## Notes + +- Correctness hinges on the transient flag staying `false` long enough to cover the runtime's + restore. The re-arm fires on the next reconciliation (a new `selection` ref), which is + necessarily _after_ the runtime has already made its keep decision for that reconciliation — so + re-arm can never let keep win over the clear it is paired with. This is a Mendix runtime timing + detail not fully verifiable from unit tests; a manual/E2E verification in the real widget is + planned (drive a JS clear-selection action with keep enabled) before merge. +- Re-arm compares the observed `selection` **reference** to the one captured at clear time rather + than inspecting emptiness. Reason: the user may re-select a different item before the empty + selection ever reconciles; keying off "empty" would strand keep in the `false` state and drop + that new selection on the next page/refresh. A ref change is the reconciliation signal itself, + independent of the resulting selection contents. +- The keep flag (`keepActive` observable) and the re-arm `when` live directly on each helper; no + separate class. `beforeClear()` snapshots the current ref, drops the flag, and installs a fresh + `when` (disposing any prior pending one, covering re-entrant/double clears). diff --git a/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/proposal.md b/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/proposal.md new file mode 100644 index 0000000000..cf8c4580d9 --- /dev/null +++ b/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/proposal.md @@ -0,0 +1,61 @@ +## Why + +The "Clear selection" JS action (used by both Data Grid 2 and Gallery) does not clear the +selection when the widget's `Keep selection` property is enabled. Keep selection always wins. + +- **Observed**: With `Keep selection` on, invoking the `Clear_Selection` JS action empties the + selection momentarily, but the previous selection is restored on the next datasource + reconciliation. The user sees the selection stay put. +- **Expected**: An explicit clear action empties the selection, and keep selection continues to + apply on subsequent paging/refresh. This matches the `Keep selection` property description: + "selected items will stay selected unless cleared by the user or a Nanoflow." + +## Root Cause + +`createSelectionHelper.ts` installs an unconditional keep-selection predicate at helper creation: + +```ts +if (config.keepSelection) { + selection?.setKeepSelection(() => true); +} +``` + +`setKeepSelection` registers a predicate the Mendix runtime evaluates lazily during each +datasource reconciliation to decide whether to retain the prior selection. Because it returns +`true` unconditionally, the clear action's `setSelection([])` (in `helpers.ts`) is undone on the +next reconciliation — the runtime re-applies the kept selection. There is no path for the clear +action to make the predicate yield. + +No existing test caught this: the test util (`SelectionMultiValueBuilder`) stubs +`setKeepSelection` as a no-op, so the keep-vs-clear interaction is never exercised. + +## What Changes + +Scope: `packages/shared/widget-plugin-grid` only. Both Data Grid 2 and Gallery inherit the fix +through the shared `createSelectionHelper` — no per-widget code changes. + +- Replace the constant predicate with one backed by observable state owned by the selection + helper: a stable `keepSelection` config flag plus a transient "retain now" observable the + predicate reads live (`setKeepSelection(() => active.get())`). +- `clearSelection()` (Multi) and `remove()`/clear path (Single) drop the transient flag to + `false` around `setSelection([])`, then re-arm it on the next datasource reconciliation — a + `when` guard that compares the observed `selection` **reference** against the one snapshotted at + clear time (the runtime schedules new props, so a fresh ref means the reconciliation ran and the + runtime's keep decision is already past). This re-arms whether the reconciliation delivers an + empty selection (clear landed) or a new one (the user re-selected before the clear landed). +- The keep flag and re-arm `when` live directly on each selection helper (`MultiSelectionHelper`, + `SingleSelectionHelper`); no separate armer class. Install the predicate and manage the `when` + disposer through the standard `setup()` host lifecycle instead of eagerly in + `createSelectionHelper`. +- Make the test util capture the predicate so the interaction is testable. + +## Impact + +- **Widgets affected**: Data Grid 2 (`datagrid-web`), Gallery (`gallery-web`) — behavior only, + no source changes in those packages. +- **API**: No public API change. `createSelectionHelper` signature unchanged (config still + carries `keepSelection`). +- **Behavior**: Bug fix — clear action now wins over keep selection, then keep resumes. Not + breaking; restores the documented contract. +- **Tests**: `widget-plugin-test-utils` `SelectionMultiValueBuilder` gains a real + `setKeepSelection` capture (was a no-op stub). diff --git a/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/tasks.md b/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/tasks.md new file mode 100644 index 0000000000..fec2198a6a --- /dev/null +++ b/packages/shared/widget-plugin-grid/openspec/changes/fix-keep-clear-selection-conflict/tasks.md @@ -0,0 +1,67 @@ +## 1. Test Setup + + + +- [x] 1.1 Update `SelectionMultiValueBuilder` (and single builder if present) in + `widget-plugin-test-utils` to capture the `setKeepSelection` predicate and expose it +- [x] 1.2 Write failing test: clear-beats-keep for multi selection (predicate returns `false` + during the clear cycle, selection ends empty) +- [x] 1.3 Write failing test: clear-beats-keep for single selection (`remove()` path) +- [x] 1.4 Add edge-case tests: keep-resumes-after-clear, rearm-on-reselect, + double-clear-reentrancy, clear-without-keep, dispose-mid-clear +- [x] 1.5 Add regression test: predicate installed via `setup()`, not constructor + +## 2. Implementation + + + +- [x] 2.1 Add `keepSelection` (stable config) + `keepActive` (observable box) state directly to + `MultiSelectionHelper`; pass `keepSelection` in via constructor +- [x] 2.2 Implement `setup()` on the helper: install `setKeepSelection(() => keepActive.get())` + when configured; return disposer that cancels any pending re-arm `when` +- [x] 2.3 Update `clearSelection()` to snapshot the current `selection` ref, drop `keepActive`, + `setSelection([])`, then re-arm via `when(() => selectionValue !== clearedRef, () => +keepActive.set(true))` guarded by the config flag +- [x] 2.4 Apply the same flag + re-arm logic to `SingleSelectionHelper` clear path (`remove()`) +- [x] 2.5 Update `createSelectionHelper.ts`: pass `config.keepSelection` to constructors, remove + the unconditional `setKeepSelection(() => true)`, and `host.add(helper)` +- [x] 2.6 Delete `KeepSelectionArmer.ts` and remove `onReconcile` calls from `updateProps` +- [x] 2.7 Verify no regressions in existing selection specs + +## 3. Refactoring + + + +- [x] 3.1 Clean up implementation; ensure re-entrancy disposer handling is clear (dispose prior + pending `when` on each `beforeClear`) +- [x] 3.2 Keep the inlined keep/clear logic minimal and consistent across Multi and Single + +## 4. Verification + +- [x] 4.1 All tests passing (including new tests) — `pnpm run test` in `widget-plugin-grid` +- [x] 4.2 Full test suite passes for `datagrid-web` and `gallery-web` (no regressions) — + selection-touching suites (row-interaction, item-keyboard) all green; the remaining failing + suites fail identically on clean `main` due to a pre-existing dual-mendix-version type + conflict (10.24 vs 11.10) in test helpers, unrelated to this change +- [ ] 4.3 Manual/E2E verify: drive `Clear_Selection` JS action with `Keep selection` enabled in a + real widget; confirm selection clears and keep resumes on next page change +- [x] 4.4 Add per-widget CHANGELOG entries for `datagrid-web` and `gallery-web` +- [x] 4.5 Code review ready (clean, documented) + +## Notes + + + +- Runtime timing (does the re-arm fire before/after the runtime's keep restore?) is + the main risk; task 4.3 is the gate that confirms the unit-level approach holds in the real + runtime. +- Final design: the keep logic is inlined into each helper (no separate class). `beforeClear()` + snapshots the current `selection` ref, drops the `keepActive` flag, and installs a + `when(() => selectionValue !== clearedRef, () => keepActive.set(true))`. Rationale: the runtime + does not mutate the selection in place on `setSelection([])`; it schedules new props, so the + next reconciliation delivers a fresh `selectionValue` ref. Comparing the ref (rather than + inspecting emptiness) re-arms on the reconciliation signal itself — correct whether the delivered + selection is empty (clear landed) or a new selection (the user re-selected before the empty + landed). Emptiness-based re-arm would strand keep off in the re-select case. The `when` + self-disposes after firing; `beforeClear()` disposes any prior pending `when` (double/re-entrant + clears), and `setup()`'s disposer cancels a pending `when` on teardown. diff --git a/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/schema.yaml b/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/schema.yaml new file mode 100644 index 0000000000..36c850f9f8 --- /dev/null +++ b/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/schema.yaml @@ -0,0 +1,106 @@ +name: tdd-refactor +version: 1 +description: Test-driven development for refactoring and fixes - proposal → design → tasks +artifacts: + - id: proposal + generates: proposal.md + description: Problem statement and change overview + template: proposal.md + instruction: | + Create a focused proposal for the refactoring or fix. + + For bugs: + - **Why**: What's broken? Observed vs. expected behavior + - **Root Cause**: Technical reason for the bug (if known) + - **What Changes**: What will be fixed? Which files/components? + - **Impact**: Who/what is affected? Is this breaking? + + For refactoring: + - **Why**: What problem does current code have? Tech debt, maintainability, performance? + - **What Changes**: What will be restructured? Be specific about scope + - **Impact**: Affected code, APIs, or behavior changes (should be minimal for pure refactoring) + + Keep it concise (1 page max). This is about fixing or improving existing code, + not adding new features. If you're adding features, use spec-driven instead. + requires: [] + + - id: design + generates: design.md + description: Test plan defining what needs to pass + template: design.md + instruction: | + Define the test cases that will verify the fix or refactoring. + + For bugs: + - Reproduction test (currently failing) + - Edge cases related to the bug + - Regression tests for related functionality + + For refactoring: + - Existing behavior preservation tests + - Tests for improved cases (if applicable) + - Performance/maintainability verification (if relevant) + + Format: + ``` + ## Test Cases + + ### Category Name + + - Test name - Description (unit/integration/e2e) + - **Given**: Setup/preconditions + - **When**: Action/trigger + - **Then**: Expected outcome + ``` + + Write tests that will FAIL initially (for bugs) or verify existing behavior (for refactoring). + Tests define the contract - implementation must make them pass without breaking anything. + requires: + - proposal + + - id: tasks + generates: tasks.md + description: Implementation task breakdown + template: tasks.md + instruction: | + Break down the implementation into trackable TDD tasks. + + **IMPORTANT: Follow the template below exactly.** Use checkbox format: `- [ ] X.Y Description` + + Group tasks by phase: + + ## 1. Test Setup + - [ ] 1.1 Write failing test for main issue + - [ ] 1.2 Add edge case tests + + ## 2. Implementation + - [ ] 2.1 Fix core issue (make main test pass) + - [ ] 2.2 Handle edge cases (make edge tests pass) + + ## 3. Refactoring + - [ ] 3.1 Clean up implementation while keeping tests green + - [ ] 3.2 Extract common logic/improve structure + + ## 4. Verification + - [ ] 4.1 All tests passing + - [ ] 4.2 No regressions (run full test suite) + + Order tasks by TDD cycle: Red (failing test) → Green (make it pass) → Refactor (clean up). + Each task should be completable in one session. + requires: + - design + +apply: + requires: [design, tasks] + tracks: tasks.md + instruction: | + Follow TDD workflow: + 1. Review proposal.md - understand what's broken or needs refactoring + 2. Review design.md - understand what tests need to pass + 3. Work through tasks.md in order: + - Write failing tests first (Red) + - Implement minimal fix to pass tests (Green) + - Refactor while keeping tests green + 4. Mark task checkboxes as you complete them + + All tests must pass before marking complete. Pause if blocked or need clarification. diff --git a/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/design.md b/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/design.md new file mode 100644 index 0000000000..1c14dda47f --- /dev/null +++ b/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/design.md @@ -0,0 +1,28 @@ +## Test Cases + + + +### Reproduction Tests + +- Test name - Description (unit/integration/e2e) + - **Given**: Setup/preconditions + - **When**: Action/trigger + - **Then**: Expected outcome + +### Edge Cases + +- Test name - Description (unit/integration/e2e) + - **Given**: Setup/preconditions + - **When**: Action/trigger + - **Then**: Expected outcome + +### Regression Tests + +- Test name - Description (unit/integration/e2e) + - **Given**: Setup/preconditions + - **When**: Action/trigger + - **Then**: Expected outcome + +## Notes + + diff --git a/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/proposal.md b/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/proposal.md new file mode 100644 index 0000000000..57679f7fd5 --- /dev/null +++ b/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/proposal.md @@ -0,0 +1,15 @@ +## Why + + + +## Root Cause + + + +## What Changes + + + +## Impact + + diff --git a/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/tasks.md b/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/tasks.md new file mode 100644 index 0000000000..735b974d88 --- /dev/null +++ b/packages/shared/widget-plugin-grid/openspec/schemas/tdd-refactor/templates/tasks.md @@ -0,0 +1,33 @@ +## 1. Test Setup + + + +- [ ] 1.1 Write failing test for main issue +- [ ] 1.2 Add edge case tests +- [ ] 1.3 Add regression tests for related functionality + +## 2. Implementation + + + +- [ ] 2.1 Fix core issue (make main test pass) +- [ ] 2.2 Handle edge cases (make edge tests pass) +- [ ] 2.3 Verify no regressions + +## 3. Refactoring + + + +- [ ] 3.1 Clean up implementation +- [ ] 3.2 Extract common logic/improve structure +- [ ] 3.3 Remove duplication + +## 4. Verification + +- [ ] 4.1 All tests passing (including new tests) +- [ ] 4.2 Full test suite passes (no regressions) +- [ ] 4.3 Code review ready (clean, documented) + +## Notes + + diff --git a/packages/shared/widget-plugin-grid/src/selection/__tests__/keepSelection.spec.ts b/packages/shared/widget-plugin-grid/src/selection/__tests__/keepSelection.spec.ts new file mode 100644 index 0000000000..d8961c2fd3 --- /dev/null +++ b/packages/shared/widget-plugin-grid/src/selection/__tests__/keepSelection.spec.ts @@ -0,0 +1,193 @@ +import "../../utils/mobx-test-setup"; +import { ObjectItem, SelectionMultiValue, SelectionSingleValue } from "mendix"; +import { runInAction } from "mobx"; +import { objectItems, SelectionMultiValueBuilder } from "@mendix/widget-plugin-test-utils"; +import { MultiSelectionHelper, SingleSelectionHelper } from "../helpers"; + +type KeepSelectionPredicate = (item: ObjectItem) => boolean; + +describe("keep selection vs clear selection", () => { + describe("MultiSelectionHelper", () => { + let builder: SelectionMultiValueBuilder; + let selectionValue: SelectionMultiValue; + let items: ObjectItem[]; + let helper: MultiSelectionHelper; + let dispose: (() => void) | void; + + const predicate = (): KeepSelectionPredicate | undefined => builder.getKeepSelectionPredicate(); + + // Simulates a datasource reconciliation: the runtime delivers a *new* + // SelectionMultiValue ref (in-place `setSelection` does not change the + // ref the helper observes). `updateProps` is how the helper sees it. + function reconcile(next: ObjectItem[]): void { + const nextValue = { + type: "Multi", + selection: next, + setSelection: selectionValue.setSelection, + setKeepSelection: selectionValue.setKeepSelection + } as unknown as SelectionMultiValue; + selectionValue = nextValue; + runInAction(() => helper.updateProps(nextValue, items)); + } + + beforeEach(() => { + items = objectItems(4); + builder = new SelectionMultiValueBuilder(); + selectionValue = builder.build(); + helper = new MultiSelectionHelper(selectionValue, items, true); + dispose = helper.setup(); + }); + + afterEach(() => { + dispose?.(); + }); + + it("clear-beats-keep: clear empties selection and the keep predicate yields during the clear cycle", () => { + reconcile([items[0], items[1]]); + expect(selectionValue.selection).toHaveLength(2); + // Keep is armed before clearing. + expect(predicate()!(items[0])).toBe(true); + + helper.clearSelection(); + + // setSelection([]) was issued and, at the moment of the reconciliation, + // the keep predicate returns false so the runtime would not restore. + expect(selectionValue.selection).toEqual([]); + expect(predicate()!(items[0])).toBe(false); + }); + + it("keep-resumes-after-clear: keep re-arms once the empty selection is observed", () => { + reconcile([items[0], items[1]]); + helper.clearSelection(); + expect(predicate()!(items[0])).toBe(false); + + // The runtime delivers the empty selection (the clear landed). + reconcile([]); + + expect(predicate()!(items[0])).toBe(true); + }); + + it("rearm-on-reselect: keep re-arms even if a new selection lands before the empty does", () => { + reconcile([items[0], items[1]]); + helper.clearSelection(); + expect(predicate()!(items[0])).toBe(false); + + // The user re-selects a different item before the empty reconciliation + // ever arrives; the next reconciliation delivers a non-empty selection. + reconcile([items[2]]); + + expect(predicate()!(items[0])).toBe(true); + }); + + it("double-clear-reentrancy: two clears before the empty lands re-arm exactly once", () => { + reconcile([items[0], items[1]]); + + helper.clearSelection(); + helper.clearSelection(); + expect(predicate()!(items[0])).toBe(false); + + reconcile([]); + expect(predicate()!(items[0])).toBe(true); + }); + + it("clear-without-keep: no predicate installed when keep is disabled", () => { + const b2 = new SelectionMultiValueBuilder(); + const sv2 = b2.build(); + const h2 = new MultiSelectionHelper(sv2, items, false); + const d2 = h2.setup(); + + expect(b2.getKeepSelectionPredicate()).toBeUndefined(); + + sv2.setSelection([items[0]]); + h2.clearSelection(); + expect(sv2.selection).toEqual([]); + + d2?.(); + }); + + it("dispose-mid-clear: pending re-arm watcher is cleaned up on teardown", () => { + reconcile([items[0]]); + helper.clearSelection(); + expect(predicate()!(items[0])).toBe(false); + + // Teardown before the empty selection is observed. + dispose?.(); + dispose = undefined; + + // The empty reconciliation fires now, but the watcher is disposed, + // so keep must stay off. + reconcile([]); + expect(predicate()!(items[0])).toBe(false); + }); + + it("predicate installed once: keep predicate is installed via setup(), not the constructor", () => { + const b3 = new SelectionMultiValueBuilder(); + const sv3 = b3.build(); + const h3 = new MultiSelectionHelper(sv3, items, true); + + expect(b3.getKeepSelectionPredicate()).toBeUndefined(); + + const d3 = h3.setup(); + expect(b3.getKeepSelectionPredicate()).toBeDefined(); + d3?.(); + }); + }); + + describe("SingleSelectionHelper", () => { + let selectionValue: SelectionSingleValue; + let capturedPredicate: KeepSelectionPredicate | undefined; + let items: ObjectItem[]; + let helper: SingleSelectionHelper; + let dispose: (() => void) | void; + + function makeValue(selection: ObjectItem | undefined): SelectionSingleValue { + const value = { + type: "Single", + selection, + setSelection(next: ObjectItem | undefined) { + value.selection = next; + }, + setKeepSelection(next: KeepSelectionPredicate | undefined) { + capturedPredicate = next; + } + }; + return value as unknown as SelectionSingleValue; + } + + function reconcile(next: ObjectItem | undefined): void { + selectionValue = makeValue(next); + runInAction(() => helper.updateProps(selectionValue)); + } + + beforeEach(() => { + items = objectItems(4); + capturedPredicate = undefined; + selectionValue = makeValue(undefined); + helper = new SingleSelectionHelper(selectionValue, true); + dispose = helper.setup(); + }); + + afterEach(() => { + dispose?.(); + }); + + it("clear-beats-keep: remove empties single selection and the keep predicate yields", () => { + reconcile(items[0]); + expect(capturedPredicate!(items[0])).toBe(true); + + helper.remove(); + + expect(selectionValue.selection).toBeUndefined(); + expect(capturedPredicate!(items[0])).toBe(false); + }); + + it("keep-resumes-after-clear: keep re-arms once the empty selection is observed", () => { + reconcile(items[0]); + helper.remove(); + expect(capturedPredicate!(items[0])).toBe(false); + + reconcile(undefined); + expect(capturedPredicate!(items[0])).toBe(true); + }); + }); +}); diff --git a/packages/shared/widget-plugin-grid/src/selection/createSelectionHelper.ts b/packages/shared/widget-plugin-grid/src/selection/createSelectionHelper.ts index c271cda1d2..d9acf6b5f7 100644 --- a/packages/shared/widget-plugin-grid/src/selection/createSelectionHelper.ts +++ b/packages/shared/widget-plugin-grid/src/selection/createSelectionHelper.ts @@ -20,12 +20,13 @@ export function createSelectionHelper( } if (selection.type === "Multi") { - helper = new MultiSelectionHelper(selection, datasource.items ?? []); + helper = new MultiSelectionHelper(selection, datasource.items ?? [], config.keepSelection); } else if (selection.type === "Single") { - helper = new SingleSelectionHelper(selection); + helper = new SingleSelectionHelper(selection, config.keepSelection); } - if (config.keepSelection) { - selection?.setKeepSelection(() => true); + + if (helper instanceof MultiSelectionHelper || helper instanceof SingleSelectionHelper) { + host.add(helper); } function setup(): (() => void) | void { diff --git a/packages/shared/widget-plugin-grid/src/selection/helpers.ts b/packages/shared/widget-plugin-grid/src/selection/helpers.ts index 27502fb26b..47f21ec19a 100644 --- a/packages/shared/widget-plugin-grid/src/selection/helpers.ts +++ b/packages/shared/widget-plugin-grid/src/selection/helpers.ts @@ -1,12 +1,22 @@ import type { ObjectItem, SelectionMultiValue, SelectionSingleValue } from "mendix"; -import { action, computed, makeObservable, observable } from "mobx"; +import { action, computed, makeObservable, observable, when } from "mobx"; +import { SetupComponent } from "@mendix/widget-plugin-mobx-kit/main"; +import { Direction, MoveEvent1D, MoveEvent2D, MultiSelectionStatus, ScrollKeyCode, SelectionMode, Size } from "./types"; import { MultiSelectionService } from "../interfaces/MultiSelectionService"; import { SingleSelectionService } from "../interfaces/SingleSelectionService"; -import { Direction, MoveEvent1D, MoveEvent2D, MultiSelectionStatus, ScrollKeyCode, SelectionMode, Size } from "./types"; -export class SingleSelectionHelper implements SingleSelectionService { +export class SingleSelectionHelper implements SingleSelectionService, SetupComponent { type = "Single" as const; - constructor(private selectionValue: SelectionSingleValue) { + /** Transient "retain now" flag the installed keep predicate reads live. */ + private keepActive = observable.box(true); + /** Disposer for the pending re-arm `when`, if a clear is in flight. */ + private disposeRearm: (() => void) | undefined; + private keepInstalled = false; + + constructor( + private selectionValue: SelectionSingleValue, + private keepSelection = false + ) { type PrivateMembers = "selectionValue"; makeObservable(this, { selectionValue: observable.ref, @@ -14,6 +24,19 @@ export class SingleSelectionHelper implements SingleSelectionService { }); } + setup(): (() => void) | void { + if (!this.keepSelection) { + return; + } + this.selectionValue.setKeepSelection(() => this.keepActive.get()); + this.keepInstalled = true; + return () => { + this.disposeRearm?.(); + this.disposeRearm = undefined; + this.keepInstalled = false; + }; + } + updateProps(value: SelectionSingleValue): void { this.selectionValue = value; } @@ -25,18 +48,47 @@ export class SingleSelectionHelper implements SingleSelectionService { this.selectionValue.setSelection(value); } remove(): void { + this.beforeClear(); this.selectionValue.setSelection(undefined); } + + /** + * Make the keep predicate yield around a clear, then re-arm it on the next + * datasource reconciliation. The runtime schedules new props on + * reconciliation, so a fresh `selectionValue` ref means the reconciliation + * (and its keep decision) is already past — safe to retain again. + */ + private beforeClear(): void { + if (!this.keepInstalled) { + return; + } + this.disposeRearm?.(); + const clearedRef = this.selectionValue; + this.keepActive.set(false); + this.disposeRearm = when( + () => this.selectionValue !== clearedRef, + () => { + this.keepActive.set(true); + this.disposeRearm = undefined; + } + ); + } } -export class MultiSelectionHelper implements MultiSelectionService { +export class MultiSelectionHelper implements MultiSelectionService, SetupComponent { type = "Multi" as const; private rangeStart: number | undefined; private rangeEnd: number | undefined; + /** Transient "retain now" flag the installed keep predicate reads live. */ + private keepActive = observable.box(true); + /** Disposer for the pending re-arm `when`, if a clear is in flight. */ + private disposeRearm: (() => void) | undefined; + private keepInstalled = false; constructor( private selectionValue: SelectionMultiValue, - private selectableItems: ObjectItem[] + private selectableItems: ObjectItem[], + private keepSelection = false ) { this.rangeStart = undefined; type PrivateMembers = "selectionValue" | "selectableItems"; @@ -48,6 +100,19 @@ export class MultiSelectionHelper implements MultiSelectionService { }); } + setup(): (() => void) | void { + if (!this.keepSelection) { + return; + } + this.selectionValue.setKeepSelection(() => this.keepActive.get()); + this.keepInstalled = true; + return () => { + this.disposeRearm?.(); + this.disposeRearm = undefined; + this.keepInstalled = false; + }; + } + isSelected(value: ObjectItem): boolean { return this.selectionValue.selection.some(obj => obj.id === value.id); } @@ -57,6 +122,28 @@ export class MultiSelectionHelper implements MultiSelectionService { this.selectableItems = items; } + /** + * Make the keep predicate yield around a clear, then re-arm it on the next + * datasource reconciliation. The runtime schedules new props on + * reconciliation, so a fresh `selectionValue` ref means the reconciliation + * (and its keep decision) is already past — safe to retain again. + */ + private beforeClear(): void { + if (!this.keepInstalled) { + return; + } + this.disposeRearm?.(); + const clearedRef = this.selectionValue; + this.keepActive.set(false); + this.disposeRearm = when( + () => this.selectionValue !== clearedRef, + () => { + this.keepActive.set(true); + this.disposeRearm = undefined; + } + ); + } + get selectionStatus(): MultiSelectionStatus { const selectionIds = new Set(this.selectionValue.selection.map(obj => obj.id)); const numberOfSelectedOnCurrentPage = this.selectableItems.reduce((acc, item) => { @@ -253,6 +340,7 @@ export class MultiSelectionHelper implements MultiSelectionService { * Clears the current selection by removing all selected items and resetting the selection range. */ clearSelection(): void { + this.beforeClear(); this.selectionValue.setSelection([]); this._resetRange(); } diff --git a/packages/shared/widget-plugin-test-utils/src/builders/SelectionMultiValueBuilder.ts b/packages/shared/widget-plugin-test-utils/src/builders/SelectionMultiValueBuilder.ts index fa4ea179ed..5feee023d3 100644 --- a/packages/shared/widget-plugin-test-utils/src/builders/SelectionMultiValueBuilder.ts +++ b/packages/shared/widget-plugin-test-utils/src/builders/SelectionMultiValueBuilder.ts @@ -1,16 +1,22 @@ import type { ObjectItem, SelectionMultiValue } from "mendix"; +type KeepSelectionPredicate = (item: ObjectItem) => boolean; + export class SelectionMultiValueBuilder { private selectionValue: SelectionMultiValue; + private keepSelectionPredicate: KeepSelectionPredicate | undefined; constructor() { + const builder = this; const value = { type: "Multi", selection: [] as ObjectItem[], setSelection(next: ObjectItem[]) { this.selection = next; }, - setKeepSelection(): void {} + setKeepSelection(predicate: KeepSelectionPredicate | undefined): void { + builder.keepSelectionPredicate = predicate; + } }; this.selectionValue = value as SelectionMultiValue; @@ -24,4 +30,9 @@ export class SelectionMultiValueBuilder { this.selectionValue.setSelection(items); return this; } + + /** Returns the predicate last passed to `setKeepSelection`, or `undefined` if none installed. */ + getKeepSelectionPredicate(): KeepSelectionPredicate | undefined { + return this.keepSelectionPredicate; + } } diff --git a/packages/shared/widget-plugin-test-utils/src/builders/SelectionSingleValueBuilder.ts b/packages/shared/widget-plugin-test-utils/src/builders/SelectionSingleValueBuilder.ts index c53b30d2a3..0ada48ed72 100644 --- a/packages/shared/widget-plugin-test-utils/src/builders/SelectionSingleValueBuilder.ts +++ b/packages/shared/widget-plugin-test-utils/src/builders/SelectionSingleValueBuilder.ts @@ -1,14 +1,21 @@ import type { ObjectItem, SelectionSingleValue } from "mendix"; +type KeepSelectionPredicate = (item: ObjectItem) => boolean; + export class SelectionSingleValueBuilder { private selectionValue: SelectionSingleValue; + private keepSelectionPredicate: KeepSelectionPredicate | undefined; constructor() { + const builder = this; const value = { type: "Single", selection: undefined as ObjectItem | undefined, setSelection(next: ObjectItem) { this.selection = next; + }, + setKeepSelection(predicate: KeepSelectionPredicate | undefined): void { + builder.keepSelectionPredicate = predicate; } }; @@ -23,4 +30,9 @@ export class SelectionSingleValueBuilder { this.selectionValue.setSelection(items); return this; } + + /** Returns the predicate last passed to `setKeepSelection`, or `undefined` if none installed. */ + getKeepSelectionPredicate(): KeepSelectionPredicate | undefined { + return this.keepSelectionPredicate; + } }