From 8fd7f8f810dcf4f40889333eccb074becf17660e Mon Sep 17 00:00:00 2001 From: R-Hart80 Date: Fri, 5 Jun 2026 12:14:39 -0300 Subject: [PATCH 1/3] fix: make annotation placeholder rows respect property cardinality (closes #203) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Annotation properties like skos:prefLabel (SKOS S14) and skos:definition allow at most one value per language tag, but the editor was unconditionally appending an empty @en placeholder even when an English value already existed, inviting a duplicate that violates the cardinality constraint. - Add `AnnotationCardinality` type (`single` | `single-per-lang` | `multiple`) and a `cardinality` field to every entry in `ANNOTATION_PROPERTIES`. - Add `getAnnotationCardinality(iri)` helper (defaults to `"multiple"` for unknown IRIs, the safe non-restrictive fallback). - Centralise placeholder logic in `lib/ontology/annotationCardinality.ts`: `ensureTrailingPlaceholder(values, cardinality)` replaces the three identical copies of `ensureTrailingEmpty` that lived in the three panels. - `"multiple"` → unchanged trailing-empty behaviour. - `"single-per-lang"` → placeholder lang is the first common language (en → pt → es → fr → de → it) not yet covered by a filled value; no placeholder when all common langs are present. - `"single"` → no placeholder once any value is filled. - Update ClassDetailPanel, PropertyDetailPanel, IndividualDetailPanel to use the cardinality-aware helper at every call site. - Keep `ensureTrailingEmpty` exported from ClassDetailPanel as a deprecated wrapper so existing test imports compile without changes. - Add 23 unit tests covering all three cardinality modes and the cardinality lookup helper. Co-Authored-By: Claude Sonnet 4.6 --- .../ontology/annotationCardinality.test.ts | 137 ++++++++++++++++++ components/editor/ClassDetailPanel.tsx | 21 ++- components/editor/IndividualDetailPanel.tsx | 35 ++--- components/editor/PropertyDetailPanel.tsx | 32 ++-- lib/ontology/annotationCardinality.ts | 48 ++++++ lib/ontology/annotationProperties.ts | 115 +++++++++------ 6 files changed, 293 insertions(+), 95 deletions(-) create mode 100644 __tests__/lib/ontology/annotationCardinality.test.ts create mode 100644 lib/ontology/annotationCardinality.ts diff --git a/__tests__/lib/ontology/annotationCardinality.test.ts b/__tests__/lib/ontology/annotationCardinality.test.ts new file mode 100644 index 00000000..f3a404fd --- /dev/null +++ b/__tests__/lib/ontology/annotationCardinality.test.ts @@ -0,0 +1,137 @@ +import { describe, it, expect } from "vitest"; +import { ensureTrailingPlaceholder } from "@/lib/ontology/annotationCardinality"; +import { getAnnotationCardinality } from "@/lib/ontology/annotationProperties"; + +// ── ensureTrailingPlaceholder — "multiple" ──────────────────────────────── + +describe('ensureTrailingPlaceholder — "multiple"', () => { + it("appends empty row to empty array", () => { + expect(ensureTrailingPlaceholder([], "multiple")).toEqual([{ value: "", lang: "en" }]); + }); + + it("appends empty row when last item has a value", () => { + const result = ensureTrailingPlaceholder([{ value: "hello", lang: "en" }], "multiple"); + expect(result).toHaveLength(2); + expect(result[1]).toEqual({ value: "", lang: "en" }); + }); + + it("does not append when last item is already empty", () => { + const input = [{ value: "hello", lang: "en" }, { value: "", lang: "en" }]; + expect(ensureTrailingPlaceholder(input, "multiple")).toHaveLength(2); + }); + + it("does not append when last item is whitespace-only", () => { + expect(ensureTrailingPlaceholder([{ value: " ", lang: "en" }], "multiple")).toHaveLength(1); + }); +}); + +// ── ensureTrailingPlaceholder — "single-per-lang" ──────────────────────── + +describe('ensureTrailingPlaceholder — "single-per-lang"', () => { + it("adds an @en placeholder for an empty array", () => { + const result = ensureTrailingPlaceholder([], "single-per-lang"); + expect(result).toEqual([{ value: "", lang: "en" }]); + }); + + it("does NOT add a second @en placeholder when @en is already filled (SKOS S14)", () => { + const input = [{ value: "Foo", lang: "en" }]; + const result = ensureTrailingPlaceholder(input, "single-per-lang"); + // The placeholder lang must differ from "en" + expect(result).toHaveLength(2); + expect(result[1].value).toBe(""); + expect(result[1].lang).not.toBe("en"); + }); + + it("adds a placeholder with the next uncovered language when @en is filled", () => { + const result = ensureTrailingPlaceholder([{ value: "Foo", lang: "en" }], "single-per-lang"); + expect(result[1].lang).toBe("pt"); + }); + + it("keeps an existing trailing empty placeholder as-is", () => { + const input = [{ value: "Foo", lang: "en" }, { value: "", lang: "pt" }]; + expect(ensureTrailingPlaceholder(input, "single-per-lang")).toStrictEqual(input); + }); + + it("adds no placeholder when all common languages are already filled", () => { + const input = [ + { value: "A", lang: "en" }, + { value: "B", lang: "pt" }, + { value: "C", lang: "es" }, + { value: "D", lang: "fr" }, + { value: "E", lang: "de" }, + { value: "F", lang: "it" }, + ]; + expect(ensureTrailingPlaceholder(input, "single-per-lang")).toStrictEqual(input); + }); + + it("skips covered languages and finds the next available one", () => { + const input = [{ value: "Foo", lang: "en" }, { value: "Bar", lang: "pt" }]; + const result = ensureTrailingPlaceholder(input, "single-per-lang"); + expect(result).toHaveLength(3); + expect(result[2].lang).toBe("es"); + }); +}); + +// ── ensureTrailingPlaceholder — "single" ───────────────────────────────── + +describe('ensureTrailingPlaceholder — "single"', () => { + it("returns one empty placeholder for an empty array", () => { + expect(ensureTrailingPlaceholder([], "single")).toEqual([{ value: "", lang: "en" }]); + }); + + it("returns the filled value with no trailing empty once a value exists", () => { + const input = [{ value: "2024-01-01", lang: "en" }]; + expect(ensureTrailingPlaceholder(input, "single")).toStrictEqual(input); + }); + + it("strips any trailing empty when a value is present", () => { + const input = [{ value: "ABC", lang: "en" }, { value: "", lang: "en" }]; + const result = ensureTrailingPlaceholder(input, "single"); + expect(result).toEqual([{ value: "ABC", lang: "en" }]); + }); + + it("keeps the empty placeholder while the user is still typing (value is non-empty)", () => { + const input = [{ value: "2024", lang: "en" }]; + expect(ensureTrailingPlaceholder(input, "single")).toStrictEqual(input); + }); +}); + +// ── getAnnotationCardinality ────────────────────────────────────────────── + +describe("getAnnotationCardinality", () => { + it("returns single-per-lang for skos:prefLabel", () => { + expect(getAnnotationCardinality("http://www.w3.org/2004/02/skos/core#prefLabel")).toBe("single-per-lang"); + }); + + it("returns single-per-lang for skos:definition", () => { + expect(getAnnotationCardinality("http://www.w3.org/2004/02/skos/core#definition")).toBe("single-per-lang"); + }); + + it("returns single for skos:notation", () => { + expect(getAnnotationCardinality("http://www.w3.org/2004/02/skos/core#notation")).toBe("single"); + }); + + it("returns single for dcterms:created", () => { + expect(getAnnotationCardinality("http://purl.org/dc/terms/created")).toBe("single"); + }); + + it("returns single for dcterms:modified", () => { + expect(getAnnotationCardinality("http://purl.org/dc/terms/modified")).toBe("single"); + }); + + it("returns multiple for skos:altLabel", () => { + expect(getAnnotationCardinality("http://www.w3.org/2004/02/skos/core#altLabel")).toBe("multiple"); + }); + + it("returns multiple for rdfs:comment (COMMENT_IRI)", () => { + expect(getAnnotationCardinality("http://www.w3.org/2000/01/rdf-schema#comment")).toBe("multiple"); + }); + + it("returns single-per-lang for rdfs:label (LABEL_IRI)", () => { + expect(getAnnotationCardinality("http://www.w3.org/2000/01/rdf-schema#label")).toBe("single-per-lang"); + }); + + it("defaults to multiple for unknown IRIs", () => { + expect(getAnnotationCardinality("http://example.org/custom#prop")).toBe("multiple"); + }); +}); diff --git a/components/editor/ClassDetailPanel.tsx b/components/editor/ClassDetailPanel.tsx index 9119b552..99fd895e 100644 --- a/components/editor/ClassDetailPanel.tsx +++ b/components/editor/ClassDetailPanel.tsx @@ -37,7 +37,8 @@ import { ParentClassPicker } from "@/components/editor/ParentClassPicker"; import { AnnotationRow } from "@/components/editor/standard/AnnotationRow"; import { InlineAnnotationAdder } from "@/components/editor/standard/InlineAnnotationAdder"; import { RelationshipSection, type RelationshipGroup, type RelationshipTarget } from "@/components/editor/standard/RelationshipSection"; -import { LABEL_IRI, COMMENT_IRI, DEFINITION_IRI, RELATIONSHIP_PROPERTY_IRIS, SEE_ALSO_IRI, getAnnotationPropertyInfo } from "@/lib/ontology/annotationProperties"; +import { LABEL_IRI, COMMENT_IRI, DEFINITION_IRI, RELATIONSHIP_PROPERTY_IRIS, SEE_ALSO_IRI, getAnnotationPropertyInfo, getAnnotationCardinality } from "@/lib/ontology/annotationProperties"; +import { ensureTrailingPlaceholder } from "@/lib/ontology/annotationCardinality"; import { AutoSaveAffordanceBar } from "@/components/editor/AutoSaveAffordanceBar"; import { CrossReferencesPanel } from "@/components/editor/CrossReferencesPanel"; import { SimilarConceptsPanel } from "@/components/editor/SimilarConceptsPanel"; @@ -45,12 +46,9 @@ import { EntityHistoryTab } from "@/components/editor/EntityHistoryTab"; import { useAutoSave } from "@/lib/hooks/useAutoSave"; import { useToast } from "@/lib/context/ToastContext"; -/** Ensure an array of localized strings always ends with an empty placeholder row */ +/** @deprecated Use ensureTrailingPlaceholder from lib/ontology/annotationCardinality with an explicit cardinality. */ export function ensureTrailingEmpty(arr: LocalizedString[]): LocalizedString[] { - if (arr.length === 0 || arr[arr.length - 1].value.trim() !== "") { - return [...arr, { value: "", lang: "en" }]; - } - return arr; + return ensureTrailingPlaceholder(arr, "multiple"); } /** Minimal data from the tree node, used as fallback when the API has no data yet */ @@ -199,7 +197,7 @@ export function ClassDetailPanel({ } else { regularAnnotations.push({ property_iri: a.property_iri, - values: ensureTrailingEmpty(a.values.map((v) => ({ ...v }))), + values: ensureTrailingPlaceholder(a.values.map((v) => ({ ...v })), getAnnotationCardinality(a.property_iri)), }); } } @@ -426,7 +424,7 @@ export function ClassDetailPanel({ prev.map((a) => { if (a.property_iri !== propertyIri) return a; const updated = a.values.map((v, vi) => (vi === valueIdx ? { ...v, [field]: newVal } : v)); - return { ...a, values: ensureTrailingEmpty(updated) }; + return { ...a, values: ensureTrailingPlaceholder(updated, getAnnotationCardinality(propertyIri)) }; }) ); }, @@ -439,7 +437,7 @@ export function ClassDetailPanel({ prev.map((a) => { if (a.property_iri !== propertyIri) return a; const filtered = a.values.filter((_, vi) => vi !== valueIdx); - return { ...a, values: ensureTrailingEmpty(filtered) }; + return { ...a, values: ensureTrailingPlaceholder(filtered, getAnnotationCardinality(propertyIri)) }; }) ); requestAnimationFrame(() => triggerSave()); @@ -869,16 +867,17 @@ export function ClassDetailPanel({ onAdd={(propertyIri, value, lang) => { setEditAnnotations((prev) => { const existing = prev.find((a) => a.property_iri === propertyIri); + const cardinality = getAnnotationCardinality(propertyIri); if (existing) { return prev.map((a) => a.property_iri === propertyIri - ? { ...a, values: ensureTrailingEmpty([...a.values, { value, lang }]) } + ? { ...a, values: ensureTrailingPlaceholder([...a.values, { value, lang }], cardinality) } : a ); } return [ ...prev, - { property_iri: propertyIri, values: ensureTrailingEmpty([{ value, lang }]) }, + { property_iri: propertyIri, values: ensureTrailingPlaceholder([{ value, lang }], cardinality) }, ]; }); requestAnimationFrame(() => triggerSave()); diff --git a/components/editor/IndividualDetailPanel.tsx b/components/editor/IndividualDetailPanel.tsx index adcf78d0..d94604d6 100644 --- a/components/editor/IndividualDetailPanel.tsx +++ b/components/editor/IndividualDetailPanel.tsx @@ -25,7 +25,8 @@ import { AnnotationRow } from "@/components/editor/standard/AnnotationRow"; import { InlineAnnotationAdder } from "@/components/editor/standard/InlineAnnotationAdder"; import { RelationshipSection, type RelationshipGroup, type RelationshipTarget } from "@/components/editor/standard/RelationshipSection"; import { PropertyAssertionSection } from "@/components/editor/standard/PropertyAssertionSection"; -import { LABEL_IRI, COMMENT_IRI, DEFINITION_IRI, SEE_ALSO_IRI, getAnnotationPropertyInfo } from "@/lib/ontology/annotationProperties"; +import { LABEL_IRI, COMMENT_IRI, DEFINITION_IRI, SEE_ALSO_IRI, getAnnotationPropertyInfo, getAnnotationCardinality } from "@/lib/ontology/annotationProperties"; +import { ensureTrailingPlaceholder } from "@/lib/ontology/annotationCardinality"; import { AutoSaveAffordanceBar } from "@/components/editor/AutoSaveAffordanceBar"; import { useEntityAutoSave } from "@/lib/hooks/useEntityAutoSave"; import { useToast } from "@/lib/context/ToastContext"; @@ -37,12 +38,6 @@ import { import { type IndividualDraftEntry } from "@/lib/stores/draftStore"; import { useIriLabels } from "@/lib/hooks/useIriLabels"; -function ensureTrailingEmpty(arr: LocalizedString[]): LocalizedString[] { - if (arr.length === 0 || arr[arr.length - 1].value.trim() !== "") { - return [...arr, { value: "", lang: "en" }]; - } - return arr; -} interface IndividualDetailPanelProps { projectId: string; @@ -198,9 +193,9 @@ export function IndividualDetailPanel({ }); const initEditState = useCallback((d: ParsedIndividualDetail) => { - setEditLabels(ensureTrailingEmpty(d.labels.map((l) => ({ ...l })))); - setEditComments(ensureTrailingEmpty(d.comments.map((c) => ({ ...c })))); - setEditDefinitions(ensureTrailingEmpty(d.definitions.map((def) => ({ ...def })))); + setEditLabels(ensureTrailingPlaceholder(d.labels.map((l) => ({ ...l })), "single-per-lang")); + setEditComments(ensureTrailingPlaceholder(d.comments.map((c) => ({ ...c })), "multiple")); + setEditDefinitions(ensureTrailingPlaceholder(d.definitions.map((def) => ({ ...def })), "single-per-lang")); setEditTypeIris([...d.typeIris]); setEditSameAsIris([...d.sameAsIris]); setEditDifferentFromIris([...d.differentFromIris]); @@ -229,7 +224,7 @@ export function IndividualDetailPanel({ const regularAnnotations = d.annotations .filter((a) => a.property_iri !== DEFINITION_IRI) - .map((a) => ({ ...a, values: ensureTrailingEmpty(a.values.map((v) => ({ ...v }))) })); + .map((a) => ({ ...a, values: ensureTrailingPlaceholder(a.values.map((v) => ({ ...v })), getAnnotationCardinality(a.property_iri)) })); setEditAnnotations(regularAnnotations); }, []); @@ -283,8 +278,8 @@ export function IndividualDetailPanel({ const d = restoredDraft as IndividualDraftEntry; // eslint-disable-next-line react-hooks/set-state-in-effect -- restoring draft state from store; matches PropertyDetailPanel auto-enter pattern setEditLabels(d.labels); - setEditComments(ensureTrailingEmpty(d.comments)); - setEditDefinitions(ensureTrailingEmpty(d.definitions)); + setEditComments(ensureTrailingPlaceholder(d.comments, "multiple")); + setEditDefinitions(ensureTrailingPlaceholder(d.definitions, "single-per-lang")); setEditTypeIris(d.typeIris); setEditSameAsIris(d.sameAsIris); setEditDifferentFromIris(d.differentFromIris); @@ -303,7 +298,7 @@ export function IndividualDetailPanel({ // ── Edit helpers ── const updateLabel = useCallback((index: number, field: "value" | "lang", val: string) => { - setEditLabels((prev) => ensureTrailingEmpty(prev.map((l, i) => (i === index ? { ...l, [field]: val } : l)))); + setEditLabels((prev) => ensureTrailingPlaceholder(prev.map((l, i) => (i === index ? { ...l, [field]: val } : l)), "single-per-lang")); }, []); const removeLabel = useCallback((index: number) => { setEditLabels((prev) => prev.filter((_, i) => i !== index)); @@ -311,18 +306,18 @@ export function IndividualDetailPanel({ }, [triggerSave]); const updateComment = useCallback((index: number, field: "value" | "lang", val: string) => { - setEditComments((prev) => ensureTrailingEmpty(prev.map((c, i) => (i === index ? { ...c, [field]: val } : c)))); + setEditComments((prev) => ensureTrailingPlaceholder(prev.map((c, i) => (i === index ? { ...c, [field]: val } : c)), "multiple")); }, []); const removeComment = useCallback((index: number) => { - setEditComments((prev) => ensureTrailingEmpty(prev.filter((_, i) => i !== index))); + setEditComments((prev) => ensureTrailingPlaceholder(prev.filter((_, i) => i !== index), "multiple")); requestAnimationFrame(() => triggerSave()); }, [triggerSave]); const updateDefinition = useCallback((index: number, field: "value" | "lang", val: string) => { - setEditDefinitions((prev) => ensureTrailingEmpty(prev.map((d, i) => (i === index ? { ...d, [field]: val } : d)))); + setEditDefinitions((prev) => ensureTrailingPlaceholder(prev.map((d, i) => (i === index ? { ...d, [field]: val } : d)), "single-per-lang")); }, []); const removeDefinition = useCallback((index: number) => { - setEditDefinitions((prev) => ensureTrailingEmpty(prev.filter((_, i) => i !== index))); + setEditDefinitions((prev) => ensureTrailingPlaceholder(prev.filter((_, i) => i !== index), "single-per-lang")); requestAnimationFrame(() => triggerSave()); }, [triggerSave]); @@ -332,7 +327,7 @@ export function IndividualDetailPanel({ prev.map((a) => { if (a.property_iri !== propertyIri) return a; const updated = a.values.map((v, vi) => (vi === valueIdx ? { ...v, [field]: val } : v)); - return { ...a, values: ensureTrailingEmpty(updated) }; + return { ...a, values: ensureTrailingPlaceholder(updated, getAnnotationCardinality(propertyIri)) }; }) ); }, [] @@ -342,7 +337,7 @@ export function IndividualDetailPanel({ setEditAnnotations((prev) => prev.map((a) => { if (a.property_iri !== propertyIri) return a; - return { ...a, values: ensureTrailingEmpty(a.values.filter((_, vi) => vi !== valueIdx)) }; + return { ...a, values: ensureTrailingPlaceholder(a.values.filter((_, vi) => vi !== valueIdx), getAnnotationCardinality(propertyIri)) }; }) ); requestAnimationFrame(() => triggerSave()); diff --git a/components/editor/PropertyDetailPanel.tsx b/components/editor/PropertyDetailPanel.tsx index d67eabeb..75316e3f 100644 --- a/components/editor/PropertyDetailPanel.tsx +++ b/components/editor/PropertyDetailPanel.tsx @@ -25,7 +25,8 @@ import { LanguagePicker } from "@/components/editor/LanguagePicker"; import { AnnotationRow } from "@/components/editor/standard/AnnotationRow"; import { InlineAnnotationAdder } from "@/components/editor/standard/InlineAnnotationAdder"; import { RelationshipSection, type RelationshipGroup, type RelationshipTarget } from "@/components/editor/standard/RelationshipSection"; -import { LABEL_IRI, COMMENT_IRI, DEFINITION_IRI, SEE_ALSO_IRI, getAnnotationPropertyInfo } from "@/lib/ontology/annotationProperties"; +import { LABEL_IRI, COMMENT_IRI, DEFINITION_IRI, SEE_ALSO_IRI, getAnnotationPropertyInfo, getAnnotationCardinality } from "@/lib/ontology/annotationProperties"; +import { ensureTrailingPlaceholder } from "@/lib/ontology/annotationCardinality"; import { AutoSaveAffordanceBar } from "@/components/editor/AutoSaveAffordanceBar"; import { useEntityAutoSave } from "@/lib/hooks/useEntityAutoSave"; import { useToast } from "@/lib/context/ToastContext"; @@ -38,13 +39,6 @@ import { import { type PropertyDraftEntry } from "@/lib/stores/draftStore"; import { useIriLabels } from "@/lib/hooks/useIriLabels"; -/** Ensure an array of localized strings always ends with an empty placeholder row */ -function ensureTrailingEmpty(arr: LocalizedString[]): LocalizedString[] { - if (arr.length === 0 || arr[arr.length - 1].value.trim() !== "") { - return [...arr, { value: "", lang: "en" }]; - } - return arr; -} const PROPERTY_TYPE_LABELS: Record = { object: { label: "Object Property", letter: "O", color: "bg-emerald-100 border-emerald-300 text-emerald-700 dark:bg-emerald-900/30 dark:border-emerald-700 dark:text-emerald-400" }, @@ -209,8 +203,8 @@ export function PropertyDetailPanel({ const initEditState = useCallback((d: ParsedPropertyDetail) => { setEditPropertyType(d.propertyType); setEditLabels(d.labels.length > 0 ? d.labels.map((l) => ({ ...l })) : [{ value: "", lang: "en" }]); - setEditComments(ensureTrailingEmpty(d.comments.map((c) => ({ ...c })))); - setEditDefinitions(ensureTrailingEmpty(d.definitions.map((def) => ({ ...def })))); + setEditComments(ensureTrailingPlaceholder(d.comments.map((c) => ({ ...c })), "multiple")); + setEditDefinitions(ensureTrailingPlaceholder(d.definitions.map((def) => ({ ...def })), "single-per-lang")); setEditDomainIris([...d.domainIris]); setEditRangeIris([...d.rangeIris]); setEditParentIris([...d.parentIris]); @@ -240,7 +234,7 @@ export function PropertyDetailPanel({ // Annotations: filter out definition (shown in its own section) const regularAnnotations = d.annotations .filter((a) => a.property_iri !== DEFINITION_IRI) - .map((a) => ({ ...a, values: ensureTrailingEmpty(a.values.map((v) => ({ ...v }))) })); + .map((a) => ({ ...a, values: ensureTrailingPlaceholder(a.values.map((v) => ({ ...v })), getAnnotationCardinality(a.property_iri)) })); if (!regularAnnotations.find((a) => a.property_iri === DEFINITION_IRI)) { // Don't add definition here — it has its own section @@ -292,8 +286,8 @@ export function PropertyDetailPanel({ const d = restoredDraft as PropertyDraftEntry; setEditPropertyType(d.propertyType); setEditLabels(d.labels); - setEditComments(ensureTrailingEmpty(d.comments)); - setEditDefinitions(ensureTrailingEmpty(d.definitions)); + setEditComments(ensureTrailingPlaceholder(d.comments, "multiple")); + setEditDefinitions(ensureTrailingPlaceholder(d.definitions, "single-per-lang")); setEditDomainIris(d.domainIris); setEditRangeIris(d.rangeIris); setEditParentIris(d.parentIris); @@ -321,20 +315,20 @@ export function PropertyDetailPanel({ }, [triggerSave]); const updateComment = useCallback((index: number, field: "value" | "lang", val: string) => { - setEditComments((prev) => ensureTrailingEmpty(prev.map((c, i) => (i === index ? { ...c, [field]: val } : c)))); + setEditComments((prev) => ensureTrailingPlaceholder(prev.map((c, i) => (i === index ? { ...c, [field]: val } : c)), "multiple")); }, []); const removeComment = useCallback((index: number) => { - setEditComments((prev) => ensureTrailingEmpty(prev.filter((_, i) => i !== index))); + setEditComments((prev) => ensureTrailingPlaceholder(prev.filter((_, i) => i !== index), "multiple")); requestAnimationFrame(() => triggerSave()); }, [triggerSave]); const updateDefinition = useCallback((index: number, field: "value" | "lang", val: string) => { - setEditDefinitions((prev) => ensureTrailingEmpty(prev.map((d, i) => (i === index ? { ...d, [field]: val } : d)))); + setEditDefinitions((prev) => ensureTrailingPlaceholder(prev.map((d, i) => (i === index ? { ...d, [field]: val } : d)), "single-per-lang")); }, []); const removeDefinition = useCallback((index: number) => { - setEditDefinitions((prev) => ensureTrailingEmpty(prev.filter((_, i) => i !== index))); + setEditDefinitions((prev) => ensureTrailingPlaceholder(prev.filter((_, i) => i !== index), "single-per-lang")); requestAnimationFrame(() => triggerSave()); }, [triggerSave]); @@ -344,7 +338,7 @@ export function PropertyDetailPanel({ prev.map((a) => { if (a.property_iri !== propertyIri) return a; const updated = a.values.map((v, vi) => (vi === valueIdx ? { ...v, [field]: val } : v)); - return { ...a, values: ensureTrailingEmpty(updated) }; + return { ...a, values: ensureTrailingPlaceholder(updated, getAnnotationCardinality(propertyIri)) }; }) ); }, @@ -357,7 +351,7 @@ export function PropertyDetailPanel({ prev.map((a) => { if (a.property_iri !== propertyIri) return a; const filtered = a.values.filter((_, vi) => vi !== valueIdx); - return { ...a, values: ensureTrailingEmpty(filtered) }; + return { ...a, values: ensureTrailingPlaceholder(filtered, getAnnotationCardinality(propertyIri)) }; }) ); requestAnimationFrame(() => triggerSave()); diff --git a/lib/ontology/annotationCardinality.ts b/lib/ontology/annotationCardinality.ts new file mode 100644 index 00000000..403a9e87 --- /dev/null +++ b/lib/ontology/annotationCardinality.ts @@ -0,0 +1,48 @@ +import type { LocalizedString } from "@/lib/api/client"; +import type { AnnotationCardinality } from "./annotationProperties"; + +/** Language tags to try in order when choosing a default lang for a new placeholder row. */ +const COMMON_LANGS = ["en", "pt", "es", "fr", "de", "it"]; + +/** + * Ensures that an array of localized strings has an appropriate trailing + * placeholder row, respecting the annotation property's cardinality: + * + * - "multiple": always one trailing empty row (original behaviour). + * - "single-per-lang": placeholder lang is the first common language not yet + * covered by a filled value; no placeholder when all common langs are filled. + * - "single": no placeholder once any value is filled; one empty row when empty. + */ +export function ensureTrailingPlaceholder( + values: LocalizedString[], + cardinality: AnnotationCardinality, +): LocalizedString[] { + switch (cardinality) { + case "single": { + const hasFilled = values.some((v) => v.value.trim() !== ""); + if (hasFilled) return values.filter((v) => v.value.trim() !== ""); + return [{ value: "", lang: "en" }]; + } + + case "single-per-lang": { + // Keep any existing trailing empty placeholder as-is. + if (values.length > 0 && values[values.length - 1].value.trim() === "") return values; + // Append a placeholder whose lang is the first common language not yet + // covered by a filled value. This prevents offering a duplicate @en row + // when the annotation already has an English value (SKOS S14 / rdfs:label + // convention). If all common languages are already present, no placeholder. + const filledLangs = new Set(values.filter((v) => v.value.trim()).map((v) => v.lang)); + const nextLang = COMMON_LANGS.find((l) => !filledLangs.has(l)); + if (nextLang === undefined) return values; + return [...values, { value: "", lang: nextLang }]; + } + + case "multiple": + default: { + if (values.length === 0 || values[values.length - 1].value.trim() !== "") { + return [...values, { value: "", lang: "en" }]; + } + return values; + } + } +} diff --git a/lib/ontology/annotationProperties.ts b/lib/ontology/annotationProperties.ts index 24006cc1..e3f10085 100644 --- a/lib/ontology/annotationProperties.ts +++ b/lib/ontology/annotationProperties.ts @@ -7,67 +7,75 @@ * - `curie`: prefixed name (e.g., "skos:prefLabel") — shown in tooltips * - `displayLabel`: plain-language name (e.g., "Preferred Label") — shown in UI * - `vocabulary`: grouping label for the picker + * - `cardinality`: how many values the property allows */ +/** How many values an annotation property accepts. */ +export type AnnotationCardinality = + | "single" // at most one value (e.g. dcterms:created) + | "single-per-lang" // at most one value per language tag (e.g. skos:prefLabel, SKOS S14) + | "multiple"; // unbounded (e.g. skos:altLabel, rdfs:comment) + export interface KnownAnnotationProperty { iri: string; curie: string; displayLabel: string; vocabulary: string; + cardinality: AnnotationCardinality; } export const ANNOTATION_PROPERTIES: KnownAnnotationProperty[] = [ - // ── DC Elements 1.1 ── - { iri: "http://purl.org/dc/elements/1.1/contributor", curie: "dc:contributor", displayLabel: "Contributor", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/coverage", curie: "dc:coverage", displayLabel: "Coverage", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/creator", curie: "dc:creator", displayLabel: "Creator", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/date", curie: "dc:date", displayLabel: "Date", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/description", curie: "dc:description", displayLabel: "Description", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/format", curie: "dc:format", displayLabel: "Format", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/identifier", curie: "dc:identifier", displayLabel: "Identifier", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/language", curie: "dc:language", displayLabel: "Language", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/publisher", curie: "dc:publisher", displayLabel: "Publisher", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/relation", curie: "dc:relation", displayLabel: "Relation", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/rights", curie: "dc:rights", displayLabel: "Rights", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/source", curie: "dc:source", displayLabel: "Source", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/subject", curie: "dc:subject", displayLabel: "Subject", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/title", curie: "dc:title", displayLabel: "Title", vocabulary: "DC Elements" }, - { iri: "http://purl.org/dc/elements/1.1/type", curie: "dc:type", displayLabel: "Type", vocabulary: "DC Elements" }, + // ── DC Elements 1.1 — legacy, all multi-valued ── + { iri: "http://purl.org/dc/elements/1.1/contributor", curie: "dc:contributor", displayLabel: "Contributor", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/coverage", curie: "dc:coverage", displayLabel: "Coverage", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/creator", curie: "dc:creator", displayLabel: "Creator", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/date", curie: "dc:date", displayLabel: "Date", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/description", curie: "dc:description", displayLabel: "Description", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/format", curie: "dc:format", displayLabel: "Format", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/identifier", curie: "dc:identifier", displayLabel: "Identifier", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/language", curie: "dc:language", displayLabel: "Language", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/publisher", curie: "dc:publisher", displayLabel: "Publisher", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/relation", curie: "dc:relation", displayLabel: "Relation", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/rights", curie: "dc:rights", displayLabel: "Rights", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/source", curie: "dc:source", displayLabel: "Source", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/subject", curie: "dc:subject", displayLabel: "Subject", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/title", curie: "dc:title", displayLabel: "Title", vocabulary: "DC Elements", cardinality: "multiple" }, + { iri: "http://purl.org/dc/elements/1.1/type", curie: "dc:type", displayLabel: "Type", vocabulary: "DC Elements", cardinality: "multiple" }, // ── DC Terms ── - { iri: "http://purl.org/dc/terms/contributor", curie: "dcterms:contributor", displayLabel: "Contributor", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/created", curie: "dcterms:created", displayLabel: "Date Created", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/creator", curie: "dcterms:creator", displayLabel: "Creator", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/date", curie: "dcterms:date", displayLabel: "Date", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/description", curie: "dcterms:description", displayLabel: "Description", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/format", curie: "dcterms:format", displayLabel: "Format", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/identifier", curie: "dcterms:identifier", displayLabel: "Identifier", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/language", curie: "dcterms:language", displayLabel: "Language", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/license", curie: "dcterms:license", displayLabel: "License", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/modified", curie: "dcterms:modified", displayLabel: "Date Modified", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/publisher", curie: "dcterms:publisher", displayLabel: "Publisher", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/rights", curie: "dcterms:rights", displayLabel: "Rights", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/source", curie: "dcterms:source", displayLabel: "Source", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/subject", curie: "dcterms:subject", displayLabel: "Subject", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/title", curie: "dcterms:title", displayLabel: "Title", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/type", curie: "dcterms:type", displayLabel: "Type", vocabulary: "DC Terms" }, - { iri: "http://purl.org/dc/terms/abstract", curie: "dcterms:abstract", displayLabel: "Abstract", vocabulary: "DC Terms" }, + { iri: "http://purl.org/dc/terms/contributor", curie: "dcterms:contributor", displayLabel: "Contributor", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/created", curie: "dcterms:created", displayLabel: "Date Created", vocabulary: "DC Terms", cardinality: "single" }, + { iri: "http://purl.org/dc/terms/creator", curie: "dcterms:creator", displayLabel: "Creator", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/date", curie: "dcterms:date", displayLabel: "Date", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/description", curie: "dcterms:description", displayLabel: "Description", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/format", curie: "dcterms:format", displayLabel: "Format", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/identifier", curie: "dcterms:identifier", displayLabel: "Identifier", vocabulary: "DC Terms", cardinality: "single" }, + { iri: "http://purl.org/dc/terms/language", curie: "dcterms:language", displayLabel: "Language", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/license", curie: "dcterms:license", displayLabel: "License", vocabulary: "DC Terms", cardinality: "single" }, + { iri: "http://purl.org/dc/terms/modified", curie: "dcterms:modified", displayLabel: "Date Modified", vocabulary: "DC Terms", cardinality: "single" }, + { iri: "http://purl.org/dc/terms/publisher", curie: "dcterms:publisher", displayLabel: "Publisher", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/rights", curie: "dcterms:rights", displayLabel: "Rights", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/source", curie: "dcterms:source", displayLabel: "Source", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/subject", curie: "dcterms:subject", displayLabel: "Subject", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/title", curie: "dcterms:title", displayLabel: "Title", vocabulary: "DC Terms", cardinality: "single" }, + { iri: "http://purl.org/dc/terms/type", curie: "dcterms:type", displayLabel: "Type", vocabulary: "DC Terms", cardinality: "multiple" }, + { iri: "http://purl.org/dc/terms/abstract", curie: "dcterms:abstract", displayLabel: "Abstract", vocabulary: "DC Terms", cardinality: "multiple" }, // ── SKOS ── - { iri: "http://www.w3.org/2004/02/skos/core#prefLabel", curie: "skos:prefLabel", displayLabel: "Preferred Label", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#altLabel", curie: "skos:altLabel", displayLabel: "Synonym(s)", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#hiddenLabel", curie: "skos:hiddenLabel", displayLabel: "Hidden Label", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#definition", curie: "skos:definition", displayLabel: "Definition", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#example", curie: "skos:example", displayLabel: "Example(s)", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#scopeNote", curie: "skos:scopeNote", displayLabel: "Scope Note", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#editorialNote", curie: "skos:editorialNote", displayLabel: "Editorial Note", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#historyNote", curie: "skos:historyNote", displayLabel: "History Note", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#changeNote", curie: "skos:changeNote", displayLabel: "Change Note", vocabulary: "SKOS" }, - { iri: "http://www.w3.org/2004/02/skos/core#notation", curie: "skos:notation", displayLabel: "Notation", vocabulary: "SKOS" }, + { iri: "http://www.w3.org/2004/02/skos/core#prefLabel", curie: "skos:prefLabel", displayLabel: "Preferred Label", vocabulary: "SKOS", cardinality: "single-per-lang" }, + { iri: "http://www.w3.org/2004/02/skos/core#altLabel", curie: "skos:altLabel", displayLabel: "Synonym(s)", vocabulary: "SKOS", cardinality: "multiple" }, + { iri: "http://www.w3.org/2004/02/skos/core#hiddenLabel", curie: "skos:hiddenLabel", displayLabel: "Hidden Label", vocabulary: "SKOS", cardinality: "multiple" }, + { iri: "http://www.w3.org/2004/02/skos/core#definition", curie: "skos:definition", displayLabel: "Definition", vocabulary: "SKOS", cardinality: "single-per-lang" }, + { iri: "http://www.w3.org/2004/02/skos/core#example", curie: "skos:example", displayLabel: "Example(s)", vocabulary: "SKOS", cardinality: "multiple" }, + { iri: "http://www.w3.org/2004/02/skos/core#scopeNote", curie: "skos:scopeNote", displayLabel: "Scope Note", vocabulary: "SKOS", cardinality: "multiple" }, + { iri: "http://www.w3.org/2004/02/skos/core#editorialNote", curie: "skos:editorialNote", displayLabel: "Editorial Note", vocabulary: "SKOS", cardinality: "multiple" }, + { iri: "http://www.w3.org/2004/02/skos/core#historyNote", curie: "skos:historyNote", displayLabel: "History Note", vocabulary: "SKOS", cardinality: "multiple" }, + { iri: "http://www.w3.org/2004/02/skos/core#changeNote", curie: "skos:changeNote", displayLabel: "Change Note", vocabulary: "SKOS", cardinality: "multiple" }, + { iri: "http://www.w3.org/2004/02/skos/core#notation", curie: "skos:notation", displayLabel: "Notation", vocabulary: "SKOS", cardinality: "single" }, // ── RDFS / OWL ── - { iri: "http://www.w3.org/2000/01/rdf-schema#seeAlso", curie: "rdfs:seeAlso", displayLabel: "See Also", vocabulary: "RDFS" }, - { iri: "http://www.w3.org/2000/01/rdf-schema#isDefinedBy", curie: "rdfs:isDefinedBy", displayLabel: "Defined By", vocabulary: "RDFS" }, + { iri: "http://www.w3.org/2000/01/rdf-schema#seeAlso", curie: "rdfs:seeAlso", displayLabel: "See Also", vocabulary: "RDFS", cardinality: "multiple" }, + { iri: "http://www.w3.org/2000/01/rdf-schema#isDefinedBy", curie: "rdfs:isDefinedBy", displayLabel: "Defined By", vocabulary: "RDFS", cardinality: "multiple" }, ]; /** Well-known IRIs excluded from the general "Annotations" section (shown in their own sections) */ @@ -108,3 +116,20 @@ export function getAnnotationPropertiesByVocabulary(): Record = { + [LABEL_IRI]: "single-per-lang", + [COMMENT_IRI]: "multiple", +}; + +/** + * Returns the cardinality for an annotation property IRI. + * Falls back to "multiple" for unknown IRIs — the safe default that never + * artificially restricts user-defined annotation properties. + */ +export function getAnnotationCardinality(iri: string): AnnotationCardinality { + const found = ANNOTATION_PROPERTIES.find((p) => p.iri === iri); + if (found) return found.cardinality; + return EXTRA_CARDINALITIES[iri] ?? "multiple"; +} From 4c6b40a30413ab69b9eb1bad42199d9d72135beb Mon Sep 17 00:00:00 2001 From: R-Hart80 Date: Tue, 16 Jun 2026 14:34:36 -0300 Subject: [PATCH 2/3] fix(cardinality): allow custom locales when COMMON_LANGS exhausted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When all common languages (en, pt, es, fr, de, it) are already filled, the add-row logic now appends a blank-lang placeholder instead of returning early — letting users type locales like "ja" or "ko" that are not in the predefined list. Also normalises filledLangs comparison by trimming and lower-casing each lang tag, preventing missed matches from casing differences. Addresses CodeRabbit review comment on PR #273. Co-Authored-By: Claude Sonnet 4.6 --- __tests__/lib/ontology/annotationCardinality.test.ts | 6 ++++-- lib/ontology/annotationCardinality.ts | 12 ++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/__tests__/lib/ontology/annotationCardinality.test.ts b/__tests__/lib/ontology/annotationCardinality.test.ts index f3a404fd..fcdaadd5 100644 --- a/__tests__/lib/ontology/annotationCardinality.test.ts +++ b/__tests__/lib/ontology/annotationCardinality.test.ts @@ -52,7 +52,7 @@ describe('ensureTrailingPlaceholder — "single-per-lang"', () => { expect(ensureTrailingPlaceholder(input, "single-per-lang")).toStrictEqual(input); }); - it("adds no placeholder when all common languages are already filled", () => { + it("adds a blank-lang placeholder when all common languages are already filled", () => { const input = [ { value: "A", lang: "en" }, { value: "B", lang: "pt" }, @@ -61,7 +61,9 @@ describe('ensureTrailingPlaceholder — "single-per-lang"', () => { { value: "E", lang: "de" }, { value: "F", lang: "it" }, ]; - expect(ensureTrailingPlaceholder(input, "single-per-lang")).toStrictEqual(input); + const result = ensureTrailingPlaceholder(input, "single-per-lang"); + expect(result).toHaveLength(7); + expect(result[6]).toEqual({ value: "", lang: "" }); }); it("skips covered languages and finds the next available one", () => { diff --git a/lib/ontology/annotationCardinality.ts b/lib/ontology/annotationCardinality.ts index 403a9e87..24213920 100644 --- a/lib/ontology/annotationCardinality.ts +++ b/lib/ontology/annotationCardinality.ts @@ -31,9 +31,17 @@ export function ensureTrailingPlaceholder( // covered by a filled value. This prevents offering a duplicate @en row // when the annotation already has an English value (SKOS S14 / rdfs:label // convention). If all common languages are already present, no placeholder. - const filledLangs = new Set(values.filter((v) => v.value.trim()).map((v) => v.lang)); + const filledLangs = new Set( + values + .filter((v) => v.value.trim() !== "") + .map((v) => v.lang.trim().toLowerCase()) + .filter(Boolean), + ); const nextLang = COMMON_LANGS.find((l) => !filledLangs.has(l)); - if (nextLang === undefined) return values; + if (nextLang === undefined) { + // All common languages are filled — add a blank row for a custom locale. + return [...values, { value: "", lang: "" }]; + } return [...values, { value: "", lang: nextLang }]; } From 5830928888021fd0ddea1ca26183efbb4fb38563 Mon Sep 17 00:00:00 2001 From: "John R. D'Orazio" Date: Tue, 18 Aug 2026 23:39:28 +0200 Subject: [PATCH 3/3] fix(cardinality): filter the language picker instead of guessing a locale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks the single-per-lang strategy per maintainer review on #273. The previous approach walked a new hardcoded COMMON_LANGS list (en/pt/es/fr/de/it) to pick a default language for the placeholder row, then fell back to a blank tag once that list was exhausted. That added a fourth competing language list to the codebase and still let the user pick an already-used language from the unfiltered picker. Instead: - "single": unchanged — no placeholder once a value is filled. - "single-per-lang": still offers one trailing row, but with a blank language tag, and the picker on every row of that annotation now excludes the languages that already carry a value. A duplicate is unreachable rather than merely un-suggested. The very first row still defaults to `en` since there is nothing to collide with. - "multiple": unchanged — trailing @en row, picker unfiltered. COMMON_LANGS is gone; exclusions are computed from the annotation's own values via the new `usedLanguages` helper, and the picker filters `FREQUENT_LANGUAGES` / `ALL_LANGUAGES` from lib/i18n/languageCodes.ts. - LanguagePicker: new `excludeCodes` prop. Normalizes tags through findLanguageByCode, never hides the row's own current value, and blocks excluded tags from the "Use custom code" escape hatch too. - AnnotationRow: new `excludeLangs` prop, forwarded to the picker. - Wired through the label, definition and annotation rows of ClassDetailPanel, PropertyDetailPanel and IndividualDetailPanel. InlineAnnotationAdder needs no wiring — it only offers properties that have no values yet. Tests: rewrote the single-per-lang cases, added `usedLanguages` coverage (27 total), and added five LanguagePicker exclusion tests (21 total). Co-Authored-By: Claude Opus 5 (1M context) --- .../components/editor/LanguagePicker.test.tsx | 59 ++++++++++++++++ .../ontology/annotationCardinality.test.ts | 62 ++++++++++++----- components/editor/ClassDetailPanel.tsx | 5 +- components/editor/IndividualDetailPanel.tsx | 8 +-- components/editor/LanguagePicker.tsx | 44 ++++++++++-- components/editor/PropertyDetailPanel.tsx | 6 +- components/editor/standard/AnnotationRow.tsx | 7 ++ lib/ontology/annotationCardinality.ts | 69 ++++++++++--------- 8 files changed, 196 insertions(+), 64 deletions(-) diff --git a/__tests__/components/editor/LanguagePicker.test.tsx b/__tests__/components/editor/LanguagePicker.test.tsx index 01979279..bdef60e9 100644 --- a/__tests__/components/editor/LanguagePicker.test.tsx +++ b/__tests__/components/editor/LanguagePicker.test.tsx @@ -249,4 +249,63 @@ describe("LanguagePicker", () => { expect(screen.queryByText(/Use custom code/)).toBeNull(); }); + + it("hides languages listed in excludeCodes", async () => { + render(); + fireEvent.click(screen.getByLabelText("Language tag")); + + await waitFor(() => { + expect(screen.getByText("English")).toBeDefined(); + }); + + expect(screen.queryByText("French")).toBeNull(); + expect(screen.queryByText("German")).toBeNull(); + }); + + it("keeps the row's own language visible even when it is excluded", async () => { + render(); + fireEvent.click(screen.getByLabelText("Language tag")); + + await waitFor(() => { + expect(screen.getByText("French")).toBeDefined(); + }); + + expect(screen.queryByText("English")).toBeNull(); + }); + + it("matches excludeCodes case-insensitively", async () => { + render(); + fireEvent.click(screen.getByLabelText("Language tag")); + + await waitFor(() => { + expect(screen.getByText("English")).toBeDefined(); + }); + + expect(screen.queryByText("French")).toBeNull(); + }); + + it("does not offer an excluded code through the custom-code escape hatch", async () => { + const user = userEvent.setup(); + render(); + fireEvent.click(screen.getByLabelText("Language tag")); + + await waitFor(() => { + expect(screen.getByPlaceholderText("Search languages...")).toBeDefined(); + }); + + await user.type(screen.getByPlaceholderText("Search languages..."), "grc"); + + expect(screen.queryByText(/Use custom code/)).toBeNull(); + }); + + it("shows the full list when excludeCodes is omitted", async () => { + render(); + fireEvent.click(screen.getByLabelText("Language tag")); + + await waitFor(() => { + expect(screen.getByText("French")).toBeDefined(); + }); + + expect(screen.getByText("German")).toBeDefined(); + }); }); diff --git a/__tests__/lib/ontology/annotationCardinality.test.ts b/__tests__/lib/ontology/annotationCardinality.test.ts index fcdaadd5..aa1731ab 100644 --- a/__tests__/lib/ontology/annotationCardinality.test.ts +++ b/__tests__/lib/ontology/annotationCardinality.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { ensureTrailingPlaceholder } from "@/lib/ontology/annotationCardinality"; +import { ensureTrailingPlaceholder, usedLanguages } from "@/lib/ontology/annotationCardinality"; import { getAnnotationCardinality } from "@/lib/ontology/annotationProperties"; // ── ensureTrailingPlaceholder — "multiple" ──────────────────────────────── @@ -28,23 +28,23 @@ describe('ensureTrailingPlaceholder — "multiple"', () => { // ── ensureTrailingPlaceholder — "single-per-lang" ──────────────────────── describe('ensureTrailingPlaceholder — "single-per-lang"', () => { - it("adds an @en placeholder for an empty array", () => { - const result = ensureTrailingPlaceholder([], "single-per-lang"); - expect(result).toEqual([{ value: "", lang: "en" }]); + it("defaults the first row to @en when nothing is filled yet", () => { + expect(ensureTrailingPlaceholder([], "single-per-lang")).toEqual([{ value: "", lang: "en" }]); }); - it("does NOT add a second @en placeholder when @en is already filled (SKOS S14)", () => { - const input = [{ value: "Foo", lang: "en" }]; - const result = ensureTrailingPlaceholder(input, "single-per-lang"); - // The placeholder lang must differ from "en" + it("does NOT offer a second @en row when @en is already filled (SKOS S14)", () => { + const result = ensureTrailingPlaceholder([{ value: "Foo", lang: "en" }], "single-per-lang"); expect(result).toHaveLength(2); - expect(result[1].value).toBe(""); - expect(result[1].lang).not.toBe("en"); + expect(result[1]).toEqual({ value: "", lang: "" }); }); - it("adds a placeholder with the next uncovered language when @en is filled", () => { - const result = ensureTrailingPlaceholder([{ value: "Foo", lang: "en" }], "single-per-lang"); - expect(result[1].lang).toBe("pt"); + it("leaves the placeholder language blank so the filtered picker forces a choice", () => { + const result = ensureTrailingPlaceholder( + [{ value: "Foo", lang: "en" }, { value: "Bar", lang: "pt" }], + "single-per-lang", + ); + expect(result).toHaveLength(3); + expect(result[2]).toEqual({ value: "", lang: "" }); }); it("keeps an existing trailing empty placeholder as-is", () => { @@ -52,7 +52,7 @@ describe('ensureTrailingPlaceholder — "single-per-lang"', () => { expect(ensureTrailingPlaceholder(input, "single-per-lang")).toStrictEqual(input); }); - it("adds a blank-lang placeholder when all common languages are already filled", () => { + it("still offers a row when many languages are already filled", () => { const input = [ { value: "A", lang: "en" }, { value: "B", lang: "pt" }, @@ -65,12 +65,36 @@ describe('ensureTrailingPlaceholder — "single-per-lang"', () => { expect(result).toHaveLength(7); expect(result[6]).toEqual({ value: "", lang: "" }); }); +}); - it("skips covered languages and finds the next available one", () => { - const input = [{ value: "Foo", lang: "en" }, { value: "Bar", lang: "pt" }]; - const result = ensureTrailingPlaceholder(input, "single-per-lang"); - expect(result).toHaveLength(3); - expect(result[2].lang).toBe("es"); +// ── usedLanguages ───────────────────────────────────────────────────────── + +describe("usedLanguages", () => { + it("lists the languages already carrying a value for single-per-lang", () => { + const values = [ + { value: "Foo", lang: "en" }, + { value: "Bar", lang: "PT" }, + { value: "", lang: "es" }, + ]; + expect(usedLanguages(values, "single-per-lang")).toEqual(["en", "pt"]); + }); + + it("ignores rows whose value is only whitespace", () => { + expect(usedLanguages([{ value: " ", lang: "en" }], "single-per-lang")).toEqual([]); + }); + + it("drops empty language tags", () => { + const values = [{ value: "Foo", lang: "" }, { value: "Bar", lang: "de" }]; + expect(usedLanguages(values, "single-per-lang")).toEqual(["de"]); + }); + + it("excludes nothing for multiple — the picker stays unfiltered", () => { + const values = [{ value: "Foo", lang: "en" }, { value: "Bar", lang: "fr" }]; + expect(usedLanguages(values, "multiple")).toEqual([]); + }); + + it("excludes nothing for single", () => { + expect(usedLanguages([{ value: "Foo", lang: "en" }], "single")).toEqual([]); }); }); diff --git a/components/editor/ClassDetailPanel.tsx b/components/editor/ClassDetailPanel.tsx index 99fd895e..0409c387 100644 --- a/components/editor/ClassDetailPanel.tsx +++ b/components/editor/ClassDetailPanel.tsx @@ -38,7 +38,7 @@ import { AnnotationRow } from "@/components/editor/standard/AnnotationRow"; import { InlineAnnotationAdder } from "@/components/editor/standard/InlineAnnotationAdder"; import { RelationshipSection, type RelationshipGroup, type RelationshipTarget } from "@/components/editor/standard/RelationshipSection"; import { LABEL_IRI, COMMENT_IRI, DEFINITION_IRI, RELATIONSHIP_PROPERTY_IRIS, SEE_ALSO_IRI, getAnnotationPropertyInfo, getAnnotationCardinality } from "@/lib/ontology/annotationProperties"; -import { ensureTrailingPlaceholder } from "@/lib/ontology/annotationCardinality"; +import { ensureTrailingPlaceholder, usedLanguages } from "@/lib/ontology/annotationCardinality"; import { AutoSaveAffordanceBar } from "@/components/editor/AutoSaveAffordanceBar"; import { CrossReferencesPanel } from "@/components/editor/CrossReferencesPanel"; import { SimilarConceptsPanel } from "@/components/editor/SimilarConceptsPanel"; @@ -664,6 +664,7 @@ export function ClassDetailPanel({ className="flex-1 rounded-md border border-slate-300 bg-white px-2.5 py-1.5 text-sm focus:border-primary-500 focus:outline-hidden focus:ring-1 focus:ring-primary-500 dark:border-slate-600 dark:bg-slate-700 dark:text-white" /> { updateLabel(index, "lang", code); @@ -716,6 +717,7 @@ export function ClassDetailPanel({ updateAnnotationValue(DEFINITION_IRI, vIdx, "value", v)} @@ -827,6 +829,7 @@ export function ClassDetailPanel({ updateAnnotationValue(annotation.property_iri, vIdx, "value", v)} diff --git a/components/editor/IndividualDetailPanel.tsx b/components/editor/IndividualDetailPanel.tsx index d94604d6..77a44c2b 100644 --- a/components/editor/IndividualDetailPanel.tsx +++ b/components/editor/IndividualDetailPanel.tsx @@ -26,7 +26,7 @@ import { InlineAnnotationAdder } from "@/components/editor/standard/InlineAnnota import { RelationshipSection, type RelationshipGroup, type RelationshipTarget } from "@/components/editor/standard/RelationshipSection"; import { PropertyAssertionSection } from "@/components/editor/standard/PropertyAssertionSection"; import { LABEL_IRI, COMMENT_IRI, DEFINITION_IRI, SEE_ALSO_IRI, getAnnotationPropertyInfo, getAnnotationCardinality } from "@/lib/ontology/annotationProperties"; -import { ensureTrailingPlaceholder } from "@/lib/ontology/annotationCardinality"; +import { ensureTrailingPlaceholder, usedLanguages } from "@/lib/ontology/annotationCardinality"; import { AutoSaveAffordanceBar } from "@/components/editor/AutoSaveAffordanceBar"; import { useEntityAutoSave } from "@/lib/hooks/useEntityAutoSave"; import { useToast } from "@/lib/context/ToastContext"; @@ -454,7 +454,7 @@ export function IndividualDetailPanel({ {editLabels.map((label, index) => (
updateLabel(index, "value", e.target.value)} onBlur={() => triggerSave()} placeholder="Label text" className="flex-1 rounded-md border border-slate-300 bg-white px-2.5 py-1.5 text-sm focus:border-primary-500 focus:outline-hidden focus:ring-1 focus:ring-primary-500 dark:border-slate-600 dark:bg-slate-700 dark:text-white" /> - { updateLabel(index, "lang", code); triggerSave(); }} /> + { updateLabel(index, "lang", code); triggerSave(); }} /> {editLabels.length > 1 ? ( ) :
} @@ -479,7 +479,7 @@ export function IndividualDetailPanel({
}>
{editDefinitions.map((def, index) => ( - updateDefinition(index, "value", v)} onLangChange={(l) => updateDefinition(index, "lang", l)} onRemove={editDefinitions.filter((d) => d.value.trim()).length > 0 && index < editDefinitions.length - 1 ? () => removeDefinition(index) : undefined} onBlur={() => triggerSave()} showPropertyLabel={false} placeholder="Add a definition..." /> @@ -674,7 +674,7 @@ export function IndividualDetailPanel({ {editAnnotations.map((ann) => (
{ann.values.map((v, vi) => ( - updateAnnotationValue(ann.property_iri, vi, "value", val)} onLangChange={(lang) => updateAnnotationValue(ann.property_iri, vi, "lang", lang)} onRemove={ann.values.filter((x) => x.value.trim()).length > 0 && vi < ann.values.length - 1 ? () => removeAnnotationValue(ann.property_iri, vi) : undefined} diff --git a/components/editor/LanguagePicker.tsx b/components/editor/LanguagePicker.tsx index e061f960..01898696 100644 --- a/components/editor/LanguagePicker.tsx +++ b/components/editor/LanguagePicker.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, useRef, useEffect, useCallback } from "react"; +import { useState, useRef, useEffect, useCallback, useMemo } from "react"; import { Command } from "cmdk"; import { ChevronDown } from "lucide-react"; import { langToFlag } from "@/lib/utils"; @@ -15,6 +15,13 @@ interface LanguagePickerProps { value: string; onChange: (code: string) => void; disabled?: boolean; + /** + * BCP 47 codes to hide from the list — used by `single-per-lang` annotations + * so a language that already has a value can't be picked a second time. + * The row's own `value` is never excluded, so the current selection stays + * visible and re-selectable. + */ + excludeCodes?: readonly string[]; } /** Set of codes that appear in the "Frequently used" group */ @@ -46,7 +53,7 @@ const GROUP_HEADING_CLASS = * language, a "Use custom code" option appears so users can enter arbitrary * BCP 47 tags (e.g. `grc`, `cu`, `sga`). */ -export function LanguagePicker({ value, onChange, disabled }: LanguagePickerProps) { +export function LanguagePicker({ value, onChange, disabled, excludeCodes }: LanguagePickerProps) { const [open, setOpen] = useState(false); const [search, setSearch] = useState(""); const containerRef = useRef(null); @@ -96,6 +103,33 @@ export function LanguagePicker({ value, onChange, disabled }: LanguagePickerProp const canonicalCode = langInfo?.code ?? value; const displayLabel = canonicalCode || "lang"; + // Codes hidden from the list, normalized. The current selection is always + // kept so the row can still show (and re-pick) its own language. + const excludedCodes = useMemo(() => { + if (!excludeCodes?.length) return null; + const set = new Set( + excludeCodes + .map((c) => (findLanguageByCode(c)?.code ?? c).trim().toLowerCase()) + .filter(Boolean), + ); + set.delete(canonicalCode.trim().toLowerCase()); + return set.size > 0 ? set : null; + }, [excludeCodes, canonicalCode]); + + const isExcluded = useCallback( + (code: string) => !!excludedCodes?.has(code.trim().toLowerCase()), + [excludedCodes], + ); + + const frequentLanguages = useMemo( + () => (excludedCodes ? FREQUENT_LANGUAGES.filter((l) => !excludedCodes.has(l.code.toLowerCase())) : FREQUENT_LANGUAGES), + [excludedCodes], + ); + const otherLanguages = useMemo( + () => (excludedCodes ? OTHER_LANGUAGES.filter((l) => !excludedCodes.has(l.code.toLowerCase())) : OTHER_LANGUAGES), + [excludedCodes], + ); + const handleSelect = (code: string) => { onChange(code); setSearch(""); @@ -106,7 +140,7 @@ export function LanguagePicker({ value, onChange, disabled }: LanguagePickerProp // Show "Use custom code" when search text is non-empty and doesn't exactly match a known code const trimmedSearch = search.trim(); const showCustomOption = - trimmedSearch.length > 0 && !findLanguageByCode(trimmedSearch); + trimmedSearch.length > 0 && !findLanguageByCode(trimmedSearch) && !isExcluded(trimmedSearch); return (
@@ -174,7 +208,7 @@ export function LanguagePicker({ value, onChange, disabled }: LanguagePickerProp )} - {FREQUENT_LANGUAGES.map((lang) => ( + {frequentLanguages.map((lang) => ( - {OTHER_LANGUAGES.map((lang) => ( + {otherLanguages.map((lang) => ( (
updateLabel(index, "value", e.target.value)} onBlur={() => triggerSave()} placeholder="Label text" className="flex-1 rounded-md border border-slate-300 bg-white px-2.5 py-1.5 text-sm focus:border-primary-500 focus:outline-hidden focus:ring-1 focus:ring-primary-500 dark:border-slate-600 dark:bg-slate-700 dark:text-white" /> - { updateLabel(index, "lang", code); triggerSave(); }} /> + { updateLabel(index, "lang", code); triggerSave(); }} /> {editLabels.length > 1 ? ( ) : ( @@ -524,6 +524,7 @@ export function PropertyDetailPanel({ updateDefinition(index, "value", v)} @@ -713,6 +714,7 @@ export function PropertyDetailPanel({ updateAnnotationValue(ann.property_iri, vi, "value", val)} diff --git a/components/editor/standard/AnnotationRow.tsx b/components/editor/standard/AnnotationRow.tsx index 415e906a..fceeb4ad 100644 --- a/components/editor/standard/AnnotationRow.tsx +++ b/components/editor/standard/AnnotationRow.tsx @@ -17,6 +17,11 @@ interface AnnotationRowProps { showPropertyLabel?: boolean; /** Custom placeholder for the value input (defaults to "Value") */ placeholder?: string; + /** + * Language tags to hide from this row's picker. Set for `single-per-lang` + * annotations so a language that already has a value can't be picked twice. + */ + excludeLangs?: readonly string[]; } export function AnnotationRow({ @@ -29,6 +34,7 @@ export function AnnotationRow({ onBlur, showPropertyLabel = true, placeholder = "Value", + excludeLangs, }: AnnotationRowProps) { const { displayLabel, curie } = getAnnotationPropertyInfo(propertyIri); const isLongValue = value.length > 80; @@ -65,6 +71,7 @@ export function AnnotationRow({ /> )} { onLangChange(code); diff --git a/lib/ontology/annotationCardinality.ts b/lib/ontology/annotationCardinality.ts index 24213920..c8e0d750 100644 --- a/lib/ontology/annotationCardinality.ts +++ b/lib/ontology/annotationCardinality.ts @@ -1,56 +1,59 @@ import type { LocalizedString } from "@/lib/api/client"; import type { AnnotationCardinality } from "./annotationProperties"; -/** Language tags to try in order when choosing a default lang for a new placeholder row. */ -const COMMON_LANGS = ["en", "pt", "es", "fr", "de", "it"]; - /** * Ensures that an array of localized strings has an appropriate trailing * placeholder row, respecting the annotation property's cardinality: * - * - "multiple": always one trailing empty row (original behaviour). - * - "single-per-lang": placeholder lang is the first common language not yet - * covered by a filled value; no placeholder when all common langs are filled. - * - "single": no placeholder once any value is filled; one empty row when empty. + * - `"single"`: no placeholder once a value is filled; one empty row when empty. + * - `"single-per-lang"`: one trailing empty row, but with no preset language — + * the user picks from a list that excludes the languages already present + * (see {@link usedLanguages} and `LanguagePicker`'s `excludeCodes`). Only when + * nothing is filled yet does the row default to `en`, since there is nothing + * to collide with. + * - `"multiple"`: always one trailing empty row defaulting to `en`. */ export function ensureTrailingPlaceholder( values: LocalizedString[], cardinality: AnnotationCardinality, ): LocalizedString[] { + const hasFilled = values.some((v) => v.value.trim() !== ""); + switch (cardinality) { - case "single": { - const hasFilled = values.some((v) => v.value.trim() !== ""); - if (hasFilled) return values.filter((v) => v.value.trim() !== ""); - return [{ value: "", lang: "en" }]; - } + case "single": + // At most one value: drop stray empty rows once something is filled. + return hasFilled ? values.filter((v) => v.value.trim() !== "") : [{ value: "", lang: "en" }]; - case "single-per-lang": { - // Keep any existing trailing empty placeholder as-is. + case "single-per-lang": if (values.length > 0 && values[values.length - 1].value.trim() === "") return values; - // Append a placeholder whose lang is the first common language not yet - // covered by a filled value. This prevents offering a duplicate @en row - // when the annotation already has an English value (SKOS S14 / rdfs:label - // convention). If all common languages are already present, no placeholder. - const filledLangs = new Set( - values - .filter((v) => v.value.trim() !== "") - .map((v) => v.lang.trim().toLowerCase()) - .filter(Boolean), - ); - const nextLang = COMMON_LANGS.find((l) => !filledLangs.has(l)); - if (nextLang === undefined) { - // All common languages are filled — add a blank row for a custom locale. - return [...values, { value: "", lang: "" }]; - } - return [...values, { value: "", lang: nextLang }]; - } + // Blank lang forces an explicit pick from the filtered picker, so the + // user can't silently re-use a language that already has a value. + return [...values, { value: "", lang: hasFilled ? "" : "en" }]; case "multiple": - default: { + default: if (values.length === 0 || values[values.length - 1].value.trim() !== "") { return [...values, { value: "", lang: "en" }]; } return values; - } } } + +/** + * Language tags already carrying a value in `values`, normalized to lowercase. + * + * Passed to `LanguagePicker`'s `excludeCodes` for `single-per-lang` annotations + * so the picker can't offer a language that would violate the cardinality + * constraint. Returns an empty array for other cardinalities — `multiple` + * annotations show the unfiltered language list. + */ +export function usedLanguages( + values: LocalizedString[], + cardinality: AnnotationCardinality, +): string[] { + if (cardinality !== "single-per-lang") return []; + return values + .filter((v) => v.value.trim() !== "") + .map((v) => v.lang.trim().toLowerCase()) + .filter(Boolean); +}