Page MenuHomePhabricator

Field: explore implementation options
Closed, ResolvedPublic

Description

This task covers determining how we will implement a Field component in Codex. There are several implementation routes we could take and we should evaluate them carefully in terms of end user experience, developer experience, and library maintenance.


Background

The Field component is still being designed (see T330419), but it will likely contain some or all of these features:

  • A semantic label, which is connected to the input via an aria attribute for accessibility support
  • "Optional" indicator (fields will be required by default, but can be marked as optional)
  • A tooltip or popup next to the label
  • A description below the label
  • An input (e.g. TextInput), control (e.g. ToggleSwitch), or input group (Radio group)
  • Helper text below the input
  • A character counter for some inputs (e.g. TextArea)
  • An inline validation message
  • Field actions (e.g. a button to delete the field)

Here is a work-in-progress design exploration:

image.png (166×966 px, 24 KB)


Implementation options

There are 2 main paths we're consider for implementation, detailed below. For each, I've created a proof of concept that implements a few of the Field features (semantic label for both inputs and input groups, helper text, inline validation and error styles).

Option 1: Standalone Field component used alongside input/control components

Option 1 is a Field component that developer users would use to wrap an input or control component (or group). It would contain a slot for the input/control. The dev user would pass field props into the Field component, but continue to pass input props into the input component.

Pros:

  • Field features are documented in a single place (the Field demo page)
  • Input groups can be handled internally; no need for new RadioGroup and CheckboxGroup components
  • Encourages composition for maximum flexibility. Inputs/controls can be used with or without the Field component, so simple inputs could be composed into more complex ones. Label is a separate component that can be used on its own without all the other Field stuff.
  • We're providing simpler building blocks that will allow people to compose their own solutions
  • Potentially easier to implement future Field features (since they're all contained within the Field component)

Cons:

  • Dev users have to use 2 components (Field + the input component)
  • Not including the Field features by default inside input/control components could discourage devs from adding important elements like semantic labels. We would need to consider how to effectively document a11y recommendations.
  • More difficult to add Field features that only apply to some inputs, e.g. character count. Such features should be limited (character count is the only one we've identified), but their implementation is not ideal with this option.

Option 2: Internal Field component used inside input/control components

Option 2 would integrate the Field component within input/control components, so that they would internally use the Field component, rather than the dev user using it. Dev users would only use the input/control component itself, and pass both field and input props into that component. That said, the Field component would be available to dev users if they needed to build their own custom input/control (but most dev users would not need to use it directly).

Pros:

  • Dev users only need to use a single component
  • Encourages use of Field features like semantic labels
  • More easily enables us to apply input-specific Field features, like character count, which only applies to TextInput and TextArea

Cons:

  • Could limit or discourage composition. Technically, you could still compose inputs into a more complex input and use that within the Field component, but this is not the "happy path" and may not be as clear with this implementation.
  • We would probably need to have RadioGroup and CheckboxGroup components to properly handle semantic labeling of these things. More components to know about and maintain.
  • Documentation may be less clear or redundant since we'd need to document Field features on every input/control page, and field-related props would be included in the props table for all of those components (could be noisy)
  • Potentially more complex from a library maintenance standpoint, because we'd need to guarantee that things like nested fields would work (there's a lot of props, providing, and injecting going on to get everything to work right). See the nested fields demo for an example - the field description of the Lookup lives within the TextInput component, so the Lookup's menu is output below the entire TextInput. We'd need to account for issues like this, and it might make it harder for other devs to compose their own fields.

Acceptance criteria

  • Discuss these options with stakeholders
  • Choose a path forward - we've chosen option 1
  • Document with an ADR

Event Timeline

AnneT triaged this task as Medium priority.Mar 1 2023, 4:12 PM
AnneT moved this task from Inbox to Up Next on the Design-System-Team board.

Thank you guys for the detailed explanation of the problem.

Reading the above description and using past experience I would personally be keener toward Option 2. The reasoning for the decision (happy to give more details):

  • Reduce the uncertainty of WHAT component the user uses with Option 1. Allow a user to use ANYTHING they want may give false expectations (eg a fancy calendar input or date picker)
  • Not providing the free use for all provided by Option 1, it will also allow you to develop and deliver this component by component and not have to do ALL components at once (the thing that may be expected in option 1)
  • It allows the Field component to be more complex to set up to give finer control (this means you can actually make a complex field component that is complex to use as you will be the only one using it and probably some advanced users).
  • In the long run, there may be some specific features and implementations for specific input. If you offer Option 1, then this feature will have to be included in the generic Field component documentation (eg a property that counts the character in the field). This should be a TextArea, Input-only feature, but if you make a generic component it will have to be in the generic documentation.
  • For documentation purposes, I think it will not be much duplication as you will be able to create a Generic Field Component usage Page, and then just add the specific props/feature for the current component on the Doc.

I looked at Wikit, but couldn't find a place that discusses why we went for what is effectively Option 2.
One of the main difference between Wikit and Codex, that I see, is that Wikit had a much smaller audience that was also much less diverse (in terms of use-case). So, I think, it made more sense there to emphasize ease-of-use and standardization over flexibility. Whereas for Codex, it might be the case that it pays off to lean more towards flexibility in order to satisfy a wide range of use-cases? Though, I can also see the value of having "convenience components"/"syntactic sugar" that are just a thin wrapper around field + input for common use-cases, especially with respect to user scripts and gadgets.

One of the learnings from Wikit that might be relevant here. At several occasions, we needed to extend the "field" part of a component to add functionality that I also do not see in your patches yet:

  • the ability to add a suffix slot, which we used to add an "ℹ️" icon-button that has a popover next to the label (see https://query.wikidata.org/querybuilder/)
  • we (mis-?)used that suffix slot also for an asterisk to denote required input fields

I feel it might be easier to add such functionality later when we have a generic field component over changing a lot of specialized components. OTOH, especially for the "required" use-case, one might need/want a change both to the label / copy as well as to the input.

So, in the end, I think it would be beneficial to have both the integrated components as well as the individual pieces. That would be maybe similar to Vuetify.
So a relevant question could probably also be: "What do we start with / consider primary?" and "What will we consider stable?"

My personal two cents (i.e. not speaking for WMDE-TechWish) as someone who will certainly use this: I prefer the option that results in smaller components that can be reused and combined in more flexible ways.

Example: Let's say I want to build a component that is made of two input fields next to each other, e.g. to input a pair of coordinates (latitude and longitude). I want to wrap them in a single field with a single label and a single red error message below. I'm not sure if this is possible with option 2.

  • What happens when I use the input component from option 2, disable all field-related features (i.e. it shouldn't have a separate label or error message), and wrap two of them in a field? Does this create the DOM elements for 3 fields, but 2 of them don't have a visible effect?
  • In option 2, can the field component even be used to wrap components that haven't been developed with field features in mind?

Can't we have the best of both worlds? When we go with option 1 isn't it still possible to add a TextInputField component to Codex that can conveniently be used whenever we want to have all the pros from option 2, like "encouraging use of field features like semantic labels"? It should need very little code.

It might totally be that I got this wrong and all this is easily possible with both options.

Restricted Application raised the priority of this task from Medium to High. · View Herald TranscriptMar 6 2023, 5:45 PM
AnneT updated the task description. (Show Details)

Thanks very much for all of these comments; they have helped me uncover more considerations and reach a point where I believe one path is more clearly correct for our developer community.

Complex field examples

Based on @thiemowmde's comment above and similar comments made by @egardner, I updated my proofs of concept to add examples of more complex fields, detailed below.

Coordinate location field

This is a field with inputs for coordinates and level of precision. It consists of a Field component set to output a <fieldset>, with a TextInput and a Select, both of which have hidden labels. Validation only applies to the TextInput component, and any error messages are displayed at the level of the entire fieldset.
Code for option 1
Code for option 2

As you can see, the code on the developer-user end is quite similar. In option 1, I labeled the TextInput via the CdxLabel component, and the Select via aria-label. In option 2, I'm simply passing the label and hideLabel props into the inputs. Option 1 is more flexible, option 2 is more obvious.

Weight field

This is a field with a numerical value and units (based on the Wikidata "quantity" property type). It consists of a Field component set to ouput a <fieldset>, with a TextInput and a Lookup, both of which should have visible labels, their own error handling, and error messages shown at both the level of the individual input and the entire fieldset. (I'm not sure this last part has a real use case, but it seems possible that developers might want the option of including messages per input or for the entire fieldset.)
Code for option 1
Code for option 2

Again, the code that the developer user would write is quite similar. In option 1, we are nesting Field components inside an outer Field component. A flaw of option 1 is that you must bind :disabled to both the outer Field and the inner Fields; the outer Field will not tell the inner Fields whether they should be disabled (although we can probably figure out a sensible way to do this). A flaw of option 2 is that the Field component within the Lookup actually wraps the TextInput, not the Lookup itself. As a result, the Lookup's menu ends up displaying under its TextInput's description, something we would have to fix and try to predict for other components

General comments

Although I agree with @SimoneThisDot's comments in favor of limiting potential usage in the name of encouraging predictable and proper usage, I think option 2 will be both more difficult to implement and maintain, and too limiting for our developer community. As challenging as it can be to deal with, we in that community often need or want to build something totally custom. I think we likely need to enable a wide variety of permutations of fields, and so providing simpler building blocks is the way to go.

Another vote in favor of simpler building blocks is that option 2 took a lot more code to implement, and currently contains a major bug (see the Lookup issue described in the "Weight field" section above). Although we want to contain complexity within the library as much as possible, in order to save the developer user from having to deal with it, I personally do not find option 1 to be significantly more complex from a developer user standpoint. The dev user will have to import and use an additional component, and we will want to carefully document how to include a semantic label for your input/control, but I think these things could be acceptable (but please let me know if you disagree!)

A final note is that we have very limited resources to build Codex. Building a simple Field component that the developer user can then implement as they wish may allow them to find solutions for their own problems, rather than trying to solve all problems in Codex.

All that said, the major downsides with option 1 are:

  • A more open API may lead to improper usage
  • We need to ensure Field works with all components at once
  • Implementing Field features that only apply to some inputs will be more difficult (e.g. character count)

Comments are still welcome; we won't be implementing this right away so there is still plenty of room for discussion!

This is something we plan to add to the Codex component as well, although I'm not 100% how we'll do so in an accessible way yet (maybe we can make the text part of the label? I just want to make sure it gets read by screen readers when the user tabs into the input)

  • we (mis-?)used that suffix slot also for an asterisk to denote required input fields

I'm also not 100% sure what we'll do about this....we plan on making fields required by default and allowing them to be marked as optional, which will be done by appending the word "(optional)" to the label (translated, of course). But I'm not yet sure how we'll do this semantically.

So, in the end, I think it would be beneficial to have both the integrated components as well as the individual pieces. That would be maybe similar to Vuetify.

Can't we have the best of both worlds? When we go with option 1 isn't it still possible to add a TextInputField component to Codex that can conveniently be used whenever we want to have all the pros from option 2, like "encouraging use of field features like semantic labels"? It should need very little code.

We have considered providing Field, TextInput, and TextInputField, similar to OOUI's architecture (e.g. there's RadioSelectWidget and RadioSelectInputWidget, which is for use in forms). My first reaction was that it's confusing to have both TextInput and TextInputField, but I have heard this suggested several times now so I think it's worth considering. It might be the best way for us to provide both flexibility and a "happy path" for users with simpler use cases.

A flaw of option 1 is that you must bind :disabled to both the outer Field and the inner Fields; the outer Field will not tell the inner Fields whether they should be disabled (although we can probably figure out a sensible way to do this).

We could use provide/inject so that a nested Field can find its parent Field and disable itself if the parent is disabled. We could also explore using this technique to communicate state between Field components and the input components inside them, although that would require a bit more complexity, as we'd have to duplicate that logic in TextInput, Checkbox, Radio, etc (we could wrap it in a composable, but still, all those components would have to know about it).

A flaw of option 2 is that the Field component within the Lookup actually wraps the TextInput, not the Lookup itself. As a result, the Lookup's menu ends up displaying under its TextInput's description, something we would have to fix and try to predict for other components

I suppose this could be addressed by having TextInput take a prop that suppresses the Field wrapping or makes it a no-op, and Lookup then wrapping itself in its own Field. But that's messy, and it would have to be repeated in every component that wraps TextInput.

We have considered providing Field, TextInput, and TextInputField, similar to OOUI's architecture (e.g. there's RadioSelectWidget and RadioSelectInputWidget, which is for use in forms). My first reaction was that it's confusing to have both TextInput and TextInputField, but I have heard this suggested several times now so I think it's worth considering. It might be the best way for us to provide both flexibility and a "happy path" for users with simpler use cases.

I do think this is confusing, and I don't think that having to figure out the difference between TextInput and TextInputField is that much less complex or confusing than having to combine the Field+TextInput components, especially if we document this clearly.

Overall, I prefer option 1, for the reasons you lay out. I think it will be beneficial to have a simpler component library with clearer component boundaries, and I don't think there's much benefit in trying to hide the Field component by wrapping other components in it.

Update - we will draft a decision record for this next week.

Change 921094 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] docs: Add ADR for Field component implementation

https://gerrit.wikimedia.org/r/921094

AnneT changed the task status from Open to In Progress.May 18 2023, 8:09 PM

Change 921094 merged by jenkins-bot:

[design/codex@main] docs: Add ADR for Field component implementation

https://gerrit.wikimedia.org/r/921094

AnneT updated the task description. (Show Details)

Change 927786 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from v0.11.0 to v0.12.0

https://gerrit.wikimedia.org/r/927786

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/8ca78d2715/w

Change 927786 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.11.0 to v0.12.0

https://gerrit.wikimedia.org/r/927786

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/8ca78d2715/w/