Jill wip/clearable button #617

jrenee42 · 2021-05-03T21:56:06Z

for ui #898
https://app.zenhub.com/workspaces/applicationui-5e3b05772922e316b3a210a4/issues/influxdata/ui/898

Changes

adds an 'x' that shows up in the right of the textfield if and only if:

there is content in the textfield
the 'onClear' property is present; with a function to be activated when the button is clicked

the button is reachable by tabbing, so the user can type in data, then click 'tab' and the press 'return' to clear the field without ever having to use the mouse.

it uses the dismiss button, with some styling changes to get it to fit nicely in the field.

if the field is large, then the button is sized up, else it stays in its default, smaller size.

Added to storybook as well; so added to the documentation.

Screenshots

Screen.Recording.2021-05-03.at.4.28.31.PM.mov

Checklist

Check all that apply

Updated documentation to reflect changes
Added entry to top of Changelog with link to PR (not issue)
Tests pass
Peer reviewed and approved

daviddkkim

#616

When the input is focused, the X icon appears. When the X icon is focused, the X icon stays appeared. When neither the X icon or the input is focused, the X icon does not exist.

daviddkkim · 2021-05-03T22:39:53Z

src/Components/Inputs/Input.tsx

+
+    const clearElement = correctlyTypedValue && onClear && (
+      <DismissButton
+        style={clearBtnStyle}


Do we need to expose this style prop? I think we could just use color = {ComponentColor.default}. I don't see where we would need to change the color or style of the dismiss button within the input but if you can think of concrete future usecases for it, would love to hear it

As for styling, I think the default color set is the subtlety we are looking for rather than the primary color that comes out of box with the dismiss button component.

are you saying that the color should. be changed from what it currently is?

Two things:

I think the Default Color Dismiss button might be more suitable than the Primary Color Dismiss Button.

I'm trying to understand the rationale for style={clearBtnStyle}. It seems like its just passing in undefined.
In what scenarios would we want to pass in custom style to this dismiss button?

daviddkkim · 2021-05-03T22:41:40Z

src/Components/Inputs/Input.scss

-.cf-input-icon {
-  transform: translate(-50%, -50%);
+.cf-input-icon,
+.cf-input-clear-btn {


This might be a .cf-input__clear-btn based on BEM methodology but I could be wrong

daviddkkim · 2021-05-03T22:44:15Z

src/Components/Inputs/Input.scss

+  }
+
+  &.large {
+    width: 32px;


Let's use variables instead of hard coding the width and heights if we can.

daviddkkim · 2021-05-03T22:46:36Z

src/Components/Inputs/Input.scss

+    right: 4px;
+  }
+
+  &.large {


I believe the possible sizes of inputs are :

extra small

small

medium

large

I think you've only accounted for small and large. Try a medium or extra small input on your storybook, I imagine the X won't scale for those.

i only did small (default) and large since those are the only use cases we need for the ui; I can add the others but we aren't using them at this point

daviddkkim · 2021-05-03T22:47:53Z

src/Components/Inputs/Input.tsx

@@ -173,6 +182,19 @@ export const Input = forwardRef<InputRef, InputProps>(

    const iconElement = icon && <Icon glyph={icon} className="cf-input-icon" />

+    const clearClasses = classnames('cf-input-clear-btn', {
+      large: size === ComponentSize.Large,


need to account for extra small, and medium as well

daviddkkim · 2021-05-03T22:48:36Z

src/Components/Inputs/Input.tsx

+   * */
+  onClear?: () => void
+  /** optional extra styling for the clear button */
+  clearBtnStyle?: CSSProperties


Bit of a redundant feedback here but Not sure if we need to expose this. Im trying to think of a case where we would need a special css treatment of the dismiss button and can't think of it.

i needed it previously, then after adjusting for component size don't need it; but it was already there so I left it in.

...just removed it

daviddkkim · 2021-05-03T22:49:31Z

src/Components/Inputs/Input.scss

+    & .cf-dismiss-button--x {
+      &:before,
+      &:after {
+        width: 20px;


Let's use variables instead of hard coding the width and heights if we can.

mavarius

Having the button disappear when you click it is a jarring experience. As long as the field or the button has focus then it should be visible.

You may need to prevent this from appearing when type="number".

Ensure that the button is vertically centered regardless of the input size. It may help here to use the extra small and medium button sizes along with those input sizes.

The text should stop before the clear button otherwise it becomes obscured. See the VisibilityInput component for how this should be handled.

mavarius · 2021-05-03T23:21:03Z

src/Components/Inputs/Documentation/Inputs.stories.tsx

+    return (
+      <div className="story--example">
+        <Input
+          min={number('min', 0)}


Did you mean minLength & maxLength? min & max are for type="number".

the 'min' and such I just copied from another storybook story.
(for the default, plain text input).

in terms of the button disappearing, @daviddkkim specifically requested that the 'x' button only show when there is content. so after pressing it, there is no content so it disappears. easy to make it always appear tho.

mavarius · 2021-05-03T23:30:22Z

src/Components/Inputs/Input.tsx

@@ -12,18 +12,19 @@ import classnames from 'classnames'
 // Components
 import {Icon} from '../Icon/Base/Icon'
 import {StatusIndicator} from './StatusIndicator'
-
+import {DismissButton} from '../Button/Composed/DismissButton'
+//import {Button} from '../Button/Composed/Button'


Remove if no longer used.

jrenee42 · 2021-05-04T14:43:25Z

#616

When the input is focused, the X icon appears. When the X icon is focused, the X icon stays appeared. When neither the X icon or the input is focused, the X icon does not exist.

then how will the users discover this feature? what if they enter text, click away, then the 'x' disappears, then they want to clear it, so then they click in the field, and then it appears? that is non-intuitive.

daviddkkim · 2021-05-04T18:39:11Z

#616

When the input is focused, the X icon appears. When the X icon is focused, the X icon stays appeared. When neither the X icon or the input is focused, the X icon does not exist.

then how will the users discover this feature? what if they enter text, click away, then the 'x' disappears, then they want to clear it, so then they click in the field, and then it appears? that is non-intuitive.

Good point. I think there is a middle ground here between my first suggestion of making the icon appear on text and Daniel's suggestion of making icon appear on focus for accessibility reasons.

How about

When the input is focused, the X icon appears. When the X icon is focused, the X icon stays appeared. When neither X icon or the input is focused but the text value is non empty, the X icon stays appeared. If neither X icon or the input is focused AND the text value is empty, the X icon disappears.

@mavarius @jrenee42 thoughts?

jrenee42 · 2021-07-08T18:43:57Z

having it appear sometimes/not others may make styling tricky.

right now, the button always appears if the button is 'enabled' for the field.

#616

When the input is focused, the X icon appears. When the X icon is focused, the X icon stays appeared. When neither the X icon or the input is focused, the X icon does not exist.

then how will the users discover this feature? what if they enter text, click away, then the 'x' disappears, then they want to clear it, so then they click in the field, and then it appears? that is non-intuitive.

Good point. I think there is a middle ground here between my first suggestion of making the icon appear on text and Daniel's suggestion of making icon appear on focus for accessibility reasons.

How about

When the input is focused, the X icon appears. When the X icon is focused, the X icon stays appeared. When neither X icon or the input is focused but the text value is non empty, the X icon stays appeared. If neither X icon or the input is focused AND the text value is empty, the X icon disappears.

@mavarius @jrenee42 thoughts?

jrenee42 · 2021-07-08T18:46:40Z

having it appear sometimes/not others may make styling tricky.

right now, the button always appears if the button is 'enabled' for the field.

#616

When the input is focused, the X icon appears. When the X icon is focused, the X icon stays appeared. When neither the X icon or the input is focused, the X icon does not exist.

then how will the users discover this feature? what if they enter text, click away, then the 'x' disappears, then they want to clear it, so then they click in the field, and then it appears? that is non-intuitive.

Good point. I think there is a middle ground here between my first suggestion of making the icon appear on text and Daniel's suggestion of making icon appear on focus for accessibility reasons.
How about

When the input is focused, the X icon appears. When the X icon is focused, the X icon stays appeared. When neither X icon or the input is focused but the text value is non empty, the X icon stays appeared. If neither X icon or the input is focused AND the text value is empty, the X icon disappears.

@mavarius @jrenee42 thoughts?

it actually looks ok to have the button hide when there is no content.
(the code and styling part is trivial).

daviddkkim · 2021-07-13T20:43:50Z

This looks almost ready @jrenee42.

The styling is implemented slightly different than our input components with an icon. See

This isn't a huge issue but I'd want to keep consistency.

I'd want to see what other input's icon sizes are for each respective input size. I think for medium, we see 14 or 16 px. This one is 13.44px. It looks like the width of the strokes in the X is determined by the height, if that's the case I think 1px or 2px would work best. We use 2px border radius throughout our styles i believe.

For hover state, active/focused state of the style,
Please take a look at the visibility input. I think the default state is fine as it is (except, instead of using #ffffff and opacity, I'd settle on an actual color in our design token) but the hover state and focus/active states of the dismiss button goes against how we style.

I'm happy to make these finishing touches/style changes but I am occupied with million other tasks at the moment and won't be able to get around to it until I get some breathing room!

jrenee42 · 2021-07-13T23:01:54Z

This looks almost ready @jrenee42.

The styling is implemented slightly different than our input components with an icon. See

This isn't a huge issue but I'd want to keep consistency.

I'd want to see what other input's icon sizes are for each respective input size. I think for medium, we see 14 or 16 px. This one is 13.44px. It looks like the width of the strokes in the X is determined by the height, if that's the case I think 1px or 2px would work best. We use 2px border radius throughout our styles i believe.

For hover state, active/focused state of the style,
Please take a look at the visibility input. I think the default state is fine as it is (except, instead of using #ffffff and opacity, I'd settle on an actual color in our design token) but the hover state and focus/active states of the dismiss button goes against how we style.

I'm happy to make these finishing touches/style changes but I am occupied with million other tasks at the moment and won't be able to get around to it until I get some breathing room!

ok. my concerns are that the dismiss button is a part of clockface, so maybe that needs changing too for consistency,
and having it be a button (other than an icon) means that it can accept tabs and focus for accessibility.

I will talk to Kristina and find out the priority on this (vs. other bugs/issues). thanks for the review!

daviddkkim · 2021-07-26T23:16:32Z

This looks almost ready @jrenee42.
The styling is implemented slightly different than our input components with an icon. See

This isn't a huge issue but I'd want to keep consistency.

I'd want to see what other input's icon sizes are for each respective input size. I think for medium, we see 14 or 16 px. This one is 13.44px. It looks like the width of the strokes in the X is determined by the height, if that's the case I think 1px or 2px would work best. We use 2px border radius throughout our styles i believe.
For hover state, active/focused state of the style,
Please take a look at the visibility input. I think the default state is fine as it is (except, instead of using #ffffff and opacity, I'd settle on an actual color in our design token) but the hover state and focus/active states of the dismiss button goes against how we style.

I'm happy to make these finishing touches/style changes but I am occupied with million other tasks at the moment and won't be able to get around to it until I get some breathing room!

ok. my concerns are that the dismiss button is a part of clockface, so maybe that needs changing too for consistency,
and having it be a button (other than an icon) means that it can accept tabs and focus for accessibility.

I will talk to Kristina and find out the priority on this (vs. other bugs/issues). thanks for the review!

@jrenee42
I think the color issue can be solved by just adding
color={ComponentColor.Tertiary}
to the dismiss button.

As for the dismiss button, yeah the dismiss button wasn't designed to be used inside of an input. That's why you won't find it usable for the input out of the box. The hardcoded height,width you have seems to not align with the icon sizes we use for inputs.

Thing to change:

Color to tertiary for dismiss button for a more muted look.
Icon sizes for input, extrasmall = 12x12, small = 15x 15 , medium = 16 x 16 , large = 20 x 20.
all the hard coded values, try to find the appropriate design tokens in the variables scss file.

daviddkkim · 2021-07-26T23:17:37Z

I can make this change or you can. I think I might have some time later in the week to tackle some of these but since this is not a priority for me, my attention on it will be sparse for full transparency.

daviddkkim

lgtm

jrenee42 requested review from zoesteinkamp and daviddkkim May 3, 2021 21:56

daviddkkim suggested changes May 3, 2021

View reviewed changes

daviddkkim requested a review from mavarius May 3, 2021 22:50

mavarius reviewed May 3, 2021

View reviewed changes

jrenee42 requested review from juliajames12 and a team as code owners July 8, 2021 18:36

feat: adding optional clearable 'x' button to input text fields

ee2a911

jrenee42 force-pushed the jill-wip/clearableButton branch from 0204e06 to ee2a911 Compare October 19, 2021 15:46

jrenee42 requested review from daviddkkim and removed request for zoesteinkamp October 19, 2021 15:58

daviddkkim approved these changes Oct 19, 2021

View reviewed changes

jrenee42 merged commit e8d3d7f into master Oct 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jill wip/clearable button #617

Jill wip/clearable button #617

jrenee42 commented May 3, 2021 •

edited

Loading

daviddkkim left a comment

daviddkkim May 3, 2021

daviddkkim May 3, 2021

jrenee42 May 4, 2021

daviddkkim May 4, 2021

daviddkkim May 3, 2021

daviddkkim May 3, 2021

daviddkkim May 3, 2021 •

edited

Loading

jrenee42 May 4, 2021

daviddkkim May 3, 2021

jrenee42 Jul 8, 2021

daviddkkim May 3, 2021

jrenee42 May 4, 2021

jrenee42 Jul 8, 2021

daviddkkim May 3, 2021

mavarius left a comment

mavarius May 3, 2021

jrenee42 May 4, 2021

mavarius May 3, 2021

jrenee42 commented May 4, 2021

daviddkkim commented May 4, 2021

jrenee42 commented Jul 8, 2021

jrenee42 commented Jul 8, 2021

daviddkkim commented Jul 13, 2021 •

edited

Loading

jrenee42 commented Jul 13, 2021

daviddkkim commented Jul 26, 2021 •

edited

Loading

daviddkkim commented Jul 26, 2021

daviddkkim left a comment

Jill wip/clearable button #617

Jill wip/clearable button #617

Conversation

jrenee42 commented May 3, 2021 • edited Loading

Changes

Screenshots

Checklist

daviddkkim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daviddkkim May 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mavarius left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrenee42 commented May 4, 2021

daviddkkim commented May 4, 2021

jrenee42 commented Jul 8, 2021

jrenee42 commented Jul 8, 2021

daviddkkim commented Jul 13, 2021 • edited Loading

jrenee42 commented Jul 13, 2021

daviddkkim commented Jul 26, 2021 • edited Loading

daviddkkim commented Jul 26, 2021

daviddkkim left a comment

Choose a reason for hiding this comment

jrenee42 commented May 3, 2021 •

edited

Loading

daviddkkim May 3, 2021 •

edited

Loading

daviddkkim commented Jul 13, 2021 •

edited

Loading

daviddkkim commented Jul 26, 2021 •

edited

Loading