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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-10-15
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
## Context

See `proposal.md` for the problem. Current state:

- `AddOrEditAttribute` receives `source` and `applied` as props. They fix the card's value options and type for its whole life. The form holds `source_attribute_id` and `source_name`. `ProductAttributeFormSchema` turns a name change into a `create` plan (copies the values) and an unchanged name into a `sync` plan.
- `AttributeNameInput` flags any clash on blur. `useApplyAttribute.apply` repeats that check and blocks.
- `useApplyAttribute.commit` already calls `replaceAttribute` when the persisted attribute id differs from `appliedId`.
- The chip color picker (`withEditableSwatch`) renders its content in a portal. React events from a portal bubble to the React parent, so they reach `MultiSelect`'s `handleBoxClick`. That handler focuses the input and opens the value popover. The new popover then counts the picker as an outside interaction and closes it.

## Goals / Non-Goals

**Goals:**

- A card can change the attribute it works on after a name blur.
- Picker interactions never open the value popover.

**Non-Goals:**

- No API, database or schema change. No server-side duplicate-name change.
- No change to the preset row or `+ Add` popover contents.
- No type chooser in the new-attribute form.

## Decisions

### 1. The card owns its source in state

`AddOrEditAttribute` keeps `activeSource` in `useState`, starting from `source ?? applied`. `existingValues` and `type` for `AttributeValuesField` come from `activeSource`. Switching sets `activeSource` and writes the form fields `source_attribute_id`, `source_name`, `name` (stored name), `type` and `values` (`[]`).

Alternative: remount the card with a new `source` prop. Rejected: the parent would own transient draft state, and the name input would lose focus and its blur sequence.

### 2. One classifier for a name match

`product-attribute-form.ts` gets a pure function that takes the typed name, the store attributes, the ids applied to the product (without the card's own `appliedId`), the current `source_attribute_id` and the selected value count. It returns `none`, `reuse`, `selected-clash` or `applied-clash` with the matched attribute. `AttributeNameInput` and `useApplyAttribute.apply` both use it, so the blur check and the Apply backstop cannot disagree. `findAttributeNameClash` stays as the matching primitive.

`AttributeNameInput` gets two new props: `appliedIds` and `onReuse(attribute)`. On `reuse` it calls `onReuse` and clears the name error. The two clash kinds set a `duplicate` error with different messages.

Alternative: put the match logic inside the card only. Rejected: the Apply backstop needs the same rules.

### 3. Apply needs no new write path

After a switch, `source_attribute_id` is the switched-to attribute and `name` equals its stored name. The schema returns a `sync` plan. New values are added to that attribute through the existing batch request. When the card edits an applied attribute with another id, `commit` already takes the `replaceAttribute` branch. No change is needed there. The unique-name-with-values path already returns a `create` plan with copied values, so the original stays unchanged. A test proves it.

### 4. Portal events are ignored by the box click handler

`handleBoxClick` returns early when the event target is not a DOM descendant of the box (`!event.currentTarget.contains(event.target as Node)`). This ignores clicks that bubbled through a portal. It applies to every `MultiSelect` consumer. It is safe: no consumer relies on a portal click opening the panel.

### 5. A layer-open signal stops the outside click from opening the popover

Radix closes the picker on pointer down. The click that follows can land on the box and open the popover. `MultiSelect` provides a small context. A chip renderer reports `open` and `closed` through it. The box records on `pointerdown` (capture) whether a chip layer was open, and `handleBoxClick` skips the open action once when it was. The context and its hook live in `components/ui/multi-select-chip-layer.ts`. A new `EditableSwatch` component (`features/products/components/editable-swatch.tsx`) reports through `ColorPicker`'s `onOpenChange`; `withEditableSwatch` renders it. The context is opt-in: a consumer that never reports behaves as before.

Alternatives: (a) query the DOM for an open Radix layer. Rejected: couples to library internals. (b) Keep the picker open and handle outside clicks ourselves. Rejected: re-implements dismiss behaviour.

## Risks / Trade-offs

- [Switching drops the card's name focus or draft] → The switch runs in the blur handler and only touches name, type, source fields and the empty `values`. It runs only when no values are selected, so no draft is lost.
- [Stale `activeSource` after the attribute list refetches] → The store list changes only through Apply, which closes the card.
- [Pointer-down flag goes stale] → The flag is recomputed at every `pointerdown`, and consumed by the next click.
- [Change in `MultiSelect` affects other fields] → Decision 4 only removes portal-bubbled clicks. Decision 5 is opt-in. Existing `multi-select` tests must still pass.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## Why

A merchant who types the name of an attribute that already exists, such as "Size", gets a blocking duplicate-name error. They cannot reuse the attribute and must cancel and pick it from `+ Add`. Separately, picking a color in a chip's color picker closes the picker and opens the values dropdown, so the color cannot be adjusted in one go.

## What Changes

- On blur, a name that matches an existing store attribute (trimmed, case-insensitive) and has no selected values switches the card to that attribute. Its values, type and stored name load. This works for `+ Add new`, presets, `+ Add` picks and edits of an applied attribute.
- A name that matches an existing attribute while values are selected still shows "An attribute with this name already exists."
- A name that matches an attribute already applied to this product shows "This attribute is already added to this product." and loads nothing.
- Renaming with selected values to a unique name keeps creating a new attribute with copies of the values. The original attribute keeps its values. This is existing behaviour and now has a test that proves it.
- Picking a color in a chip's color picker keeps the picker open and never opens the values dropdown. An outside click only closes the picker.

## Capabilities

### New Capabilities

None.

### Modified Capabilities

- `product-variations-card`: the "Attribute name input" requirement changes from "clash blocks Apply" to "clash reuses the attribute when no values are selected". The "Recoloring a selected value" requirement gains the picker/dropdown isolation rule. A new requirement covers reusing an existing attribute by name.

## Impact

- `resources/app/features/products/components/fields/attribute-name-input.tsx`
- `resources/app/features/products/components/fields/attribute-values-field.tsx`
- `resources/app/features/products/components/attribute-value-types.tsx`
- `resources/app/features/products/components/product-form/sections/variants/attribute-list/add-or-edit-attribute.tsx`
- `resources/app/features/products/components/product-form/sections/variants/attribute-list/use-apply-attribute.ts`
- `resources/app/features/products/schemas/forms/product-attribute-form.ts`
- `resources/app/components/ui/multi-select.tsx` (non-breaking change for other consumers)
- No API or database change.
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
## ADDED Requirements

### Requirement: Reusing an existing attribute by name

When the name field loses focus, and its trimmed, case-insensitive value matches an existing store attribute other than the one the card currently uses, the card SHALL behave by what is selected and where the attribute is used:

- With no selected values, and the matching attribute not applied to this product, the card SHALL switch to the matching attribute. The name field SHALL show the attribute's stored name. The value options and the value type SHALL come from that attribute. No error SHALL show. This SHALL hold for the `+ Add new` form, preset cards, cards opened from `+ Add`, and the edit card of an applied attribute.
- With one or more selected values, the name field SHALL show "An attribute with this name already exists." and Apply SHALL be blocked until the name changes.
- With the matching attribute already applied to this product in another card, the name field SHALL show "This attribute is already added to this product." and the card SHALL NOT load its values. Apply SHALL be blocked until the name changes.

After a switch, selecting values and clicking Apply SHALL use the switched-to attribute: new values are added to it, and no new attribute is created. When the card was the edit card of an applied attribute, Apply SHALL replace the applied attribute with the switched-to one, following "Committing an attribute" in the Apply requirement.

After a switch, the merchant MAY enter a name that matches no attribute. With no selected values, the card SHALL keep the previous value options available. With selected values, Apply SHALL create a new attribute under "Renaming creates a new attribute".

#### Scenario: New attribute form reuses Size

- **WHEN** a merchant opens `+ Add new`, enters "size" in the name field, and the field loses focus
- **AND** the Size attribute exists and is not applied to the product
- **THEN** the name field shows "Size"
- **AND** the value popover lists the values of Size
- **AND** no error is shown

#### Scenario: Editing an applied attribute to an existing name

- **WHEN** a merchant edits the applied Color card, removes every selected value, renames it to "Material" (Material exists, not applied), and the field loses focus
- **THEN** the card shows the values of Material
- **AND** clicking Apply replaces Color with Material on the product and no new attribute is created

#### Scenario: Existing name with selected values

- **WHEN** a merchant has selected "S" on a Size card, renames it to "Material" (Material exists), and the field loses focus
- **THEN** the name field shows "An attribute with this name already exists."
- **AND** Apply is blocked

#### Scenario: Name of an attribute already on this product

- **WHEN** Size is applied to the product, and a merchant opens `+ Add new`, enters "Size", and the field loses focus
- **THEN** the name field shows "This attribute is already added to this product."
- **AND** no values are loaded

#### Scenario: Unique name with selected values copies the values

- **WHEN** a merchant selects "S" and "M" on a Size card, renames it to "Fit Size" (no such attribute), and clicks Apply
- **THEN** a new attribute "Fit Size" with copies of "S" and "M" is created
- **AND** the Size attribute still has "S" and "M"

#### Scenario: Unique name with no selected values keeps options

- **WHEN** a merchant has switched a card to Material, then renames it to "Fabric" (no such attribute) with no values selected
- **THEN** the value popover still lists the values of Material

## MODIFIED Requirements

### Requirement: Attribute name input

The attribute name in edit mode SHALL be a regular bordered text input, aligned with the value input below it. On blur, the name SHALL be checked against existing attribute names, trimmed and case-insensitive, excluding the attribute the card currently uses. A match SHALL be handled by the "Reusing an existing attribute by name" requirement. If the server still rejects the name as a duplicate on Apply, that error SHALL be shown on the name field. An empty name SHALL block Apply with a required error.

#### Scenario: Duplicate name on blur with selected values

- **WHEN** a merchant has selected a value on a Size card, renames it to "material" (Material exists), and the field loses focus
- **THEN** the name field shows the duplicate-name error and Apply is blocked

#### Scenario: Unchanged name is not a clash

- **WHEN** a merchant opens the Size card and blurs the name without changing it
- **THEN** no error is shown and nothing is reloaded

### Requirement: Recoloring a selected value

In edit mode on a `color` attribute, clicking a chip's swatch SHALL open a color picker for that value. The chosen color SHALL update the chip immediately as a draft and SHALL be persisted on Apply. Cancel SHALL revert it. In view mode, swatches SHALL NOT be interactive.

Interacting with the color picker, including picking or dragging a color and typing a hex value, SHALL keep the picker open and SHALL NOT open the value popover. A click outside the picker SHALL close the picker only. If that click lands on the value input box, the value popover SHALL stay closed.

#### Scenario: Picking a new color

- **WHEN** a merchant clicks the Orange chip's swatch in edit mode and picks `#ff6600`
- **THEN** the chip's swatch shows `#ff6600` and nothing is sent until Apply

#### Scenario: Picker stays open while picking

- **WHEN** a merchant picks several colors in the open color picker
- **THEN** the picker stays open and the value popover does not open

#### Scenario: Outside click closes only the picker

- **WHEN** the color picker is open and the merchant clicks on the value input box
- **THEN** the color picker closes
- **AND** the value popover does not open
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
## 1. Name match logic

- [x] 1.1 Add the pure name-match classifier (`none`, `reuse`, `selected-clash`, `applied-clash`) and the "already added to this product" message to `resources/app/features/products/schemas/forms/product-attribute-form.ts` → verify: schema unit tests cover each kind, case and whitespace variants, and the card's own applied id
- [x] 1.2 Add schema tests in `resources/app/features/products/tests/schemas/forms/product-attribute-form.test.ts`, including: unique new name with selected values returns a `create` plan carrying the values and colors → verify: `npm test` passes

## 2. Card switches to the matching attribute

- [x] 2.1 In `add-or-edit-attribute.tsx`, keep `activeSource` in state. Feed `existingValues` and `type` of `AttributeValuesField` from it. Pass `appliedIds` and `onReuse` to `AttributeNameInput` → verify: typecheck passes
- [x] 2.2 In `attribute-name-input.tsx`, use the classifier on blur. `reuse` calls `onReuse` and clears the error. The clash kinds show their own message → verify: component test for each kind
- [x] 2.3 `onReuse` writes `source_attribute_id`, `source_name`, `name`, `type` and `values: []` to the form → verify: the value popover lists the matched attribute's values
- [x] 2.4 In `use-apply-attribute.ts`, use the classifier in the Apply backstop in place of the direct clash check → verify: Apply with a clashing name and selected values still blocks with the right message

## 3. Color picker isolation

- [x] 3.1 In `components/ui/multi-select.tsx`, ignore box clicks whose target is outside the box DOM → verify: existing `multi-select` tests pass
- [x] 3.2 In `multi-select.tsx`, add the `pointerdown` capture flag, and put the opt-in layer-open context and `useMultiSelectChipLayer` in `components/ui/multi-select-chip-layer.ts`. Skip one box-click open when a layer was open at pointer down → verify: unit test with a chip that reports open
- [x] 3.3 Report the picker's open state through the context from the new `features/products/components/editable-swatch.tsx`, used by `withEditableSwatch` in `attribute-value-types.tsx` → verify: component test

## 4. Tests

- [x] 4.1 `attribute-list.test.tsx`: replace the current duplicate-name test. Add tests for: `+ Add new` with "size" loads Size and shows "Size"; edit of an applied attribute renamed to an existing name replaces it on Apply with no create request; existing name with selected values shows the duplicate error; name of an applied attribute shows the "already added" error; unique name with selected values sends a create request and no change to the original; unique name with none selected keeps the options → verify: `npm test` passes
- [x] 4.2 `attribute-values-field.test.tsx`: picking a color keeps the picker open and the value popover closed; an outside click on the box closes the picker only → verify: `npm test` passes

## 5. Finish

- [x] 5.1 Run `npm run typecheck`, lint and `npm test` in `resources/app/` → verify: all pass
- [x] 5.2 Confirm no `docs/` page covers the variations card (none does today), so no doc update is needed. Ask the user to check the picker and the name reuse visually, because the browser is not used in this project → verify: reported to the user
Loading
Loading