Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Engine API: expand and improve select write semantics (also anticipating future types) #97

Open
eyelidlessness opened this issue Apr 30, 2024 · 2 comments

Comments

@eyelidlessness
Copy link
Member

While the current SelectNode.select and SelectNode.deselect methods are sufficient, their ergonomics leave something to be desired for a variety of use cases that are now becoming reality (making them easier to discuss than when the interface was initially designed).

  • As discussed in feature #59: select controls for ui-vue #89, it would be helpful to have APIs to toggle individual SelectItems (allowing clients to toggle their selected state without redundantly checking their selected-ness, simplifying a per-checkbox toggle action). A proposed name of toggle was introduced. We may also consider prior art in APIs with toggle semantics, some of which include a "force" option (although we should consider it carefully, as that particular example tends to confuse).
  • Also discussed in feature #59: select controls for ui-vue #89, it would be helpful to atomically set the full selected state of multiple SelectItems at once. While a proposed name of setValue was suggested, I'd caution that it may be wise to reserve that name for other use cases (see next point).
  • A consistent need for various flexibilities around writing values has been an overarching concern while porting JavaRosa tests (Port JavaRosa tests, ignore ones that fail #25). This includes some flexibility to specify only the selected values (rather than the SelectItem object interface introduced in Refactor: engine <-> client interface implementation #67), as well as a much broader concern around the Scenario client's utilization of a highly polymorphic value assignment interface. That porting effort will include copious notes, including many on the topic of casting of value types. While I have some concerns about making the engine's APIs overly flexible, in general I think it would be a good idea for select-specific APIs to be mindful of the possibility that a shared setValue interface (across all value-bearing node types) might come to be.
  • Resurfacing thoughts from discussion prior to feature #59: select controls for ui-vue #89 (also Refactor: engine <-> client interface implementation #67 if I recall), I cautioned that a variadic method could be confusing without clearly distinguishing between partial and full value change semantics. I think something akin to the notion proposed in feature #59: select controls for ui-vue #89 to set all selected values is definitely easier to reason about than any partial-but-multiple selection change logic could be.
@lognaturel
Copy link
Member

a shared setValue interface (across all value-bearing node types) might come to be

Sounds similar to https://github.com/getodk/javarosa/blob/3f579c43f81049475e032948ba16f388e19620fd/src/main/java/org/javarosa/core/model/data/IAnswerData.java#L128 ?

There's also a setvalue action: https://getodk.github.io/xforms-spec/#action:setvalue which thankfully does about what you'd expect.

@eyelidlessness
Copy link
Member Author

a shared setValue interface (across all value-bearing node types) might come to be

Sounds similar to https://github.com/getodk/javarosa/blob/3f579c43f81049475e032948ba16f388e19620fd/src/main/java/org/javarosa/core/model/data/IAnswerData.java#L128 ?

Yep that's pretty much exactly what got me thinking of a common, non-node-type-specific API.

There's also a setvalue action: https://getodk.github.io/xforms-spec/#action:setvalue which thankfully does about what you'd expect.

Right. I don't know if the hypothetical polymorophic ValueNode.setValue would be called in our implementation of actions, but it sure seems like something to also consider.


FWIW, there's already a ValueContext interface (introduced in #67) which addresses encoding/decoding between a node's own runtime value type representation, and the XForms string serialization of it. So for instance, SelectNode has a value type (slightly simplified for discussion) which is an array of:

interface SelectItem {
  label: TextRange;
  value: string;
}

As a ValueContext implementation, SelectNode implements:

interface ValueContext {
  // ...
  encodeValue(runtimeValue: SelectItem[]): string;
  decodeValue(instanceValue: string): SelectItem[];
}

It's reasonable to consider expanding that interface (or having another one like it but for more ambiguous/polymorphic input types) to support a case like IAnswerData. However, these are internal methods, and I'd prefer to keep it that way, putting a good amount of thought into what would be appropriate for a client-facing interface. I'm particularly cautious about pushing too much XForms-y logic or semantics out to clients, especially in any way which creates "more than one way to do it" and has potential to fracture the happy path(s) for writes.

I do think the current ValueContext interface is probably sufficient to support setvalue (and setgeopoint, or any other hypothetical action which would be value-producing). Unless I'm mistaken, any values produced by the action should also be serializable to an instance value string. It would make sense to me that that common serialization is a good starting point for interaction between these various aspects of value wrangling.

And possibly a good first step would be to loosen the ValueContext interface to accept multiple variants of a given runtime value. So for instance, SelectNode might accept:

  • SelectItem[]
  • string[] (where string corresponds to SelectItem.value)
  • string (where the value is optimistically encoded)

It also seems pretty likely, given other things I've learned working on #25, that SelectNode itself may become polymorphic, e.g.

interface SelectItem<T = string> {
  label: TextRange;
  value: T;
}

... with that cascading out to ValueContext, potentially accepting:

  • SelectItem<T>[]
  • T[]
  • string[]
  • string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants