Skip to content
Open
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
4 changes: 4 additions & 0 deletions packages/pluggableWidgets/combobox-web/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

## [Unreleased]

### Fixed

- We fixed an issue where selecting all of the typed text in a multi-select combobox and pressing Backspace did not permanently clear it, so the text reappeared after clicking away and back into the combobox.

## [2.9.0] - 2026-07-24

### Fixed
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { expect, test } from "@mendix/run-e2e/fixtures";

// Regression for WC-3347: in a multi-select combobox with at least one selected chip,
// selecting all filter text and pressing Backspace used to leave the text in place —
// the custom onKeyDown treated a select-all range (selectionStart === 0) as "caret at
// the start" and moved focus to the last chip, which reverted the pending input change.
// Delete never entered that branch, hence the reported Backspace/Delete asymmetry.
test.describe("combobox-web multi-selection filter input keys", () => {
test.beforeEach(async ({ page }) => {
await page.goto("/p/combobox");
await page.click(".mx-name-actionButton1");
await page.click(".mx-name-tabPage2");
});

for (const key of ["Backspace", "Delete"]) {
test(`clears the filter for good when all text is selected and ${key} is pressed`, async ({ page }) => {
const comboBox = page.locator(".mx-name-comboBox4");
await expect(comboBox).toBeVisible({ timeout: 10000 });

// Arrange: select two options so chips exist. The bug only surfaces with at
// least one chip — with none, setActiveIndex(-1) is a no-op.
await comboBox.click();
const options = comboBox.locator("[role=listbox] [role=option]");
await expect(options.first()).toBeVisible();
await options.nth(0).click({ delay: 10 });
await options.nth(1).click({ delay: 10 });

const chips = comboBox.locator(".widget-combobox-selected-item");
await expect(chips.first()).toBeVisible();

const input = comboBox.locator("input");
await input.click();
await page.keyboard.type("zzz");
await expect(input).toHaveValue("zzz");

// Act
await input.press("ControlOrMeta+a");
await input.press(key);

// Assert: cleared immediately, and still cleared after leaving and re-entering
// the widget. Click the container rather than the input: once the text is
// genuinely gone the input can collapse to zero width and not be clickable.
await expect(input).toHaveValue("");
await page.locator("body").click({ position: { x: 5, y: 5 } });
await comboBox.locator(".widget-combobox-input-container").click();
await expect(input).toHaveValue("");
});
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-14
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
## Context

`MultiSelection` layers a custom `onKeyDown` on top of the props returned by downshift's `useMultipleSelection().getDropdownProps()` and `useCombobox().getInputProps()`:

```ts
// src/components/MultiSelection/MultiSelection.tsx:59-65
onKeyDown: (event: KeyboardEvent) => {
if (
(event.key === "Backspace" && inputRef.current?.selectionStart === 0) ||
(event.key === "ArrowLeft" && isSelectedItemsBoxStyle && inputRef.current?.selectionStart === 0)
) {
setActiveIndex(selectedItems.length - 1);
}
```

The intent is "the caret has nowhere left to go inside the filter input, so hand keyboard focus to the last selected chip". The check is wrong because `selectionStart === 0` is also true for a _range_ selection that begins at 0 — which is exactly what Ctrl/Cmd+A produces. Only the start of the range is inspected; `selectionEnd` is never consulted.

**Why the text comes back.** The multi-select combobox deliberately preserves typed filter text across focus changes — its `stateReducer` returns the previous state on blur:

```ts
// src/hooks/useDownshiftMultiSelectProps.ts:222-223
case useCombobox.stateChangeTypes.InputBlur:
return { ...state, highlightedIndex: -1 };
```

`setActiveIndex(selectedItems.length - 1)` makes downshift move DOM focus onto the chip, so the input blurs in the _same_ keystroke that natively deleted the selected text. The `InputBlur` branch then resolves to `state` — the snapshot from before the deletion — discarding the `InputChange` that would have set `inputValue` to `""`. The input is controlled by `inputValue`, so the widget re-renders with the old text; the user sees it again on the next focus. Delete never enters the branch, never steals focus, and so never triggers this — matching the reported asymmetry.

**Confirmed by the reproduction (WC-3347 diagnostic).** The revert is immediate, not deferred to the next focus: right after select-all + Backspace the input still reads `value="zzz"` across the full 5s assertion retry window (14 samples), with `aria-expanded="true"`. The controlled input snaps back within the same render pass. The user perceives it as "the text came back when I clicked in again" only because focus had already jumped to a chip, so they were not looking at the input in between. Delete, run as a control in the same spec, passes — the input clears and stays clear.

The dispatch ordering inside downshift has not been step-through verified, but the observable outcome and the responsible branch are confirmed.

**The decisive find:** downshift already ships precisely the predicate this handler needs, and applies it to its _own_ dropdown Backspace handler:

```js
// downshift 7.6.2, dist/downshift.cjs.js:3432
function isKeyDownOperationPermitted(event) {
if (event.shiftKey || event.metaKey || event.ctrlKey || event.altKey) return false;
var element = event.target;
if (
element instanceof HTMLInputElement &&
element.value !== "" &&
(element.selectionStart !== 0 || element.selectionEnd !== 0)
)
return false;
return true;
}
```

So downshift's rule is: permit chip interaction only when the input is empty, **or** the caret is collapsed at position 0, and no modifier is held. Our custom handler checks one third of that. It is not exported from the package (absent from `typings/index.d.ts` and from the CJS exports), so it cannot be imported.

## Goals / Non-Goals

**Goals:**

- Deleting a full text selection with Backspace clears the filter input permanently — no resurfacing on blur/refocus.
- Chip focus transfer keeps working for the case it was written for: Backspace with an empty filter input.
- Align this handler's gating with downshift's own semantics, so the widget and the library agree on when a key belongs to the text field versus the chip list.
- Close the Backspace/Delete asymmetry with regression coverage on both paths.

**Non-Goals:**

- Changing how multi-select preserves filter text across blur/focus. The `InputBlur` reducer returning previous state is intentional and stays; we fix the unintended focus steal instead.
- Touching `SingleSelection`, which guards on `e.currentTarget.value === ""` and is unaffected.
- Upgrading downshift (the repo also has 9.3.6 in the store for other packages; this widget stays on `^7.6.2`).
- Reworking chip removal, `selectedItemsStyle`, or menu behaviour.

## Decisions

### 1. Mirror downshift's `isKeyDownOperationPermitted` in a local helper rather than inventing a new condition

Extract a small predicate in the widget (e.g. `isChipNavigationPermitted(event)`) replicating downshift's logic, and require it in both branches.

_Why:_ the bug is a divergence from the library's own contract, so converging on that contract fixes this instance and the neighbouring ones (modifier-held keypresses, range selections not anchored at 0) in one move. Copying ~6 lines is preferable to inventing a subtly different rule that will drift from downshift's behaviour on the chips themselves.

_Alternatives considered:_

- **Minimal patch — add `&& selectionEnd === 0` to both branches.** Fixes the reported bug and is the smallest diff, but leaves the modifier-key gap and permits activation with a collapsed caret at 0 while text is present, where downshift itself would refuse. Acceptable fallback if reviewers want the tightest possible change; noted as the reduced-scope option.
- **Guard on `value === ""` only (copy `SingleSelection`).** Simple and consistent across the two components, but strictly narrower than downshift: it would drop chip navigation for a collapsed caret at position 0 with text present, a case downshift explicitly permits. Rejected as an unnecessary behaviour regression for the ArrowLeft path.
- **Fix the `InputBlur` reducer to accept `changes.inputValue`.** Treats the symptom at the state layer, and risks discarding the deliberate "keep the filter text across blur" behaviour that other flows depend on. Rejected.

### 2. Apply the same guard to the ArrowLeft branch

ArrowLeft with an active text selection should collapse that selection, per standard text-field semantics, not jump to a chip. The branch shares the identical faulty check, so it is corrected together rather than left as a known-latent twin of the same bug.

### 3. Verify both key paths, and promote the diagnostic into a real regression test

The investigation's throwaway spec (`e2e/WC-backspace-stale-diagnostic.spec.js`) confirmed the Backspace failure but its Delete control case was never observed green (a click timeout plus Playwright grep-flag trouble). The change must land coverage that actually executes both, so "Delete is fine" stops being a code-reading inference. Unit tests with React Testing Library are the cheaper home for the key-handling matrix; an e2e test covers the blur/refocus round trip that produced the user-visible symptom. Prefer unit coverage for the matrix, plus one e2e for the round trip.

## Risks / Trade-offs

- **Copied library internals can drift from downshift on upgrade** → keep the helper small, comment it with the downshift source reference and version, and note that a downshift major upgrade should re-check it against `isKeyDownOperationPermitted`.
- **Some users may rely on Backspace-at-caret-0-with-text reaching the chips** → this is preserved: downshift's predicate permits a collapsed caret at 0. Only range selections and modifier-held presses change, which is the defect being fixed.
- **Hidden dependence on the current (buggy) behaviour in existing tests** → run the widget's full unit suite and the existing `e2e/Combobox.spec.js` before concluding; the known Backspace test there is single-select (`comboBox2`, enum) and should be unaffected.
- **Mechanism is inferred, not instrumented** → the fix targets the trigger (the guard), which the reproduction directly implicates, so it holds even if the precise dispatch ordering differs. The regression test, not the mechanism narrative, is what gates success.
- **Reproduction requires a running test project** and the multi-select combobox with at least one existing chip (`comboBox4` on tab page 2); `setActiveIndex(-1)` on an empty selection means the bug does not surface with zero chips.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
## Why

In a multi-select Combobox that already has at least one selected chip, selecting all filter text and pressing Backspace appears to clear the input, but the text silently returns as soon as the user clicks outside the widget and back in. A customer reported this; it has been reproduced against a live test project. Pressing Delete instead of Backspace does not show the problem, which makes the behaviour look arbitrary and erodes trust in the widget's basic text editing.

## What Changes

- Backspace no longer moves focus to the last selected chip when the filter input has a non-collapsed text selection (e.g. after select-all). Chip activation stays reserved for a collapsed caret sitting at position 0, which is the case the behaviour was designed for.
- The same collapsed-caret condition is applied to the ArrowLeft chip-navigation path in "boxes" selected-items style, which shares the identical faulty check.
- Deleting a full text selection with Backspace now clears the filter input for good — the text does not reappear on blur/refocus.
- Regression coverage is added for both key paths (Backspace and Delete) so the asymmetry cannot silently return.
- No changes to single-select behaviour, and no changes to the chip-removal behaviour users rely on today (Backspace on an empty input still targets the last chip).

## Capabilities

### New Capabilities

- `multiselect-keyboard-interaction`: Keyboard behaviour of the multi-select Combobox filter input — when Backspace/ArrowLeft transfer focus to selected chips for removal versus when they act as ordinary text editing within the filter input.

### Modified Capabilities

<!-- None. The package has no existing specs (openspec/specs/ is empty), so this
change introduces the first spec for this behaviour rather than modifying one. -->

## Impact

- **Code**: `src/components/MultiSelection/MultiSelection.tsx` — the `onKeyDown` handler passed into `getInputProps` (the `selectionStart === 0` guard on the Backspace and ArrowLeft branches).
- **Not affected**: `src/components/SingleSelection/SingleSelection.tsx` guards its Backspace branch on `e.currentTarget.value === ""`, so single-select never hits this defect.
- **Tests**: new regression coverage in `e2e/` and/or `src/**/__tests__/`. A throwaway diagnostic spec (`e2e/WC-backspace-stale-diagnostic.spec.js`) exists from the investigation and must be either promoted to a proper regression test or deleted.
- **Release**: patch version bump kept in sync across `package.json` and `src/package.xml`, plus a user-facing `CHANGELOG.md` entry under `## [Unreleased]`.
- **Dependencies**: none. Downshift stays on its current version; the fix is local to the widget's own key handler.
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
## ADDED Requirements

### Requirement: Filter input text editing takes precedence over chip navigation

In a multi-select Combobox, keys pressed while the filter input holds an active (non-collapsed) text selection SHALL act on that text and MUST NOT transfer keyboard focus to the selected items (chips). Focus transfer to chips SHALL be permitted only when the filter input is empty, or when the caret is collapsed at position 0, and no modifier key (Shift, Control, Meta, Alt) is held — matching the gating downshift applies to its own dropdown key handling.

#### Scenario: Backspace deletes a full text selection instead of activating a chip

- **WHEN** a multi-select Combobox has at least one selected chip, the user has typed filter text, selected all of it (Ctrl/Cmd+A), and presses Backspace
- **THEN** the filter input is cleared, keyboard focus remains in the filter input, and no chip becomes active

#### Scenario: Cleared text does not reappear after leaving and re-entering the widget

- **WHEN** the user has cleared a full text selection with Backspace, then clicks outside the Combobox and clicks back into the filter input
- **THEN** the filter input is still empty

#### Scenario: Delete behaves identically to Backspace on a full text selection

- **WHEN** a multi-select Combobox has at least one selected chip, the user has typed filter text, selected all of it, and presses Delete
- **THEN** the filter input is cleared, it remains empty after clicking outside and back in, and no chip becomes active

#### Scenario: Partial text selection is not treated as chip navigation

- **WHEN** the user selects part of the filter text starting at position 0 (leaving trailing text unselected) and presses Backspace
- **THEN** only the selected characters are removed, the remaining text is preserved, and no chip becomes active

### Requirement: Backspace on an empty filter input activates the last selected chip

Chip removal by keyboard SHALL remain available: when the filter input is empty, Backspace SHALL make the last selected chip the active item so it can be removed. This behaviour is unchanged by the text-editing precedence rule above.

#### Scenario: Backspace with an empty filter input targets the last chip

- **WHEN** a multi-select Combobox has one or more selected chips and the filter input is empty, and the user presses Backspace
- **THEN** the last selected chip becomes the active item

#### Scenario: Backspace with an empty filter input and no chips does nothing

- **WHEN** a multi-select Combobox has no selected chips and the filter input is empty, and the user presses Backspace
- **THEN** no chip is activated and the widget state is unchanged

### Requirement: ArrowLeft chip navigation requires a collapsed caret

In "boxes" selected-items style, ArrowLeft SHALL move focus to the last selected chip only when the caret is collapsed at position 0. When an active text selection exists, ArrowLeft SHALL follow standard text-field behaviour and MUST NOT transfer focus to the chips.

#### Scenario: ArrowLeft with a collapsed caret at the start reaches the chips

- **WHEN** the selected-items style is "boxes", the caret sits collapsed at position 0 of the filter input, and the user presses ArrowLeft
- **THEN** the last selected chip becomes the active item

#### Scenario: ArrowLeft with an active text selection stays in the filter input

- **WHEN** the selected-items style is "boxes", the filter text is fully selected, and the user presses ArrowLeft
- **THEN** keyboard focus remains in the filter input and no chip becomes active

### Requirement: Single-select behaviour is unaffected

The single-select Combobox SHALL continue to clear its selection when Backspace is pressed with an empty filter input, unchanged by this change.

#### Scenario: Single-select clear-with-Backspace still works

- **WHEN** a clearable single-select Combobox has a selected value and the user presses Backspace with an empty filter input
- **THEN** the selection is cleared and the placeholder is shown
Loading
Loading