Add clear button by default to text input controls.

[chipsnyder]

Is your feature request related to a problem? Please describe. As part of a recent empathy challenge, it was pointed out that it would be nice to add a clear button available by default for the focused state on all text input outside of the editor's canvas.

Describe the solution you'd like I think a simple X in the text field or a clear button at the bottom would be nice maybe something similar to what the Link Picker has.

Example from Social Icons	Link Picker

[iamthomasbishop]

@chipsnyder Agree that we should probably implement a “clear” button here (probably on the focused state, but I need to look at other examples to see what approach is most common), but also pretty much across the board on the focused states of text inputs (excluding RichText on the editor canvas, obviously). Would it make more sense to attack this on the component-level or only for this case specifically, and leave the component-level work to a later time?

[chipsnyder]

Would it make more sense to attack this on the component-level or only for this case specifically, and leave the component-level work to a later time?

Component level sounds better to me! I'll update the ticket to reflect that.

[derekblank]

@iamthomasbishop @chipsnyder I'll be taking a look at this one! 👋 I'll outline some thoughts on how we might get started:

There are ways to implement a native clear button in a text field for iOS and Android, but due to a limitation with React Native, only the iOS solution is supported. Fortunately, it can be handled in Javascript, however. (I discuss this a bit more generally in pbMoDN-34o-p2, if you'd like to read further about it.) In the places where the clear button is currently being used in Gutenberg Mobile (like the Link Picker, shown in the example above), it is indeed being handled by Javascript, so that sets a good precedent already.

Background and Scope

To kick off a discussion about adding a clear button to the text fields where it is not currently being used, I just made some reasonable assumptions about scope. These can be changed as the discussion evolves, of course, but just as a starting point:

1) We should provide a clear button to any text input outside of the editor canvas. 2) We should not use the clear button on multiline text inputs (i.e., textareas). This matches native platform behavior. 3) We should not use the clear button on numerical range text inputs, as they are generally small, and often controlled by another component (like a slider).

Discovery

To get started on the first item, I took a closer look at all of the text fields that fall outside of the editor canvas, starting the search with any file with a *.native.js extension. I found that in most places, the text field is being handled by BottomSheet.Cell.

This is great, because this component is mature and fairly all-purpose. It handles a lot of the basic behavior for TextInput in Gutenberg Mobile, and includes robust test cases already. Essentially, it is the "CustomTextInput" component wrapper I described in pbMoDN-34o-p2.

However, BottomSheet.Cell does not implement the clear button behavior directly. In cases where the clear button is being used, like the Link Picker, BottomSheet.Cell is being used, but the clear button is being nested within it on an ad hoc basis (source code here).

Another case where the clear button behavior is being used is Search Control, which is one of the first TextInputs the user sees. Search Control is not using BottomSheet.Cell at all. It is importing TextInput directly from React Native, and adding the clear button in an almost identical way as the Link Picker behavior. Examples of the above:

Search Control Implemented, but ad hoc and not at the component level:

Social Link Input Not implemented at all (yet):

Range Input Likely, the clear button is not applicable here:

Next Steps

Given that there are some good precedents where the behavior we want already exists, next steps would be to define the scope, define the validity of the existing behavior, and apply that existing behavior to those areas. At the moment, pending further discussion of course, a potential path forward seems to be:

0) Clarify the scope. 1) Add clear button support to BottomSheet.Cell. Essentially, transfer the clear button code from Link Picker directly to BottomSheet.Cell instead, and update the test cases where applicable. 2) Within the scope definitions, replace raw React Native TextInputs with BottomSheet.Cell as the text field component. Don't use it in other places not in scope (like multiline TextInputs), use TextInput directly from React Native instead.

Discussion

Opening it up for questions for discussion, particularly on the scope. (More to the point, thanks for reading this far. 🤠 ) Since this is a trial task, there likely might be a disproportionate amount of information and discussion here given the actual end benefit for the user. Conversely -- in general, I believe that details are important, and since text input of any kind is a heavily used feature on Gutenberg, I welcome and appreciate any nuanced feedback here too.

[fluiddot]

1. Add clear button support to BottomSheet.Cell. Essentially, transfer the clear button code from Link Picker directly to BottomSheet.Cell instead, and update the test cases where applicable.

This step is a good start, as you commented, most of the use cases of TextInput component fall into the BottomSheet area, so adding a clear button there would satisfy plenty of cases.

2. Within the scope definitions, replace raw React Native TextInputs with BottomSheet.Cell as the text field component. Don't use it in other places not in scope (like multiline TextInputs), use TextInput directly from React Native instead.

Not sure if we should expand the use of the BottomSheet.Cell component outside the BottomSheet area, as it was created to be a sub-component, which probably means that part of its behavior is ad-hoc to the BottomSheet.

Opening it up for questions for discussion, particularly on the scope. (More to the point, thanks for reading this far. 🤠 ) Since this is a trial task, there likely might be a disproportionate amount of information and discussion here given the actual end benefit for the user. Conversely -- in general, I believe that details are important, and since text input of any kind is a heavily used feature on Gutenberg, I welcome and appreciate any nuanced feedback here too.

No worries about the amount of information, I think it makes sense to provide as much detail as possible, moreover in a trial project and touching a core functionality as text inputs, before heading to implementation 👍 .

[derekblank]

Not sure if we should expand the use of the BottomSheet.Cell component outside the BottomSheet area, as it was created to be a sub-component, which probably means that part of its behavior is ad-hoc to the BottomSheet.

Right -- this is a good point, and I agree with you about not expanding the scope of BottomSheet.Cell outside of BottomSheet.

A curious detail here is that the main native TextControl component is actually using BottomSheet.Cell too though, importing it directly:

import Cell from '../mobile/bottom-sheet/cell';

So BottomSheet.Cell is doing a lot of work. If we added the clear button handler function directly to BottomSheet.Cell, we'd certainly want to make it "opt-in" via a prop, as you mentioned. This way, we could we could set this to false in TextControl to avoid any unintended side-effects where TextControl is being used, and focus on an initial scope of TextInputs in BottomSheet, some of which need to be standardized as they are importing TextInput directly from React Native (like Search Control).

Another option would be to break Cell out of BottomSheet into a more generic Cell component that handles basic text input functionality, and lives outside of BottomSheet. This might be a very large scope, but it's certainly possible. Just to limit scope for discussion purposes, for example, if we were to reword the task title another way that included a scope definition, it could also be called something like "Standardize TextInput clear button behavior in BottomSheet text inputs". WDYT?

[fluiddot]

A curious detail here is that the main native TextControl component is actually using BottomSheet.Cell too though, importing it directly:

import Cell from '../mobile/bottom-sheet/cell';

Good catch! Now after checking the TextControl component further, I'm noticing that we're likely misusing the component in some places. As the README file states, I understand that the purpose of the component is editing text, however, I see that it's being used just as an interactable text (example reference). Nevertheless, most of this cases are rendering the TextControl component within the BottomSheet.

So BottomSheet.Cell is doing a lot of work. If we added the clear button handler function directly to BottomSheet.Cell, we'd certainly want to make it "opt-in" via a prop, as you mentioned. This way, we could we could set this to false in TextControl to avoid any unintended side-effects where TextControl is being used, and focus on an initial scope of TextInputs in BottomSheet, some of which need to be standardized as they are importing TextInput directly from React Native (like Search Control).

True, BottomSheet.Cell component is joining a lot of functionality. The "opt-in" is indeed a desired requirement for the clear button, as it will help us to prevent unintended side-effects.

Another option would be to break Cell out of BottomSheet into a more generic Cell component that handles basic text input functionality, and lives outside of BottomSheet. This might be a very large scope, but it's certainly possible. Just to limit scope for discussion purposes, for example, if we were to reword the task title another way that included a scope definition, it could also be called something like _"Standardize TextInput clear button behavior in BottomSheet text inputs".

To be honest, I'd expect the generic Cell component to be the TextControl, as the functionalities are more aligned to what's described in its README file. Being this said, I agree that this change might be out of the initial scope, but we could still consider it as part of this task, maybe we could split the task into two parts and tackle them separately:

1. Standardize TextInput clear button behavior in BottomSheet text inputs (this task will cover most cases)
2. Standardize TextInput clear button behavior in all text inputs (this task will act more as a refactor and to set a better foundation of the text input component)

@derekblank Wdyt?

[derekblank]

@fluiddot I think breaking the task into two parts is a good idea!

I think I can get started with the first step since the work and scope seems clear:

• Add clear button functionality to BottomSheetCell, with a default prop value of false, and updating relevant tests.
• Update TextInputs that are single-line, non-editor, and live inside BottomSheet to use the clear button behavior from BottomSheetCell.

These might actually be better as two separate PRs to reduce the complexity, especially since TextControl is using BottomSheetCell, and TextControl is used in a lot of places.

Then, a possible third PR after getting more feedback from others:

• Break BottomSheetCell out of BottomSheet, and either add it to TextControl directly, or to be its own component (imported by TextControl and other places).

Regarding the discussion to move BottomSheetCell outside BottomSheet: I reviewed the test cases for TextControl and BottomSheet, and there does not actually seem to be much behavior in the BottomSheetCell component that is specific to the BottomSheet itself. It all seems like general TextInput behavior. Reading through the docs on BottomSheetTextControl, however:

BottomSheetTextControl's main difference from TextControl is that it utilizes BottomSheetSubSheet. A user will tap to open a subsheet where they can enter their text into an editor. This is useful for cases where a larger text area is warranted for user input.

To make the situation even more complicated, BottomSheetTextControl does not use BottomSheetCell -- it imports TextInput directly from React Native. This makes sense, however, as BottomSheetTextControl is only used as a text editor component. I also agree with you that some of these components are no longer being used the sole purpose they were originally created for. Perhaps the broader discussion should include when we should make the distinction between a component that is just for editing text, and one that is just interactable.

[fluiddot]

Perhaps the broader discussion should include when we should make the distinction between a component that is just for editing text, and one that is just interactable.

Yeah, eventually we should open this discussion if the TextControl is refactored. I'm wondering why we ended up using that component for other purposes, probably this was originated due to the fact that BottomSheetCell component contains too much logic and, instead of decoupling it to standalone components, we overused TextControl 🤔 . In any case, I'd focus on this later on (probably in a different issue) as I feel it's a bit off the scope of this issue.