Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[InputBase] Improve documentation for custom inputComponent #16399

Merged
merged 12 commits into from
Jul 16, 2019
2 changes: 1 addition & 1 deletion docs/src/modules/utils/parseMarkdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const demoRegexp = /^"demo": "(.*)"/;
export function getContents(markdown) {
return markdown
.replace(headerRegExp, '') // Remove header information
.split(/^{{|}}$/gm) // Split markdown into an array, separating demos
.split(/^{{("demo":[^}]*)}}$/gm) // Split markdown into an array, separating demos
Copy link
Member Author

Choose a reason for hiding this comment

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

.filter(content => !emptyRegExp.test(content)); // Remove empty lines
}

Expand Down
26 changes: 26 additions & 0 deletions docs/src/modules/utils/parseMarkdown.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { expect } from 'chai';
import { getContents } from './parseMarkdown';

describe('parseMarkdown', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the kind of test where I would prefer fixtures + snapshots but it's not worth it to set this up for one test. If this test becomes an issue (breaks, readability etc) we can revisit.

describe('getContents', () => {
describe('Split markdown into an array, separating demos', () => {
it('returns a single entry without a demo', () => {
expect(getContents('# SomeGuide\nwhich has no demo')).to.deep.equal([
'# SomeGuide\nwhich has no demo',
]);
});

it('uses a `{{"demo"` marker to split', () => {
expect(
getContents('# SomeGuide\n{{"demo": "GuideDemo.js" }}\n## NextHeading'),
).to.deep.equal(['# SomeGuide\n', '"demo": "GuideDemo.js" ', '\n## NextHeading']);
});

it('ignores possible code', () => {
expect(getContents('# SomeGuide\n```jsx\n<Button props={{\nfoo: 1\n}}')).to.deep.equal([
'# SomeGuide\n```jsx\n<Button props={{\nfoo: 1\n}}',
]);
});
});
});
});
2 changes: 1 addition & 1 deletion docs/src/pages/components/checkboxes/checkboxes.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ In this case, you can apply the additional attribute (e.g. `aria-label`, `aria-l
```jsx
<Checkbox
value="checkedA"
inputProps={{ 'aria-label': 'Checkbox A' } }
inputProps={{ 'aria-label': 'Checkbox A' }}
/>
```

Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/radio-buttons/radio-buttons.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ In this case, you can apply the additional attribute (e.g. `aria-label`, `aria-l
```jsx
<RadioButton
value="radioA"
inputProps={{ 'aria-label': 'Radio A' } }
inputProps={{ 'aria-label': 'Radio A' }}
/>
```

Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/switches/switches.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,6 @@ In this case, you can apply the additional attribute (e.g. `aria-label`, `aria-l
```jsx
<Switch
value="checkedA"
inputProps={{ 'aria-label': 'Switch A' } }
inputProps={{ 'aria-label': 'Switch A' }}
/>
```
41 changes: 37 additions & 4 deletions docs/src/pages/components/text-fields/text-fields.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,50 @@ or
<InputLabel shrink>Count</InputLabel>
```

## Formatted inputs
## Integration with 3rd party input libraries

You can use third-party libraries to format an input.
You have to provide a custom implementation of the `<input>` element with the `inputComponent` property.
The provided input component should handle the `inputRef` property.
The property should be called with a value implementing the [`HTMLInputElement`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement) interface.

The following demo uses the [react-text-mask](https://github.com/text-mask/text-mask) and [react-number-format](https://github.com/s-yadav/react-number-format) libraries.
The following demo uses the [react-text-mask](https://github.com/text-mask/text-mask) and [react-number-format](https://github.com/s-yadav/react-number-format) libraries. The same concept could be applied to [e.g. react-stripe-element](https://github.com/mui-org/material-ui/issues/16037).

{{"demo": "pages/components/text-fields/FormattedInputs.js"}}

The provided input component should handle the `inputRef` property.
The property should be called with a value that implements the following interface:

```ts
interface InputElement {
focus(): void;
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
value?: string;
}
```

```jsx
function MyInputComponent(props) {
const { component: Component, inputRef, ...other } = props;

// implement `InputElement` interface
React.useImperativeHandle(inputRef, () => ({
focus: () => {
// logic to focus the rendered component from 3rd party belongs here
},
// hiding the value e.g. react-stripe-elements
}));

// `Component` will be your `SomeThirdPartyComponent` from below
return <Component {...other} />;
}

// usage
<TextField
InputProps={{
inputComponent: MyInputComponent,
inputProps: { component: SomeThirdPartyComponent },
}}
/>;
```

## Accessibility

In order for the text field to be accessible, **the input should be linked to the label and the helper text**. The underlying DOM nodes should have this structure.
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/customization/components/components.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ compiles to:
classes={{
root: classes.root, // class name, e.g. `root-x`
disabled: classes.disabled, // class name, e.g. `disabled-x`
} }
}}
>
```

Expand Down
4 changes: 2 additions & 2 deletions docs/src/pages/guides/migration-v3/migration-v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ You should be able to move the custom styles to the `root` class key.

```diff
<InputLabel
- FormLabelClasses={{ asterisk: 'bar' } }
+ classes={{ asterisk: 'bar' } }
- FormLabelClasses={{ asterisk: 'bar' }}
+ classes={{ asterisk: 'bar' }}
>
Foo
</InputLabel>
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/styles/advanced/advanced.md
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ If you are using Server-Side Rendering (SSR), you should pass the nonce in the `
<style
id="jss-server-side"
nonce={nonce}
dangerouslySetInnerHTML={{ __html: sheets.toString() } }
dangerouslySetInnerHTML={{ __html: sheets.toString() }}
/>
```

Expand Down
4 changes: 2 additions & 2 deletions docs/src/pages/system/basics/basics.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@ const Box = styled.div`

<Box
p={2}
sm={{ p: 3 } }
md={{ p: 4 } }
sm={{ p: 3 }}
md={{ p: 4 }}
/>

/**
Expand Down
1 change: 1 addition & 0 deletions docs/static/_redirects
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ https://material-ui.dev/* https://material-ui.com/:splat 301!
/r/styles-instance-warning /getting-started/faq/#i-have-several-instances-of-styles-on-the-page 302
/r/caveat-with-refs-guide /guides/composition/#caveat-with-refs 302
/r/pseudo-classes-guide /customization/components/#pseudo-classes 302
/r/input-component-ref-interface /components/text-fields/#integration-with-3rd-party-input-libraries 302

# Legacy
/v0.20.0 https://v0.material-ui.com/v0.20.0
Expand Down
11 changes: 10 additions & 1 deletion packages/material-ui/src/InputBase/InputBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,17 @@ const InputBase = React.forwardRef(function InputBase(props, ref) {

const handleChange = (event, ...args) => {
if (!isControlled) {
const element = event.target || inputRef.current;
if (element == null) {
throw new TypeError(
'Material-UI: Expected valid input target. ' +
'Did you use a custom `inputComponent` and forget to forward refs? ' +
'See https://material-ui.com/r/input-component-ref-interface for more info.',
);
}

checkDirty({
value: (event.target || inputRef.current).value,
value: element.value,
});
}

Expand Down
97 changes: 97 additions & 0 deletions packages/material-ui/src/InputBase/InputBase.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,103 @@ describe('<InputBase />', () => {
expect(typeof injectedProps.onBlur).to.equal('function');
expect(typeof injectedProps.onFocus).to.equal('function');
});

describe('target mock implementations', () => {
it('can just mock the value', () => {
function MockedValue(props) {
const { onChange } = props;

function handleChange(event) {
onChange({ target: { value: event.target.value } });
}

return <input onChange={handleChange} />;
}
MockedValue.propTypes = { onChange: PropTypes.func.isRequired };

function FilledState(props) {
const { filled } = useFormControl();
return <span {...props}>filled: {String(filled)}</span>;
}

const { getByRole, getByTestId } = render(
<FormControl>
<FilledState data-testid="filled" />
<InputBase inputComponent={MockedValue} />
</FormControl>,
);
expect(getByTestId('filled')).to.have.text('filled: false');

fireEvent.change(getByRole('textbox'), { target: { value: 1 } });
expect(getByTestId('filled')).to.have.text('filled: true');
});

it('can expose the full target with `inputRef`', () => {
function FullTarget(props) {
const { inputRef, ...other } = props;

return <input ref={inputRef} {...other} />;
}
FullTarget.propTypes = {
inputRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
};

function FilledState(props) {
const { filled } = useFormControl();
return <span {...props}>filled: {String(filled)}</span>;
}

const { getByRole, getByTestId } = render(
<FormControl>
<FilledState data-testid="filled" />
<InputBase inputComponent={FullTarget} />
</FormControl>,
);
expect(getByTestId('filled')).to.have.text('filled: false');

fireEvent.change(getByRole('textbox'), { target: { value: 1 } });
expect(getByTestId('filled')).to.have.text('filled: true');
});
});

describe('errors', () => {
it('throws on change if the target isnt mocked', () => {
/**
* This component simulates a custom input component that hides the inner
* input value for security reasons e.g. react-stripe-element.
*
* A ref is exposed to trigger a change event instead of using fireEvent.change
*/
function BadInputComponent(props) {
const { onChange, triggerChangeRef } = props;

// simulates const handleChange = () => onChange({}) and passing that
// handler to the onChange prop of `input`
React.useImperativeHandle(triggerChangeRef, () => () => onChange({}));

return <input />;
}
BadInputComponent.propTypes = {
onChange: PropTypes.func.isRequired,
triggerChangeRef: PropTypes.object,
};

const triggerChangeRef = React.createRef();
render(<InputBase inputProps={{ triggerChangeRef }} inputComponent={BadInputComponent} />);

// mocking fireEvent.change(getByRole('textbox'), { target: { value: 1 } });
// using dispatchEvents prevents us from catching the error in the browser
// in test:karma neither try-catch nor consoleErrorMock.spy catches the error
let errorMessage = '';
try {
triggerChangeRef.current();
} catch (error) {
errorMessage = String(error);
}

expect(errorMessage).to.include('Material-UI: Expected valid input target');
});
});
});

describe('with FormControl', () => {
Expand Down