Skip to content

Commit

Permalink
Reverts changes around items and itemset and adds support for items i…
Browse files Browse the repository at this point in the history
…n RankControl
  • Loading branch information
latin-panda committed Jan 23, 2025
1 parent 9133cff commit 6536500
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 109 deletions.
6 changes: 4 additions & 2 deletions packages/web-forms/src/components/controls/RankControl.vue
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ const swapItems = (index: number, newPosition: number) => {
:disabled="question.currentState.readonly"
ghost-class="fade-moving"
class="rank-control"
@update="setValues">
@update="setValues"
>
<div
v-for="(option, index) in options"
:id="option.value"
Expand All @@ -101,7 +102,8 @@ const swapItems = (index: number, newPosition: number) => {
:class="{ 'moving': highlight.index.value === index }"
tabindex="0"
@keydown.up.prevent="moveUp(index)"
@keydown.down.prevent="moveDown(index)">
@keydown.down.prevent="moveDown(index)"
>
<div class="rank-label">
<svg xmlns="http://www.w3.org/2000/svg" width="25" height="25" viewBox="0 0 768 768">
<path d="M480 511.5q25.5 0 45 19.5t19.5 45-19.5 45-45 19.5-45-19.5-19.5-45 19.5-45 45-19.5zM480 319.5q25.5 0 45 19.5t19.5 45-19.5 45-45 19.5-45-19.5-19.5-45 19.5-45 45-19.5zM480 256.5q-25.5 0-45-19.5t-19.5-45 19.5-45 45-19.5 45 19.5 19.5 45-19.5 45-45 19.5zM288 127.5q25.5 0 45 19.5t19.5 45-19.5 45-45 19.5-45-19.5-19.5-45 19.5-45 45-19.5zM288 319.5q25.5 0 45 19.5t19.5 45-19.5 45-45 19.5-45-19.5-19.5-45 19.5-45 45-19.5zM352.5 576q0 25.5-19.5 45t-45 19.5-45-19.5-19.5-45 19.5-45 45-19.5 45 19.5 19.5 45z" />
Expand Down
2 changes: 1 addition & 1 deletion packages/xforms-engine/src/client/hierarchy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export type AnyControlNode =
| AnyInputNode
| AnyNoteNode
| AnyRangeNode
| SelectNode
| RankNode
| SelectNode
| TriggerNode;

// prettier-ignore
Expand Down
4 changes: 2 additions & 2 deletions packages/xforms-engine/src/instance/RankControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import type {
} from '../client/RankNode.ts';
import type { TextRange } from '../client/TextRange.ts';
import type { XFormsXPathElement } from '../integration/xpath/adapter/XFormsXPathNode.ts';
import { createItemset } from '../lib/reactivity/createBaseItemset.ts';
import type { CurrentState } from '../lib/reactivity/node-state/createCurrentState.ts';
import type { EngineState } from '../lib/reactivity/node-state/createEngineState.ts';
import type { SharedNodeState } from '../lib/reactivity/node-state/createSharedNodeState.ts';
Expand All @@ -28,6 +27,7 @@ import { RankFunctionalityError, RankValueTypeError } from '../error/RankError.t
import { BaseItemCollectionCodec } from '../lib/codecs/BaseItemCollectionCodec.ts';
import { sharedValueCodecs } from '../lib/codecs/getSharedValueCodec.ts';
import type { AnyNodeDefinition } from '../parse/model/NodeDefinition.ts';
import { createItemCollection } from '../lib/reactivity/createItemCollection.ts';

type AssertRangeNodeDefinition = (definition: RankDefinition) => asserts definition is RankDefinition<'string'>;
const assertRangeNodeDefinition: AssertRangeNodeDefinition = (definition) => {
Expand Down Expand Up @@ -77,7 +77,7 @@ export class RankControl
const codec = new BaseItemCollectionCodec(sharedValueCodecs.string);
super(parent, definition, codec);

const valueOptions = createItemset(this, definition.bodyElement.itemset);
const valueOptions = createItemCollection(this);
const mapOptionsByValue: Accessor<RankItemMap> = this.scope.runTask(() => {
return createMemo(() => {
return new Map(valueOptions().map((item) => [item.value, item]));
Expand Down
4 changes: 2 additions & 2 deletions packages/xforms-engine/src/instance/SelectControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { ValueType } from '../client/ValueType.ts';
import { SelectValueTypeError } from '../error/SelectValueTypeError.ts';
import type { XFormsXPathElement } from '../integration/xpath/adapter/XFormsXPathNode.ts';
import { getSelectCodec } from '../lib/codecs/select/getSelectCodec.ts';
import { createSelectItems } from '../lib/reactivity/createSelectItems.ts';
import { createItemCollection } from '../lib/reactivity/createItemCollection.ts';
import type { CurrentState } from '../lib/reactivity/node-state/createCurrentState.ts';
import type { EngineState } from '../lib/reactivity/node-state/createEngineState.ts';
import type { SharedNodeState } from '../lib/reactivity/node-state/createSharedNodeState.ts';
Expand Down Expand Up @@ -93,7 +93,7 @@ export class SelectControl
this.appearances = definition.bodyElement.appearances;
this.selectType = definition.bodyElement.type;

const valueOptions = createSelectItems(this);
const valueOptions = createItemCollection(this);

const mapOptionsByValue: Accessor<SelectItemMap> = this.scope.runTask(() => {
return createMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,71 @@ import { UpsertableMap } from '@getodk/common/lib/collections/UpsertableMap.ts';
import type { Accessor } from 'solid-js';
import { createMemo } from 'solid-js';
import type { ActiveLanguage } from '../../client/FormLanguage.ts';
import type { SelectItem } from '../../client/SelectNode.ts';
import type { RankItem } from '../../client/RankNode.ts';
import type { TextRange as ClientTextRange } from '../../client/TextRange.ts';
import type { EvaluationContext } from '../../instance/internal-api/EvaluationContext.ts';
import type { TranslationContext } from '../../instance/internal-api/TranslationContext.ts';
import type { SelectControl } from '../../instance/SelectControl.ts';
import type { RankControl } from '../../instance/RankControl.ts';
import { TextChunk } from '../../instance/text/TextChunk.ts';
import { TextRange } from '../../instance/text/TextRange.ts';
import type { EngineXPathNode } from '../../integration/xpath/adapter/kind.ts';
import type { EngineXPathEvaluator } from '../../integration/xpath/EngineXPathEvaluator.ts';
import type { ItemDefinition } from '../../parse/body/control/ItemDefinition.ts';
import type { ItemsetDefinition } from '../../parse/body/control/ItemsetDefinition.ts';
import { createComputedExpression } from './createComputedExpression.ts';
import type { ReactiveScope } from './scope.ts';
import { createTextRange } from './text/createTextRange.ts';
import type { RankControl } from '../../instance/RankControl.ts';
import type { SelectControl } from '../../instance/SelectControl.ts';
import type { TranslationContext } from '../../instance/internal-api/TranslationContext.ts';
import { TextChunk } from '../../instance/text/TextChunk.ts';
import { TextRange } from '../../instance/text/TextRange.ts';

type ItemsetControl = SelectControl | RankControl;
type ItemCollectionControl = SelectControl | RankControl;

Check warning on line 22 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Union type ItemCollectionControl constituents must be sorted

Check warning on line 22 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Union type ItemCollectionControl constituents must be sorted
type ItemType = SelectItem | RankItem

Check warning on line 23 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Union type ItemType constituents must be sorted

Check warning on line 23 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Union type ItemType constituents must be sorted
type DerivedItemLabel = ClientTextRange<'item-label', 'form-derived'>;

const derivedItemLabel = (context: TranslationContext, value: string): DerivedItemLabel => {
const chunk = new TextChunk(context, 'literal', value);

return new TextRange('form-derived', 'item-label', [chunk]);
};

const createItemLabel = (
context: EvaluationContext,
definition: ItemDefinition
): Accessor<ClientTextRange<'item-label'>> => {
const { label, value } = definition;

if (label == null) {
return () => derivedItemLabel(context, value);
}

return createTextRange(context, 'item-label', label);
};

interface SourceValueItem {
readonly value: string;
readonly label: ClientTextRange<'item-label'>;
}

const createTranslatedStaticItems = (
control: ItemCollectionControl,
items: readonly ItemDefinition[]
): Accessor<readonly SourceValueItem[]> => {
return control.scope.runTask(() => {
const labeledItems = items.map((item) => {
const { value } = item;
const label = createItemLabel(control, item);

return () => ({
value,
label: label(),
});
});

return createMemo(() => {
return labeledItems.map((item) => item());
});
});
};

class ItemsetItemEvaluationContext implements EvaluationContext {
readonly isAttached: Accessor<boolean>;
Expand All @@ -25,7 +75,7 @@ class ItemsetItemEvaluationContext implements EvaluationContext {
readonly contextReference: Accessor<string>;
readonly getActiveLanguage: Accessor<ActiveLanguage>;

constructor(control: ItemsetControl, readonly contextNode: EngineXPathNode) {
constructor(control: ItemCollectionControl, readonly contextNode: EngineXPathNode) {
this.isAttached = control.isAttached;
this.scope = control.scope;
this.evaluator = control.evaluator;
Expand All @@ -34,14 +84,6 @@ class ItemsetItemEvaluationContext implements EvaluationContext {
}
}

type DerivedItemLabel = ClientTextRange<'item-label', 'form-derived'>;

export const derivedItemLabel = (context: TranslationContext, value: string): DerivedItemLabel => {
const chunk = new TextChunk(context, 'literal', value);

return new TextRange('form-derived', 'item-label', [chunk]);
};

const createItemsetItemLabel = (
context: EvaluationContext,
definition: ItemsetDefinition,
Expand All @@ -50,7 +92,9 @@ const createItemsetItemLabel = (
const { label } = definition;

if (label == null) {
return createMemo(() => derivedItemLabel(context, itemValue()));
return createMemo(() => {
return derivedItemLabel(context, itemValue());
});
}

return createTextRange(context, 'item-label', label);
Expand All @@ -61,33 +105,38 @@ interface ItemsetItem {
value(): string;
}

const createItemsetItems = (control: ItemsetControl, itemset: ItemsetDefinition): Accessor<readonly ItemsetItem[]> => {
const createItemsetItems = (
control: ItemCollectionControl,
itemset: ItemsetDefinition
): Accessor<readonly ItemsetItem[]> => {
return control.scope.runTask(() => {
const itemNodes = createComputedExpression(control, itemset.nodes, { defaultValue: [] });
const itemNodes = createComputedExpression(control, itemset.nodes, {
defaultValue: [],
});
const itemsCache = new UpsertableMap<EngineXPathNode, ItemsetItem>();

return createMemo(() => {
return itemNodes().map((itemNode) => {
return itemsCache.upsert(itemNode, () => {
const context = new ItemsetItemEvaluationContext(control, itemNode);
const value = createComputedExpression(context, itemset.value, { defaultValue: '' });
const value = createComputedExpression(context, itemset.value, {
defaultValue: '',
});
const label = createItemsetItemLabel(context, itemset, value);

return { label, value };
return {
label,
value,
};
});
});
});
});
};

export interface SourceValueItem {
readonly value: string;
readonly label: ClientTextRange<'item-label'>;
}

export const createItemset = (
control: ItemsetControl,
itemset: ItemsetDefinition,
const createItemset = (
control: ItemCollectionControl,
itemset: ItemsetDefinition
): Accessor<readonly SourceValueItem[]> => {
return control.scope.runTask(() => {
const itemsetItems = createItemsetItems(control, itemset);
Expand All @@ -102,3 +151,25 @@ export const createItemset = (
});
});
};

/**
* Creates a reactive computation of a {@link ItemCollectionControl}'s
* {@link ItemType}s, in support of the field's `valueOptions`.
*
* - The control defined with static `<item>`s will compute to an corresponding
* static list of items.
* - The control defined with a computed `<itemset>` will compute to a reactive list
* of items.
* - Items of both will produce {@link ItemType.label | labels} reactive to
* their appropriate dependencies (whether relative to the itemset item node,
* referencing a form's `itext` translations, etc).
*/
export const createItemCollection = (control: ItemCollectionControl): Accessor<readonly ItemType[]> => {
const { items, itemset } = control.definition.bodyElement;

Check failure on line 168 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Unsafe assignment of an error typed value

Check failure on line 168 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Unsafe member access .bodyElement on an `error` typed value

Check failure on line 168 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Unsafe assignment of an error typed value

Check failure on line 168 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Unsafe member access .bodyElement on an `error` typed value

if (itemset != null) {
return createItemset(control, itemset);

Check failure on line 171 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Unsafe argument of type error typed assigned to a parameter of type `ItemsetDefinition`

Check failure on line 171 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Unsafe argument of type error typed assigned to a parameter of type `ItemsetDefinition`
}

return createTranslatedStaticItems(control, items);

Check failure on line 174 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Unsafe argument of type error typed assigned to a parameter of type `readonly ItemDefinition[]`

Check failure on line 174 in packages/xforms-engine/src/lib/reactivity/createItemCollection.ts

View workflow job for this annotation

GitHub Actions / Lint (global) (22.12.0)

Unsafe argument of type error typed assigned to a parameter of type `readonly ItemDefinition[]`
};
67 changes: 0 additions & 67 deletions packages/xforms-engine/src/lib/reactivity/createSelectItems.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { LocalNamedElement } from '@getodk/common/types/dom.ts';
import { getItemsetElement } from '../../../lib/dom/query.ts';
import { getItemElements, getItemsetElement } from '../../../lib/dom/query.ts';
import type { XFormDefinition } from '../../XFormDefinition.ts';
import type { BodyElementParentContext } from '../BodyDefinition.ts';
import { ControlDefinition } from './ControlDefinition.ts';
import { ItemsetDefinition } from './ItemsetDefinition.ts';
import { ItemDefinition } from './ItemDefinition.ts';

export type RankType = 'rank';
export interface RankElement extends LocalNamedElement<RankType> {}
Expand All @@ -17,9 +18,10 @@ export class RankControlDefinition extends ControlDefinition<RankType> {
return RankControlDefinition.isRankElement(element);
}

readonly type: RankType = 'rank';
readonly type: RankType;
readonly element: RankElement;
readonly itemset: ItemsetDefinition;
readonly itemset: ItemsetDefinition | null;
readonly items: readonly ItemDefinition[];

constructor(form: XFormDefinition, parent: BodyElementParentContext, element: Element) {
if (!RankControlDefinition.isRankElement(element)) {
Expand All @@ -30,7 +32,20 @@ export class RankControlDefinition extends ControlDefinition<RankType> {

this.type = element.localName as RankType;
this.element = element;
this.itemset = new ItemsetDefinition(form, this, getItemsetElement(element));
const itemsetElement = getItemsetElement(element);
const itemElements = getItemElements(element);

if (itemsetElement === null) {
this.itemset = null;
this.items = itemElements.map((itemElement) => new ItemDefinition(form, this, itemElement));
} else {
if (itemElements.length > 0) {
throw new Error(`<${element.nodeName}> has both <itemset> and <item> children`);
}

this.items = [];
this.itemset = new ItemsetDefinition(form, this, itemsetElement);
}
}

override toJSON() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@ import {
import { describe, expect, it } from 'vitest';
import { initializeForm } from '../../../src/instance/index.ts';
import type { SelectControl } from '../../../src/instance/SelectControl.ts';
import type { createSelectItems } from '../../../src/lib/reactivity/createSelectItems.ts';
import type { createItemCollection } from '../../../src/lib/reactivity/createItemCollection.ts';
import { reactiveTestScope } from '../../helpers/reactive/internal.ts';

/**
* @todo Consider these alternative testing strategies:
*
* - Reducing tests of reactive internals like {@link createSelectItems} to more
* - Reducing tests of reactive internals like {@link createItemCollection} to more
* conventional unit tests: If there's a reasonable way to do that, it would
* probably begin (especially in this case) with relaxing the
* {@link createSelectItems} signature to accept something more minimal than a
* {@link createItemCollection} signature to accept something more minimal than a
* {@link SelectControl}. However, after some reflection on the efforts to port
* JavaRosa tests, there's quite a lot of value in form-level integration
* tests. We might benefit instead from...
Expand Down

0 comments on commit 6536500

Please sign in to comment.