(CDPE-3578) Add show/hide capabilities for <Input>

[eputnam]

(CDPE-3578) Add api for show/hide to
Currently, the trailing and leading icons on the Input component are only icons. This commit makes changes to the Input component api to allow a callback to be passed to the icon to facilitate a password show/hide for example. This includes a boolean to turn the icon into a button and then a prop so that the consumer can pass an onClick to the button.

(CDPE-3578) Fix styles, add interactive example & CHANGELOG
I was having trouble with CSS inheritence and honestly its probably not right still, but I was able to make the leading icon sit all the way to the left finally. This change also adds to the Input.md and adds a CHANGELOG entry for these changes as per CONTRIBUTING.md

[eputnam]

definitely open to thoughts on the api here. these two props (iconButton and onClickIconButton) make it probably as flexible as possible but the thought also occurred to me that we maybe don't want that. what other functions do we image the button having? would it make sense to just have a showHide prop instead?

[woodburndesigns]

Hey @eputnam I definitely see your concern with the API. In this particular case a consumer could potentially add a leading AND trailing button. Currently, if the consumer also provided the onClickIconButton the function would be fired on both elements potentially leading to confusion. Do you know if there have been any cases where folks wanted both leading and trailing icons?

[Sigler]

Hey Eric. Been reviewing this with Geoff and thinking about other use cases without making this too complex.

A few thoughts:

• Let's keep the icon that describes the input separate from the icon used in the button. eg. A key might be used on the left, and the eye might be used for the button.
• It is not intended that the icon on the left, or lead, be clickable. That affords us some wiggle room if we can agree that the trailing side be reserved for actions.
• Other use cases for a more general button would be an "x" to clear all, or a "+" for adding something.
• A nice to have would be flexibility to allow for a text string on the action - such as a "+ Add item". We don't have an immediate use case, thus why I think its nice-to-have only.

[Sigler]

Another thought.

If the trailing side is reserved for actions then we should plan to deprecate the trailing icon prop.
Alternatively we need to account for the position of the trailing icon and the button to be present simultaneously. While that is incredibly flexible it comes at the expense of quite a bit more complexity.

[eputnam]

@Sigler , @woodburndesigns , thanks for the reviews!

So how bout this: (cc: @vine77 )

1. Revert changes to the leading icon so it's back to just being an icon
2. Revert changes to trailing icon and instead add
   trailingButtonIcon: name of the icon for the button. just like "icon" or "trailingIcon" a value here determines whether the component is present.
   trailingButtonText: optional text for the button, we could treat this like the icon prop too. if only text is provided, we would just have a text-only button.
   onClickTrailingButton: callback for the button
3. When the trailing button is present, put the icon to the left of the button (?) using the spacing already built into the components.
4. Add deprecation notice of some kind (open to direction here) to the trailingIcon prop.

[woodburndesigns]

Sounds good to me! Lets go for it for now @eputnam

[Sigler]

Looks good. On your 4th point, we can add an info section about trailingIcons being deprecated in 2.0. We should also emphasize it in the notes and communicate it in the slack channel.

I'm not sure how to best implement it, but it would be nice to be able to pull up a list of everything to be deprecated in 2.0 that we can communicate when we get closer to launch.

[steveax]

Can the button be provided with an aria-label attribute to provide an accessible name for the button? Placeholder is not reliably announced by assistive technology.

[eputnam]

with trailingIcon and trailingButtonIcon

with trailingButtonIcon

with trailingButtonIcon and trailingButtonText

The only snag in this is the icon and button are absolutely positioned, so if you want to have an icon and a button with text (length/width unknown), you're gonna have a bad time. Making that work will mean changing more things and it seems like that's an edge case. If that needs to be an option, let me know.

[woodburndesigns]

Looks good! Thanks for the updates.

[vine77]

I'm merging this, but wanted to include a screenshot here of the issue with absolute positioning when using trailingButtonText: