Skip to content
This repository was archived by the owner on May 24, 2024. It is now read-only.

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

Merged
merged 18 commits into from
Jul 23, 2020
Merged

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

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
16a4113
removed placeholder prop from input
pranav300 Jul 10, 2020
1928ceb
removed placeholder prop from textarea
pranav300 Jul 10, 2020
49aacf9
Removed/Updated tests and examples
pranav300 Jul 12, 2020
58346bf
Updated search field due to changes in form-input
pranav300 Jul 12, 2020
895b581
Merge branch 'main' into remove-placeholder-from-input-and-textarea
pranav300 Jul 12, 2020
9241997
minor changes
pranav300 Jul 13, 2020
59fde03
Merge branch 'main' into remove-placeholder-from-input-and-textarea
pranav300 Jul 13, 2020
5a472c7
Merge branch 'main' into remove-placeholder-from-input-and-textarea
pranav300 Jul 13, 2020
f132725
minor changes
pranav300 Jul 14, 2020
c212ae8
updated documentation
pranav300 Jul 14, 2020
cac5390
Merge branch 'main' into remove-placeholder-from-input-and-textarea
pranav300 Jul 14, 2020
81fdeaf
minor changes
pranav300 Jul 14, 2020
08617ed
Merge branch 'main' of https://github.com/cerner/terra-core into remo…
pranav300 Jul 14, 2020
11d9810
Merge branch 'remove-placeholder-from-input-and-textarea' of https://…
pranav300 Jul 14, 2020
ad81efd
comments
pranav300 Jul 20, 2020
dc8a375
Merge branch 'main' into remove-placeholder-from-input-and-textarea
mjhenkes Jul 22, 2020
6c6adda
Merge branch 'main' of https://github.com/cerner/terra-core into remo…
pranav300 Jul 22, 2020
cc227eb
reverted changes made to search-field
pranav300 Jul 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/terra-form-input/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,19 @@ ChangeLog

Unreleased
----------
### Breaking Changes
* `placeholder` prop has been deprecated.

### Changed
* Updated UpgradeGuide
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update here ad81efd.

* Updated tests and examples to not use `placeholder prop

### Removed
* Removed themeable variable --terra-form-input-placeholder-color
* Removed themeable variable --terra-form-input-placeholder-font-style
* Removed themeable variable --terra-form-input-placeholder-disabled-color
* Removed themeable variable --terra-form-input-placeholder-disabled-font-style
* Removed tests and examples specifically for `placeholder` prop

3.7.0 - (July 7, 2020)
------------------
Expand Down
11 changes: 4 additions & 7 deletions packages/terra-form-input/src/Input.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ const propTypes = {
* NOTE: The pattern attribute works with the following input types: text, date, search, url, tel, email, and password.
*/
pattern: PropTypes.string,
/**
* Placeholder text.
*/
placeholder: PropTypes.string,
/**
* Callback ref to pass into the input dom element.
*/
Expand Down Expand Up @@ -90,7 +86,6 @@ const defaultProps = {
onFocus: undefined,
name: null,
pattern: undefined,
placeholder: undefined,
required: false,
refCallback: undefined,
type: undefined,
Expand All @@ -109,7 +104,6 @@ class Input extends React.Component {
onFocus,
name,
pattern,
placeholder,
refCallback,
required,
type,
Expand Down Expand Up @@ -154,6 +148,10 @@ class Input extends React.Component {
attributes.defaultValue = defaultValue;
}

if (attributes.placeholder) {
attributes.placeholder = null;
}
Comment on lines +151 to +153
Copy link
Contributor

@StephenEsser StephenEsser Jul 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@pranav300 pranav300 Jul 14, 2020 •
edited by neilpfeiffer
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@neilpfeiffer neilpfeiffer Jul 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


return (
<input
{...attributes}
Expand All @@ -163,7 +161,6 @@ class Input extends React.Component {
name={name}
type={type}
pattern={pattern}
placeholder={placeholder}
onBlur={onBlur}
onChange={onChange}
onFocus={onFocus}
Expand Down
11 changes: 0 additions & 11 deletions packages/terra-form-input/src/Input.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@
transition-timing-function: var(--terra-form-input-transition-timing-function, ease);
width: 100%;

&::placeholder {
color: var(--terra-form-input-placeholder-color, rgba(25, 32, 35, 0.53));
font-style: var(--terra-form-input-placeholder-font-style);
opacity: 1;
}

&:hover {
background-color: var(--terra-form-input-hover-background-color, #fff);
border: var(--terra-form-input-hover-border, 1px solid #dedfe0);
Expand All @@ -51,11 +45,6 @@
pointer-events: none;
}

&[disabled]::placeholder {
color: var(--terra-form-input-placeholder-disabled-color, rgba(25, 32, 35, 0.53));
font-style: var(--terra-form-input-placeholder-disabled-font-style);
}

&:focus {
background-color: var(--terra-form-input-focus-background-color, #fff);
background-size: var(--terra-form-input-focus-background-size);
Expand Down
7 changes: 0 additions & 7 deletions packages/terra-form-input/src/InputField.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ const propTypes = {
* Function to trigger when user changes the input value. Provide a function to create a controlled input.
*/
onChange: PropTypes.func,
/**
* Placeholder text.
*/
placeholder: PropTypes.string,
/**
* Ref callback to pass into the ref attribute of the html input element.
*/
Expand Down Expand Up @@ -118,7 +114,6 @@ const defaultProps = {
isLabelHidden: false,
labelAttrs: {},
onChange: undefined,
placeholder: undefined,
maxWidth: undefined,
refCallback: undefined,
required: false,
Expand All @@ -145,7 +140,6 @@ const InputField = (props) => {
labelAttrs,
maxWidth,
onChange,
placeholder,
Copy link
Contributor

@benbcai benbcai Jul 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update here ad81efd.

refCallback,
required,
showOptional,
Expand Down Expand Up @@ -194,7 +188,6 @@ const InputField = (props) => {
isIncomplete={isIncomplete}
type={inputType}
onChange={onChange}
placeholder={placeholder || inputAttrs.placeholder}
value={value}
defaultValue={defaultValue}
refCallback={refCallback}
Expand Down
5 changes: 0 additions & 5 deletions packages/terra-form-input/src/clinical-lowlight-theme/Input.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
--terra-form-input-disabled-border: 1px solid #242b2b;
--terra-form-input-disabled-color: #404344;
--terra-form-input-disabled-opacity: 0.25;
--terra-form-input-placeholder-disabled-font-style: normal;

--terra-form-input-focus-background-color: #222a2e;
--terra-form-input-focus-border: 1px solid #004c76;
Expand All @@ -34,10 +33,6 @@
--terra-form-input-error-focus-border: 1px solid #004c76;
--terra-form-input-error-hover-border: 1px solid #181b1d;

--terra-form-input-placeholder-color: #6f7477;
--terra-form-input-placeholder-disabled-color: #6f7477;
--terra-form-input-placeholder-font-style: normal;

--terra-form-input-background-image: none;
--terra-form-input-background-size: auto;

Expand Down
8 changes: 0 additions & 8 deletions packages/terra-form-input/src/orion-fusion-theme/Input.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,6 @@
--terra-form-input-focus-ltr-box-shadow: 0 0 8px #a6d9f4;
--terra-form-input-focus-rtl-box-shadow: 0 0 8px #a6d9f4;

// Placeholder
--terra-form-input-placeholder-color: #767676;
--terra-form-input-placeholder-font-style: normal;

// Placeholder - Disabled
--terra-form-input-placeholder-disabled-color: #c4c6c7;
--terra-form-input-placeholder-disabled-font-style: normal;

// Disabled
--terra-form-input-disabled-background-color: #f4f4f4;
--terra-form-input-disabled-border: 1px solid #b2b5b6;
Expand Down
2 changes: 1 addition & 1 deletion packages/terra-form-input/src/terra-dev-site/doc/common/NumberInputExample.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example is already using the FormField component

Copy link
Member

@neilpfeiffer neilpfeiffer Jul 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

</Field>
</div>
);
Expand Down
13 changes: 0 additions & 13 deletions packages/terra-form-input/src/terra-dev-site/doc/common/PlaceholderExample.jsx
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion packages/terra-form-input/src/terra-dev-site/doc/example/InputField.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const DefaultInputField = () => (
label="Profile Name"
help="Note: This can not be changed in the future"
type="text"
placeholder="Profile Name"
inputAttrs={{
name: 'profile',
}}
Expand Down
2 changes: 1 addition & 1 deletion packages/terra-form-input/src/terra-dev-site/doc/example/NumberInputField.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const NumberInputField = () => (
inputId="numeric-input"
label="Numeric Value"
type="number"
placeholder="Enter Digits"
help="Enter Digits"
inputAttrs={{
name: 'numeric',
}}
Expand Down
2 changes: 0 additions & 2 deletions packages/terra-form-input/src/terra-dev-site/doc/form-input/Input.1.doc.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Badge } from 'terra-form-input/package.json?dev-site-package';

import BlankExample from '../common/BlankExample?dev-site-example';
import PlaceholderExample from '../common/PlaceholderExample?dev-site-example';
import NumberInputExample from '../common/NumberInputExample?dev-site-example';
import ControlledDefaultExample from '../example/controlled/DefaultExample?dev-site-example';
import ControlledDisabledExample from '../example/controlled/DisabledExample?dev-site-example';
Expand Down Expand Up @@ -55,7 +54,6 @@ import Input from 'terra-form-input';

## Examples
<BlankExample title="Default Input" />
<PlaceholderExample title="Placeholder Input" />
<NumberInputExample title="Numeric Input" />
<ControlledDefaultExample title="Controlled Example - Valid" />
<ControlledDisabledExample title="Controlled Example - Valid Disabled" />
Expand Down
21 changes: 21 additions & 0 deletions packages/terra-form-input/src/terra-dev-site/doc/form-input/UpgradeGuide.3.doc.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,27 @@ import { Badge } from 'terra-form-input/package.json?dev-site-package';
<Badge />

# Terra Form Input Upgrade Guide
## Changes from version 3 to version 4

### Props

#### Removed
* Removed `placeholder` prop from `Input` and `InputField`.

### Changes to CSS Custom Properties

#### Removed
* --terra-form-input-placeholder-color
* --terra-form-input-placeholder-font-style
* --terra-form-input-placeholder-disabled-color
* --terra-form-input-placeholder-disabled-font-style

##### Steps to Upgrade
To upgrade from 3.0 to 4.0 each existing Input and InputField must be reviewed and updated to ensure no `placeholder` prop is passed to it. This is done to follow WCAG ([for more info](https://www.w3.org/WAI/GL/low-vision-a11y-tf/wiki/Placeholder_Research)).

* `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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update here ad81efd.


## Changes from version 2 to version 3

Expand Down
14 changes: 0 additions & 14 deletions ...ges/terra-form-input/src/terra-dev-site/test/form-input/DisabledPlaceholderInput.test.jsx
View file
Open in desktop

This file was deleted.

14 changes: 0 additions & 14 deletions packages/terra-form-input/src/terra-dev-site/test/form-input/PlaceholderFormInput.test.jsx
View file
Open in desktop

This file was deleted.

6 changes: 0 additions & 6 deletions packages/terra-form-input/tests/jest/Input.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ describe('Input', () => {
expect(wrapper).toMatchSnapshot();
});

it('should render w/ placeholder text when just a placeholder prop is passed into the Input', () => {
const input = <Input placeholder="foo" ariaLabel="label" />;
const wrapper = shallow(input);
expect(wrapper).toMatchSnapshot();
});

it('should render as controlled when just a default value and `onChange()` is passed into the Input', () => {
const input = <Input value="foo" onChange={() => { }} />;
const wrapper = shallow(input);
Expand Down
12 changes: 0 additions & 12 deletions packages/terra-form-input/tests/jest/InputField.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,16 +145,4 @@ describe('InputField', () => {
const wrapper = shallow(textarea);
expect(wrapper).toMatchSnapshot();
});

it('should render a placeholder within the InputField component', () => {
const textarea = <InputField inputId="test-input" label="Label" placeholder="Placeholder" />;
const wrapper = shallow(textarea);
expect(wrapper).toMatchSnapshot();
});

it('should render a placeholder within the InputField component when passed as an input attribute', () => {
const textarea = <InputField inputId="test-input" label="Label" inputAttrs={{ placeholder: 'Placeholder' }} />;
const wrapper = shallow(textarea);
expect(wrapper).toMatchSnapshot();
});
});
11 changes: 0 additions & 11 deletions packages/terra-form-input/tests/jest/__snapshots__/Input.test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,6 @@ exports[`Input should render as uncontrolled when just a default value is passed
/>
`;

exports[`Input should render w/ placeholder text when just a placeholder prop is passed into the Input 1`] = `
<input
aria-label="label"
className="form-input"
disabled={false}
name={null}
placeholder="foo"
required={false}
/>
`;

exports[`Input should render with a type of password and pattern prop value 1`] = `
<input
aria-label="label"
Expand Down
64 changes: 0 additions & 64 deletions packages/terra-form-input/tests/jest/__snapshots__/InputField.test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,70 +216,6 @@ exports[`InputField should render a disabled InputField component via inputAttrs
</Field>
`;

exports[`InputField should render a placeholder within the InputField component 1`] = `
<Field
error={null}
errorIcon={
<IconError
viewBox="0 0 48 48"
xmlns="http://www.w3.org/2000/svg"
/>
}
help={null}
hideRequired={false}
htmlFor="test-input"
isInline={false}
isInvalid={false}
isLabelHidden={false}
label="Label"
labelAttrs={Object {}}
required={false}
showOptional={false}
>
<Input
disabled={false}
id="test-input"
isIncomplete={false}
isInvalid={false}
name={null}
placeholder="Placeholder"
required={false}
/>
</Field>
`;

exports[`InputField should render a placeholder within the InputField component when passed as an input attribute 1`] = `
<Field
error={null}
errorIcon={
<IconError
viewBox="0 0 48 48"
xmlns="http://www.w3.org/2000/svg"
/>
}
help={null}
hideRequired={false}
htmlFor="test-input"
isInline={false}
isInvalid={false}
isLabelHidden={false}
label="Label"
labelAttrs={Object {}}
required={false}
showOptional={false}
>
<Input
disabled={false}
id="test-input"
isIncomplete={false}
isInvalid={false}
name={null}
placeholder="Placeholder"
required={false}
/>
</Field>
`;

exports[`InputField should render a valid InputField with props 1`] = `
<Field
error="Text"
Expand Down
Binary file removed ...owlight-theme/en/chrome_large/form-input-spec/Disabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...lowlight-theme/en/chrome_large/form-input-spec/Enabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...lowlight-theme/en/chrome_tiny/form-input-spec/Disabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...-lowlight-theme/en/chrome_tiny/form-input-spec/Enabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...s__/reference/en/chrome_large/form-input-spec/Disabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...ts__/reference/en/chrome_large/form-input-spec/Enabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...ts__/reference/en/chrome_tiny/form-input-spec/Disabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...ots__/reference/en/chrome_tiny/form-input-spec/Enabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...-fusion-theme/en/chrome_large/form-input-spec/Disabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...n-fusion-theme/en/chrome_large/form-input-spec/Enabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...n-fusion-theme/en/chrome_tiny/form-input-spec/Disabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Binary file removed ...on-fusion-theme/en/chrome_tiny/form-input-spec/Enabled_Placeholder[default].png
View file
Open in desktop
Binary file not shown.
Loading