[terra-form-input][terra-form-textarea] Remove prop placeholder #3075

pranav300 · 2020-07-12T20:50:32Z

Summary

This PR:

Deprecates prop placeholder from form-input and form-textarea
Removes/updates tests and examples using prop placeholder
Removes themeable variables:
** --terra-form-input-placeholder-color
** --terra-form-input-placeholder-font-style
** --terra-form-input-placeholder-disabled-color
** --terra-form-input-placeholder-disabled-font-style
** --terra-form-textarea-placeholder-color
** --terra-form-textarea-placeholder-font-style
** --terra-form-textarea-placeholder-disabled-color
** --terra-form-textarea-placeholder-disabled-font-style
Updates wdio snapshots in search-field for changes made in form-input

Closes #2992 along with #3078

Deployment Link

https://terra-core-deployed-pr-3075.herokuapp.com/

Testing

Additional Details

Thank you for contributing to Terra.
@cerner/terra

packages/terra-form-input/CHANGELOG.md

StephenEsser · 2020-07-13T20:36:42Z

packages/terra-form-input/src/Input.jsx

+    if (attributes.placeholder) {
+      attributes.placeholder = null;
+    }


This is a more aggressive approach than we usually take. Generally we allow consumers to pass native attributes through the custom props even if they are not officially supported/listed on the documentation site. I think we'll need to document the input does not allow a placeholder (and why) since this is an explicit/strict detail. Otherwise we'll want to allow the placeholder to pass through as a custom prop, but not provide any first class API for it/log a warning if it is detected.

I have added a Note on the doc pages for the component not allowing placeholder: c212ae8

@StephenEsser this is a special case where UX intentionally not only wants to discourage use of placeholders, but outright disallow it as an accessibility best practice so consumers aren't still reliant on using them, particularly if they don't have direct support from UX and able to get guidance. Documentation will be the best approach, I think as we continue to improve the docsite, having standout sections similar to Mozilla and W3C that calls out special details or best practice notes will be a good place to call this out.

+1 to being heavy handed. Let's document that this is the UX requirement though and that a passed in placeholder will be dropped.

StephenEsser · 2020-07-20T15:38:14Z

packages/terra-form-input/CHANGELOG.md

+* Removed `placeholder` prop.
+
+### Changed
+* Updated UpgradeGuide


We should provide more detail about what the change to the upgrade guide implies. What was updated/added/changed?

Update here ad81efd.

StephenEsser · 2020-07-20T15:45:45Z

packages/terra-form-input/src/terra-dev-site/doc/common/NumberInputExample.jsx

@@ -5,7 +5,7 @@ import Input from 'terra-form-input';
 const NumberInputExample = () => (
  <div>
    <Field label="Numeric Input" htmlFor="numeric">
-      <Input name="number input" placeholder="Enter Digits" id="numeric" type="number" ariaLabel="Numeric Input" />
+      <Input name="number input" id="numeric" type="number" ariaLabel="Numeric Input" />


The number example in input field was updated to provide help text when the placeholder was removed, we should probably to the same here

I don't think this will necessary as this may mislead users to use Input and FormField as separate components rather than directly consuming InputField.

The example is already using the FormField component

Thanks Stephen, I made this new issue #3084 to remove the Fields as part of the raw examples for our text-input, so users do not get confused what they are seeing and testing on this page. The use of Field here is now duplicative, since these example originally did not have the example template housing it, which now provides its own title and descriptions.

As part of our best practice UX Audits, it was identified that UX would like to showcase the InputField and other Field componsitions as the primary form components, and then either move or segment raw building block components, like the solo input element component, into a separate location. See: UX Audit: terra-form-input

StephenEsser · 2020-07-20T15:51:40Z

packages/terra-form-input/src/terra-dev-site/doc/form-input/Input.1.doc.mdx

@@ -24,6 +23,8 @@ import FormInputPropsTable from 'terra-form-input/src/Input?dev-site-props-table

 Generic input which represents an HTML input element directly.

+**Note:** To follow UX Best Practices, this component does not allow `placeholder`.


At some point we should have a writeup about why we made this decision and link out to it here (and anywhere else this note is written)

Added link to w3 Placeholder Research in Note section.

StephenEsser · 2020-07-20T16:02:24Z

packages/terra-form-input/src/terra-dev-site/doc/form-input/UpgradeGuide.3.doc.mdx

+* `Input` component will not be supporting `placeholder` prop and `native placeholder`. If consumers want to provide a detailed text guidance they can upgrade to `InputField`.
+
+* `InputField` component will not be supporting `placeholder` prop and `native placeholder`. But consumers can provide a help text using the `help` prop.


We should provide more guidance on what the upgrade entails for these sections.

If consumers want to provide a detailed text guidance they can upgrade to InputField .....

After switching to an InputField a consumer may utilize the help text field properties to provide similar guidance to the user. If we have any UX design standards/recommendations this may be a good place to mention a few of those details.

Update here ad81efd.

StephenEsser

+1 on the code changes. I think adding some detailed documentation prior to releasing will help users transition and understand the changes.

benbcai · 2020-07-20T16:57:28Z

packages/terra-form-input/src/InputField.jsx

@@ -145,7 +140,6 @@ const InputField = (props) => {
    labelAttrs,
    maxWidth,
    onChange,
-    placeholder,


Should placeholder also be deleted from the customProps just like in Input.jsx if we outright disallow it? Otherwise, it would get spread onto FormField.

Update here ad81efd.

tbiethman

I think this'll be good to go once Ben's comment is addressed.

benbcai · 2020-07-20T18:32:34Z

packages/terra-form-textarea/src/TextareaField.jsx

@@ -137,7 +132,6 @@ const TextareaField = (props) => {
    required,
    showOptional,
    onChange,
-    placeholder,


Same as this comment: #3075 (comment)

Update here ad81efd

pranav300 · 2020-07-20T20:14:56Z

Do not merge this PR until placeholder has been removed from search-field(PR #3078)

neilpfeiffer · 2020-07-22T21:00:49Z

Tested: PR #3075
Resolves: Issue #2992 along with #3078

Reviewed:

Follows pattern considerations documented in Cerner Design Standards: Text Input, Text Area and Input Field
Follows UX defined design implementation for Input & Textarea
Follows defined theme designs: terra-default-theme, orion-fusion-theme, clinical-lowlight-theme

Related Updates (also will be updating usage of placeholders):
terra-core: issue #3083
terra-clinical: issue #680
terra-framework: issue #1193

Verified by @neilpfeiffer, ready to merge after #3078,

Pass
UX Review

…ve-placeholder-from-input-and-textarea

pranav300 added 4 commits July 10, 2020 12:35

removed placeholder prop from input

16a4113

removed placeholder prop from textarea

1928ceb

Removed/Updated tests and examples

49aacf9

Updated search field due to changes in form-input

58346bf

pranav300 added Major Version Bump 📦 terra-form-textarea 📦 terra-form-input labels Jul 12, 2020

pranav300 requested review from neilpfeiffer, supreethmr, lokesh-0813, saket2403, ShettyAkarsh and avinashg1994 July 12, 2020 20:50

pranav300 requested review from dkasper-was-taken, mjhenkes, ryanthemanuel and tbiethman as code owners July 12, 2020 20:50

pranav300 self-assigned this Jul 12, 2020

Merge branch 'main' into remove-placeholder-from-input-and-textarea

895b581

mjhenkes temporarily deployed to terra-core-deployed-pr-3075 July 12, 2020 20:50 Inactive

pranav300 added 3 commits July 13, 2020 09:55

minor changes

9241997

Merge branch 'main' into remove-placeholder-from-input-and-textarea

59fde03

Merge branch 'main' into remove-placeholder-from-input-and-textarea

5a472c7

mjhenkes temporarily deployed to terra-core-deployed-pr-3075 July 13, 2020 19:39 Inactive

StephenEsser reviewed Jul 13, 2020

View reviewed changes

packages/terra-form-input/CHANGELOG.md Outdated Show resolved Hide resolved

StephenEsser reviewed Jul 13, 2020

View reviewed changes

packages/terra-form-input/CHANGELOG.md Outdated Show resolved Hide resolved

StephenEsser reviewed Jul 13, 2020

View reviewed changes

minor changes

f132725

mjhenkes temporarily deployed to terra-core-deployed-pr-3075 July 14, 2020 08:10 Inactive

updated documentation

c212ae8

StephenEsser reviewed Jul 20, 2020

View reviewed changes

StephenEsser approved these changes Jul 20, 2020

View reviewed changes

benbcai reviewed Jul 20, 2020

View reviewed changes

tbiethman suggested changes Jul 20, 2020

View reviewed changes

benbcai reviewed Jul 20, 2020

View reviewed changes

comments

ad81efd

mjhenkes temporarily deployed to terra-core-deployed-pr-3075 July 20, 2020 19:57 Inactive

pranav300 added the do not merge label Jul 20, 2020

benbcai approved these changes Jul 20, 2020

View reviewed changes

ShettyAkarsh approved these changes Jul 21, 2020

View reviewed changes

tbiethman approved these changes Jul 22, 2020

View reviewed changes

Merge branch 'main' into remove-placeholder-from-input-and-textarea

dc8a375

mjhenkes temporarily deployed to terra-core-deployed-pr-3075 July 22, 2020 17:51 Inactive

ryanthemanuel added the UX Review Ready label Jul 22, 2020

neilpfeiffer mentioned this pull request Jul 22, 2020

[form-input, form-textarea] update examples to remove field #3084

Closed

neilpfeiffer added ⭐ UX Reviewed UX Reviewed and approved. and removed UX Review Ready labels Jul 22, 2020

neilpfeiffer approved these changes Jul 22, 2020

View reviewed changes

pranav300 added 2 commits July 23, 2020 04:10

Merge branch 'main' of https://github.com/cerner/terra-core into remo…

6c6adda

…ve-placeholder-from-input-and-textarea

reverted changes made to search-field

cc227eb

mjhenkes temporarily deployed to terra-core-deployed-pr-3075 July 22, 2020 22:42 Inactive

pranav300 removed UX Review Required do not merge labels Jul 22, 2020

mjhenkes merged commit 9d2d436 into main Jul 23, 2020

mjhenkes deleted the remove-placeholder-from-input-and-textarea branch July 23, 2020 13:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[terra-form-input][terra-form-textarea] Remove prop placeholder #3075

[terra-form-input][terra-form-textarea] Remove prop placeholder #3075

pranav300 commented Jul 12, 2020 •

edited by neilpfeiffer

Loading

StephenEsser Jul 13, 2020 •

edited

Loading

pranav300 Jul 14, 2020 •

edited by neilpfeiffer

Loading

neilpfeiffer Jul 15, 2020 •

edited

Loading

ryanthemanuel Jul 15, 2020

StephenEsser Jul 20, 2020

pranav300 Jul 20, 2020

StephenEsser Jul 20, 2020

pranav300 Jul 20, 2020

StephenEsser Jul 20, 2020

neilpfeiffer Jul 22, 2020 •

edited

Loading

StephenEsser Jul 20, 2020 •

edited

Loading

pranav300 Jul 20, 2020

StephenEsser Jul 20, 2020

pranav300 Jul 20, 2020

StephenEsser left a comment •

edited

Loading

benbcai Jul 20, 2020 •

edited

Loading

pranav300 Jul 20, 2020

tbiethman left a comment •

edited

Loading

benbcai Jul 20, 2020

pranav300 Jul 20, 2020

pranav300 commented Jul 20, 2020 •

edited

Loading

neilpfeiffer commented Jul 22, 2020 •

edited

Loading

		@@ -24,6 +23,8 @@ import FormInputPropsTable from 'terra-form-input/src/Input?dev-site-props-table

		Generic input which represents an HTML input element directly.

		Note: To follow UX Best Practices, this component does not allow `placeholder`.

		* `Input` component will not be supporting `placeholder` prop and `native placeholder`. If consumers want to provide a detailed text guidance they can upgrade to `InputField`.

		* `InputField` component will not be supporting `placeholder` prop and `native placeholder`. But consumers can provide a help text using the `help` prop.

[terra-form-input][terra-form-textarea] Remove prop placeholder #3075

[terra-form-input][terra-form-textarea] Remove prop placeholder #3075

Conversation

pranav300 commented Jul 12, 2020 • edited by neilpfeiffer Loading

Summary

Deployment Link

Testing

Additional Details

StephenEsser Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

pranav300 Jul 14, 2020 • edited by neilpfeiffer Loading

Choose a reason for hiding this comment

neilpfeiffer Jul 15, 2020 • 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

neilpfeiffer Jul 22, 2020 • edited Loading

Choose a reason for hiding this comment

StephenEsser Jul 20, 2020 • 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

StephenEsser left a comment • edited Loading

Choose a reason for hiding this comment

benbcai Jul 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbiethman left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranav300 commented Jul 20, 2020 • edited Loading

neilpfeiffer commented Jul 22, 2020 • edited Loading

pranav300 commented Jul 12, 2020 •

edited by neilpfeiffer

Loading

StephenEsser Jul 13, 2020 •

edited

Loading

pranav300 Jul 14, 2020 •

edited by neilpfeiffer

Loading

neilpfeiffer Jul 15, 2020 •

edited

Loading

neilpfeiffer Jul 22, 2020 •

edited

Loading

StephenEsser Jul 20, 2020 •

edited

Loading

StephenEsser left a comment •

edited

Loading

benbcai Jul 20, 2020 •

edited

Loading

tbiethman left a comment •

edited

Loading

pranav300 commented Jul 20, 2020 •

edited

Loading

neilpfeiffer commented Jul 22, 2020 •

edited

Loading