Page MenuHomePhabricator

Add error state to TextInput component
Closed, ResolvedPublic3 Estimated Story Points

Description

Background goal

The current Codex library Figma spec sheet for the TextInput component specifies an error state (with sub-states for when the input is filled or focused while in the error state), but this isn't currently implemented.

We should consider how to implement this in TextInput, and how the responsibility for error state management and styling should be divided between TextInput (and other inputs), and the Field component specified in T309239.

User stories
  • As a user I need to know when a Text Input has an error and how to solve it.
Previous implementations
Design spec
Development considerations

A validation message (text describing the nature of the error) will be implemented as part of the Field component T309239, which will contain an input and take in either a boolean error prop or a rule/function for checking validity. Because the Field component needs to be aware of whether an error is present in order to show the proper message, this validation must happen at the level of the Field component or above.

Therefore, the TextInput component should take in a boolean error prop which, when true, will enable error styles (e.g. the red border). Error text will not be a part of the TextInput component itself.

Acceptance criteria

Design

  • Update Figma spec sheet (if needed) and add link in this task
  • Update Figma library component and publish library (if needed)
  • Add missing error text input in the DSG T302064

Code

  • Implement error text link in Codex

Design Review

More details in this comment

  • Error focus should use border-color-progressive--focus

Event Timeline

ldelench_wmf set the point value for this task to 3.
ldelench_wmf added a subscriber: ldelench_wmf.

Moving into sprint board so that it may be worked on collaboratively with AW. DST will create spec sheet.

In terms of how we're going to approach this:

  1. @bmartinezcalvo will provide a spec sheet detailing the error state
  2. DST will add clear acceptance criteria to this task informed by the above
  3. It sounds like the Abstract Wikipedia Team has a use-case for this feature, so ideally one of the AW engineers could start a patch to implement this; DST can provide guidance, setup / dev environment help, and code review to help get this merged in a timely manner
bmartinezcalvo renamed this task from Consider adding error state to TextInput component to Add error state to TextInput component.Oct 31 2022, 12:28 PM

The Figma spec sheet has been added in the task description (you can check it here) and the task is ready to be implemented.

Captura de Pantalla 2022-10-31 a las 14.09.14.png (1×2 px, 651 KB)

NOTE: the error state will be implemented just in the input field. Error helper text will not be a part of the TextInput component itself since it will be part of Field component T309239.

@bmartinezcalvo / @SWoodruff-WMF - just to confirm, there is no warning state?

Warning is not a common Text Input state and we don't have any current real use case with it. We will include it in the future in case we find a warning text input real use case. Do you have any use case in mind?

Change 852193 had a related patch set uploaded (by Jkieserman; author: Jkieserman):

[design/codex@main] add error state to TextInput component

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

I don't! Just thought I saw warning docs on some of our earlier mocks, but that sounds fine to me!

Change 852193 merged by jenkins-bot:

[design/codex@main] TextInput: Add error state to TextInput component

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

@JKieserman design sign-off done. I add here some things I found to fix:

  1. Would it be possible to group default + error + disabled in the same "Status" property line? If not, would it be possible to move the "Disabled" property under the "Status" so all the component states are more grouped?
    Captura de Pantalla 2022-11-11 a las 9.42.47.png (1×1 px, 540 KB)
  1. When the error input is focus it should use border-color-progressive--focus instead since the red focus we have in our tokens is for border-color-destructive--focus but this is not a destructive action but an error state. We have 2 ways of indicate a focus in an error form item:
    • Using the red to indicate that the item is still error even when the user is going to fix it (focus)
    • Using the blue to indicate that the error is being fixed

      Both options are ok. Just keep in mind that if we decide to paint the focus for the error state in read then we should introduce a new border-color-**error**--focus cc: @Volker_E

@JKieserman design sign-off done. I add here some things I found to fix:

  1. Would it be possible to group default + error + disabled in the same "Status" property line? If not, would it be possible to move the "Disabled" property under the "Status" so all the component states are more grouped?
    Captura de Pantalla 2022-11-11 a las 9.42.47.png (1×1 px, 540 KB)

@bmartinezcalvo disabled has a specific meaning for inputs (it's an HTML attribute), while status is a custom thing that we've introduced. I don't think it makes sense from a development perspective to combine them.

Our thinking was that the status prop could in the future support other states as well (like success and warning if they become necessary), but disabled will always be separate. I'd recommend leaving the props documentation the way it currently stands – it's useful for developers because it demonstrates exactly what options you can provide in the code.

Regarding focus, I think it still makes sense to have the focus outline be red if the input is in an erroneous state – that way the user knows they need to fix it before they can proceed. So maybe we do need to provide a new token or otherwise update our design guidelines here.

@egardner @bmartinezcalvo Bárbara is right here about the focus, I missed that in my review. The focus was set to be consistently blue for everything that is not a destructive action to simpler guide the user in OOUI. That's assuming instant validation, while tabbing out you would have the border red again. The reason for the decision was to signal the user that it's erroneous after their input, but while revisit inputing to the erroneous field to turn to normal.
If we were about to change, we would have to amend this in OOUI as well and expand to other erroneous status carrying components. And clarify the guidelines (in Figma).

@egardner @bmartinezcalvo Bárbara is right here about the focus, I missed that in my review. The focus was set to be consistently blue for everything that is not a destructive action to simpler guide the user in OOUI. That's assuming instant validation, while tabbing out you would have the border red again. The reason for the decision was to signal the user that it's erroneous after their input, but while revisit inputing to the erroneous field to turn to normal.
If we were about to change, we would have to amend this in OOUI as well and expand to other erroneous status carrying components. And clarify the guidelines (in Figma).

@egardner as Volker comments, the blue focus for error inputs is also common and it's being used in OOUI. You can also check it in other design systems e.g. Carbono where the error is just in the helper text and icon but not in the focus.

Captura de Pantalla 2022-11-21 a las 15.49.02.png (254×568 px, 20 KB)

@JKieserman moving this task to Ready for development so you can update the focus state with border-color-progressive--focus.

@bmartinezcalvo disabled has a specific meaning for inputs (it's an HTML attribute), while status is a custom thing that we've introduced. I don't think it makes sense from a development perspective to combine them.

Our thinking was that the status prop could in the future support other states as well (like success and warning if they become necessary), but disabled will always be separate. I'd recommend leaving the props documentation the way it currently stands – it's useful for developers because it demonstrates exactly what options you can provide in the code.

Regarding focus, I think it still makes sense to have the focus outline be red if the input is in an erroneous state – that way the user knows they need to fix it before they can proceed. So maybe we do need to provide a new token or otherwise update our design guidelines here.

@egardner if we can't add the disabled inside the status section we could move the disabled section under the status one in order to group both sections more. I mean organize the properties by the following order:

Props

  1. startIcon
  2. endIcon
  3. clearable
  4. status
  5. disabled
  6. placeholder

Change 859123 had a related patch set uploaded (by Jkieserman; author: Jkieserman):

[design/codex@main] CdxTextInput: error status uses progressive palette for focus

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

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

[mediawiki/core@master] Update Codex from v0.2.2 to v0.3.0

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

Change 859597 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.2.2 to v0.3.0

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

@egardner if we can't add the disabled inside the status section we could move the disabled section under the status one in order to group both sections more. I mean organize the properties by the following order:

Props

  1. startIcon
  2. endIcon
  3. clearable
  4. status
  5. disabled
  6. placeholder

The most recent patch (which has been merged, gerritbot must have missed that) includes this change too.

@egardner if we can't add the disabled inside the status section we could move the disabled section under the status one in order to group both sections more. I mean organize the properties by the following order:

Props

  1. startIcon
  2. endIcon
  3. clearable
  4. status
  5. disabled
  6. placeholder

The most recent patch (which has been merged, gerritbot must have missed that) includes this change too.

@Catrope oh yes, I can find it this new organization now in the demo. Thank you!

Captura de Pantalla 2022-11-23 a las 12.37.32.png (1×1 px, 125 KB)

bmartinezcalvo updated the task description. (Show Details)

Design sign-off done. Both error focus color and demo organization have been fixed. Moving the task to QTE sign-off.

Captura de Pantalla 2022-11-23 a las 12.39.58.png (1×1 px, 116 KB)

Tested on Chrome v106, Safari v16 and Firefox v107:

For the TextInput component in an error state:

  1. The placeholder displays correctly in an empty textbox and vanishes when cleared.
  2. The start and end icons are visible in the text box, when used.
  3. A clearable icon can be used to clear the field.
  4. The border of the text input have the specified red token colour.
  5. The component is keyboard accessible and is a tab stop. Text can be entered into it and deleted from it.
  6. When in focus, the red border colour is replaced with blue.

All conditions are satisfied.