Page MenuHomePhabricator

Add readonly state to TextInput component
Closed, ResolvedPublic

Description

The Figma spec sheet for the TextInput component includes a read-only state, but this is not yet implemented. Instead of implementing this as a prop, we should just accept the native attribute.

This should be pretty straightforward style-wise, but we should ensure that the readonly state interacts properly with other states. For example:

  • If a TextInput is both readonly and disabled, the disabled styles should take precedent
  • Should a readonly TextInput be focusable? If not, then it should always look the same regardless of focus/hover/active state. If so, we will need to determine exactly how it should look in these states.

Getting this right may require some consideration during development and possibly a conversation with the design team.

Acceptance criteria
  • TextInput properly responds to the readonly attribute: when readonly is applied, the user cannot change the value and styles match the readonly state as shown on the spec sheet
  • The read-only behavior is documented on the docs site as a boolean prop of the configurable demo (we also include the placeholder attribute here)

In the future, a separate demo similar to the disabled demo could be added.

Event Timeline

Restricted Application raised the priority of this task from Lowest to High. · View Herald TranscriptMar 13 2023, 4:09 PM
AnneT added a subscriber: egardner.
AnneT subscribed.

@egardner I've added some more info here, so this should be ready for development (although part of the dev process will be determining how readonly interacts with states like :focus)

Change 923709 had a related patch set uploaded (by Catrope; author: VolkerE):

[design/codex@main] TextInput: Add `readonly` styling and config demo

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

Should a readonly TextInput be focusable? If not, then it should always look the same regardless of focus/hover/active state. If so, we will need to determine exactly how it should look in these states.

By default, it looks like a readonly TextInput is focusable, and in the demo at https://923709--wikimedia-codex.netlify.app/components/demos/text-input.html it receives the same focus styles as a normal TextInput. I think that's probably fine because the readonly styling only affects the background color of the TextInput, but it's worth checking with @bmartinezcalvo

Should a readonly TextInput be focusable? If not, then it should always look the same regardless of focus/hover/active state. If so, we will need to determine exactly how it should look in these states.

By default, it looks like a readonly TextInput is focusable, and in the demo at https://923709--wikimedia-codex.netlify.app/components/demos/text-input.html it receives the same focus styles as a normal TextInput. I think that's probably fine because the readonly styling only affects the background color of the TextInput, but it's worth checking with @bmartinezcalvo

@Catrope I think the differences between disabled and read-only are:

  • disabled cannot be edited or focus
  • read-only cannot be edited but it can be focus. The difference between default and read-only style is that read-only uses the background-color-interactive-subtle.

If it helps, in the OOUI demo the read-only text input is focusable.

If the read-only is focusable, do we need to include this state in the Figma spec?

Question about this task. Looking at @Catrope's latest patch, it looks like this is actually complete. Is it just a matter of code review?

I think the patch (which was written by Volker but tagged into this task by me) implements everything that is needed for this task. I wanted to make sure that it was OK for a readonly TextInput to be focusable, but Bárbara has clarified that it is. I will merge Volker's patch and then this task will be done.

Change 923709 merged by jenkins-bot:

[design/codex@main] TextInput: Add `readonly` styling and config demo

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

@Catrope so should this task be resolved? or moved into "Pending Release" on the sprint board?

Yes, it should go to pending release. Apologies for the delay.

AnneT updated the task description. (Show Details)

Change 935794 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v0.13.0 to v0.14.0

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

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

Change 935794 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.13.0 to v0.14.0

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

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

https://patchdemo.wmflabs.org/wikis/64dc3126c0/w/