Skip to content

Fix relation-type and target-schema editing in the schema editor#951

Closed
JeroenDeDauw wants to merge 4 commits into
masterfrom
fix/clear-relation-attributes
Closed

Fix relation-type and target-schema editing in the schema editor#951
JeroenDeDauw wants to merge 4 commits into
masterfrom
fix/clear-relation-attributes

Conversation

@JeroenDeDauw

@JeroenDeDauw JeroenDeDauw commented Jun 27, 2026

Copy link
Copy Markdown
Member

Follows-up to #950

Result of @JeroenDeDauw's review of #950 and a follow-on design discussion: he requested the review, diagnosed the target-schema field as a free-text input that should be a constrained picker, and chose the searchable-dropdown direction.
Context: the NeoWiki codebase, PR #950, the sibling lookups, and a live browser check on Schema:Company.
Written by Claude Code, Opus 4.8

Two editing fixes in the Schema editor's relation property, both stemming from fields that let the displayed value drift away from what gets stored.

Relation type (free-text edge label)

updateRelation now emits the trimmed value even when empty, and the relation watcher no longer falls back to the property name. Clearing the field shows the required error and updates the model instead of silently keeping the previous value. The name fallback remains the on-create default (initial value + onMounted), preserving the "derive-on-create only" behaviour from #950.

Target schema (reference to an existing schema)

The field is a CdxLookup, so typed text could drift from the actual selection (e.g. Produc while Product stayed selected) with no feedback — and that stale value could be saved by changing another field. It is now a constrained picker: it only ever holds an existing schema or nothing.

  • Pick from the menu commits the schema.
  • Empty + blur clears it (and the relation editor shows "Target schema is required.").
  • Unmatched text + blur reverts to the last committed value.

Filtering still uses the existing server-side prefix search (schema-names API). SubjectCreatorDialog, which shares SchemaLookup, already ignores empty selections, so its behaviour is unchanged.

Required-field errors remain inline-only and still do not block Save — that schema-editor-wide gating was deferred in #950 and is unchanged here.

Known follow-up (not in this PR)

"Browse the full schema list before typing" is not included: the schema-names API is a prefix search capped at 10 results that rejects an empty query, so there is no cheap way to preload all names. Enabling browse needs a small new list-all-schema-names endpoint.

Tests

  • RelationAttributesEditor: clearing emits an empty relation; a cleared field stays empty (no name snap-back); whitespace-only is trimmed/required; null is passed to the lookup when no target schema is set.
  • SchemaLookup: server search + result rendering; pick commits; unmatched text reverts on blur; empty + blur clears; an unset field is not cleared.
  • Full vitest suite green (1055 tests); build + lint clean.

Manual browser check (done; recorded here for re-verification)

On Schema:CompanyEdit schema → select Main product:

  1. Relation type: clear it → stays empty, shows "Relation type is required."; type Owns → stored trimmed.
  2. Target schema: backspace ProductProduc, click away → reverts to Product.
  3. Target schema: type Off, pick Office from the dropdown → commits, no mid-type blanking.
  4. Target schema: clear it, click away → empty + "Target schema is required."

🤖 Generated with Claude Code

JeroenDeDauw and others added 3 commits June 27, 2026 15:12
Follows-up to #950

> Result of @JeroenDeDauw's review of #950: he requested the review, chose
> the fix direction for the two findings, and directed this follow-up.
> Context: the NeoWiki codebase, PR #950, and the sibling attribute editors.
> <sub>Written by Claude Code, `Opus 4.8`</sub>

Two follow-up fixes from the review of #950. Both stem from the same root: clearing
a required relation attribute left the editor holding the previous value, so the
field looked empty/blank while the stored value silently disagreed.

- Relation type: `updateRelation` now emits the trimmed value even when empty, and
  the `relation` watcher no longer falls back to the property name. Clearing the
  field shows the required error and updates the model instead of keeping the stale
  value. The name fallback stays as the on-create default (initial value +
  `onMounted`), preserving the "derive-on-create only" behavior from #950.
- Target schema: `SchemaLookup` now emits `select` on clear (dropping the
  `if ( schemaName )` guard that swallowed Codex's clear signal); the editor clears
  `targetSchema` accordingly and passes `null` to the lookup when unset. The other
  consumer, `SubjectCreatorDialog`, already ignores empty selections
  (`if ( !schemaName ) return`), so its behavior is unchanged.

Required-field errors remain inline-only and still do not block Save — that
schema-editor-wide gating was deferred in #950 and is unchanged here. These fixes
make the displayed and validated state match what gets stored.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous commit made SchemaLookup emit `select` on every cleared selection.
But CdxLookup also deselects (emits update:selected null) while the user types to
replace an existing value, so editing a set target schema round-tripped an empty
value back and wiped the field mid-edit.

Only emit the clear when the input is actually empty, matching SubjectLookup's
onSubjectSelected guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The watch no longer falls back to the property name, so the name override
implied a dependency the test does not exercise.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JeroenDeDauw JeroenDeDauw changed the title Reflect cleared relation type and target schema in the schema editor Fix relation-type and target-schema editing in the schema editor Jun 27, 2026
The target schema is a free-text CdxLookup, so typed text could drift from the
actual selection (e.g. "Produc" while "Product" stays selected) with no feedback,
and that stale value could be saved by changing another field.

Reconcile on blur: keep a schema picked from the menu, clear when the field is
emptied, otherwise revert unmatched text to the committed value. The field can now
only hold an existing schema or nothing. Filtering still uses the server-side
prefix search. Browser-verified on Schema:Company (revert, pick-to-replace, clear).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JeroenDeDauw

Copy link
Copy Markdown
Member Author

Folded into #953 for a single holistic review. #953 now targets master and contains all of this work — the relation-type clearing carries over unchanged, and the target-schema picker here (CdxLookup-based) was fully replaced there by a CdxCombobox one, so reviewing them separately would have meant reviewing code that no longer exists.

@JeroenDeDauw
JeroenDeDauw deleted the fix/clear-relation-attributes branch June 27, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant