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

[docs] Document ref forwarding (requirements) #15298

Merged
merged 11 commits into from
Apr 14, 2019
2 changes: 1 addition & 1 deletion docs/src/modules/utils/generateMarkdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ function generatePropDescription(prop) {

let notes = '';
if (isElementTypeAcceptingRefProp(type)) {
notes += '<br>[Needs to be able to hold a ref](/guides/composition#caveat-with-refs).';
notes += '<br>⚠️ [Needs to be able to hold a ref](/guides/composition#caveat-with-refs).';
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
}

return `${deprecated}${jsDocText}${signature}${notes}`;
Expand Down
20 changes: 19 additions & 1 deletion docs/src/pages/getting-started/faq/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,25 @@ export default withTheme(withStyles(styles)(Modal));

## How can I access the DOM element?

Wrap the component with the [`RootRef`](/api/root-ref/) helper.
All Material-UI components that should render something in the DOM forward their
ref to the underlying built-in component. This means that you can get DOM elements
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
by reading the ref attached to Material-UI components:

```jsx
// or a ref setter function
const ref = React.createRef();
// render
<Button ref={ref} />;
// usage
const element = ref.current;
```

If you're not sure if the Material-UI component in question forwards its ref you
can check the API documentation under "Props" e.g. the [/api/Button#props](Button API)
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
includes
> The ref is forwarded to the root element.

indicating that you can access the DOM element with a ref.

## Why are the colors I am seeing different from what I see here?

Expand Down
7 changes: 7 additions & 0 deletions docs/src/pages/guides/api/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,10 @@ The Material-UI components use a combination of the two approaches according to
- An *enum* is used when **> 2** degrees of freedom are required, or if there is the possibility that additional degrees of freedom may be required in the future.

Going back to the previous button example; since it requires 3 degrees of freedom, we use an *enum*.

### Ref

The `ref` is forwarded to the root element. This means that, without changing the rendered root element
via the `component` prop, it is forwarded to the outermost DOM element that which component
renders. If you pass a different component via the `component` prop the ref will be attached
to that component instead.
70 changes: 50 additions & 20 deletions docs/src/pages/guides/composition/composition.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,40 +127,70 @@ Here is a demo with [React Router DOM](https://github.com/ReactTraining/react-ro

You can find the details in the [TypeScript guide](/guides/typescript#usage-of-component-property).

### Caveat with refs
## Caveat with refs
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved

Some components such as `ButtonBase` (and therefore `Button`) require access to the
underlying DOM node. This was previously done with `ReactDOM.findDOMNode(this)`.
However `findDOMNode` was deprecated (which disqualifies its usage in React's concurrent mode)
in favour of component refs and ref forwarding.
This section covers caveats when using a custom component as `children` or for the
`component` prop.

It is therefore necessary that the component you pass to the `component` prop
can hold a ref. This includes:
Some of the components need access to the DOM node. This was previously possible
by using `ReactDOM.findDOMNode`. This function is deprecated in favor of `ref` and
ref forwarding. However, only the following component types can be given a `ref`:

- class components
- ref forwarding components (`React.forwardRef`)
- built-in components e.g. `div` or `a`
* Any Material-UI component
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
* class components i.e. `React.Component` or `React.PureComponent`
* built-in (or host) components e.g. `div` or `button`
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
* [React.forwardRef components](https://reactjs.org/docs/react-api.html#reactforwardref)
* [React.lazy components](https://reactjs.org/docs/react-api.html#reactlazy)
* [React.memo components](https://reactjs.org/docs/react-api.html#reactmemo)

If this is not the case we will issue a prop type warning similar to:
If you don't use one of the above types when using your components in conjunction with Material-UI, you might see a warning from
React in your console similar to:
> Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

Be aware that you will still get this warning for `lazy` and `memo` components if their
wrapped component can't hold a ref.

In some instances we issue an additional warning to help debugging, similar to:
> Invalid prop `component` supplied to `ComponentName`. Expected an element type that can hold a ref.

In addition React will issue a warning.

You can fix this warning by using `React.forwardRef`. Learn more about it in
[this section in the official React docs](https://reactjs.org/docs/forwarding-refs.html).
We will only cover the two most common use cases. For more information see [this section in the official React docs](https://reactjs.org/docs/forwarding-refs.html).

```diff
- const MyButton = props => <div role="button" {...props} />
+ const MyButton = React.forwardRef((props, ref) => <div role="button" {...props} ref={ref} />)
<Button component={MyButton} />
```

```diff
- const SomeContent = props => <div {...props}>Hello, World!</div>
+ const SomeContent = React.forwardRef((props, ref) => <div {...props} ref={ref}>Hello, World!</div>)
<Tooltip title="Hello, again.">
```

To find out if the Material-UI component you're using has this requirement, check
out the the props API documentation for that component. If you need to forward refs
the description will link to this section.

### Caveat with StrictMode or unstable_ConcurrentMode

If you pass class components to the `component` prop and don't run in strict mode you won't have to change anything
since we can safely use `ReactDOM.findDOMNode`. For function components, however, you have
to wrap your component in `React.forwardRef`:
If you use class components for the cases described above you will still see
warnings in `React.StrictMode` and `React.unstable_ConcurrentMode`. We use
`ReactDOM.findDOMNode` internally for backwards compatibility. You can use `React.forwardRef`
and a designated prop in your class component to forward the `ref` to a built-in
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
and a designated prop in your class component to forward the `ref` to a built-in
and a designated prop in your class component to forward the `ref` to a DOM element.

?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll be honest I don't know how to properly explain this. Maybe I'm using the term ref wrong maybe it isn't well defined.

If I refer to the term ref then I mean the return value from e.g. React.createRef(). So the ref is the full object { current: instance }. What we end up is attaching the ref object { current: ... } to the host component i.e. <div ref={{ current: ... }} />. Or in other words the ref is the thing you pass to the ref prop1 of a component. You'll probably still catch me doing <Component ref={ref => ...} /> where naming the parameter ref is probably a misnomer. <Component ref={current => ...} /> should be clearer.

The DOM element will then be ref.current. Or more generally ref.current will point to the component instance. For host components rendered with react-dom it will be an instance of a DOM Element.

Please tell me if that doesn't make sense. In the end the most important thing is that we don't confuse the reader. It's not that important to be technically accurate but that one understands how to fix the issue at hand.

1 ref gets a special treatment so calling it a ref might not be 100% accurate.

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
and a designated prop in your class component to forward the `ref` to a built-in
and a designated prop in your class component to forward the `ref` to a DOM

Copy link
Member

Choose a reason for hiding this comment

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

What I found confusing was the terminology "built-in component". What exactly does that refer to?

(I also overlooked the fact that a ref is a pointer to the full object, not just a discrete DOM element, so my suggested replacement wasn't any more helpful.)

component. Doing so should not trigger any more warnings related to the deprecation
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
of `ReactDOM.findDOMNode`.

```diff
- const MyButton = props => <div {...props} />
+ const MyButton = React.forwardRef((props, ref) => <div {...props} ref={ref} />)
<Button component={MyButton} />
class Component extends React.Component {
render() {
- const { props } = this;
+ const { forwardedRef, ...props } = this.props;
return <div {...props} ref={forwardedRef} />;
}
}

-export default Component;
+export default React.forwardRef((props, ref) => <Component {...props} forwardedRef={ref} />);
```

122 changes: 61 additions & 61 deletions docs/src/pages/utils/popper/ScrollPlayground.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { withStyles } from '@material-ui/core/styles';
import FormControlLabel from '@material-ui/core/FormControlLabel';
import MarkdownElement from 'docs/src/modules/components/MarkdownElement';
import Grid from '@material-ui/core/Grid';
import RootRef from '@material-ui/core/RootRef';
import Typography from '@material-ui/core/Typography';
import Button from '@material-ui/core/Button';
import Popper from '@material-ui/core/Popper';
Expand Down Expand Up @@ -178,66 +177,67 @@ class ScrollPlayground extends React.Component {
return (
<div className={classes.root}>
<div className={classes.scrollContainer}>
<RootRef rootRef={this.centerScroll}>
<Grid className={classes.scroll} container alignItems="center" justify="center">
<div>
<Button
buttonRef={node => {
this.anchorEl = node;
}}
variant="contained"
onClick={this.handleClickButton}
aria-describedby={id}
>
Toggle Popper
</Button>
<Typography className={classes.legend}>
Scroll around this container to experiment with flip and preventOverflow
modifiers.
</Typography>
<Popper
id={id}
open={open}
anchorEl={this.anchorEl}
placement={placement}
disablePortal={disablePortal}
className={classes.popper}
modifiers={{
flip: {
enabled: flip,
},
arrow: {
enabled: arrow,
element: arrowRef,
},
preventOverflow: {
enabled: preventOverflow !== 'disabled',
boundariesElement:
preventOverflow === 'disabled' ? 'scrollParent' : preventOverflow,
},
}}
>
{arrow ? <span className={classes.arrow} ref={this.handleArrowRef} /> : null}
<Paper className={classes.paper}>
<DialogTitle>{"Use Google's location service?"}</DialogTitle>
<DialogContent>
<DialogContentText>
Let Google help apps determine location.
</DialogContentText>
</DialogContent>
<DialogActions>
<Button onClick={this.handleClickButton} color="primary">
Disagree
</Button>
<Button onClick={this.handleClickButton} color="primary">
Agree
</Button>
</DialogActions>
</Paper>
</Popper>
</div>
</Grid>
</RootRef>
<Grid
className={classes.scroll}
container
alignItems="center"
justify="center"
ref={this.centerScroll}
>
<div>
<Button
buttonRef={node => {
this.anchorEl = node;
}}
variant="contained"
onClick={this.handleClickButton}
aria-describedby={id}
>
Toggle Popper
</Button>
<Typography className={classes.legend}>
Scroll around this container to experiment with flip and preventOverflow modifiers.
</Typography>
<Popper
id={id}
open={open}
anchorEl={this.anchorEl}
placement={placement}
disablePortal={disablePortal}
className={classes.popper}
modifiers={{
flip: {
enabled: flip,
},
arrow: {
enabled: arrow,
element: arrowRef,
},
preventOverflow: {
enabled: preventOverflow !== 'disabled',
boundariesElement:
preventOverflow === 'disabled' ? 'scrollParent' : preventOverflow,
},
}}
>
{arrow ? <span className={classes.arrow} ref={this.handleArrowRef} /> : null}
<Paper className={classes.paper}>
<DialogTitle>{"Use Google's location service?"}</DialogTitle>
<DialogContent>
<DialogContentText>Let Google help apps determine location.</DialogContentText>
</DialogContent>
<DialogActions>
<Button onClick={this.handleClickButton} color="primary">
Disagree
</Button>
<Button onClick={this.handleClickButton} color="primary">
Agree
</Button>
</DialogActions>
</Paper>
</Popper>
</div>
</Grid>
</div>
<Grid container spacing={2}>
<Grid item xs={12} sm={6}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ function ClickAwayListener(props) {
ClickAwayListener.propTypes = {
/**
* The wrapped element.
*
* ⚠️The component used as a child [must be able to hold a ref](/guides/composition#children).
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
*/
children: PropTypes.element.isRequired,
/**
Expand Down
7 changes: 7 additions & 0 deletions packages/material-ui/src/RootRef/RootRef.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ import { exactProp } from '@material-ui/utils';
import { setRef } from '../utils/reactHelpers';

/**
* ⚠️⚠️⚠️
* If you want the DOM element of a Material-UI component check out
* [/getting-started/faq/#how-can-i-access-the-dom-element](FAQ: How can I access the DOM element?)
* first.
*
* This component uses `findDOMNode` which is deprecated in React.StrictMode.
*
* Helper component to allow attaching a ref to a
* wrapped element to access the underlying DOM element.
*
Expand Down
5 changes: 3 additions & 2 deletions packages/material-ui/src/Slide/Slide.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,9 @@ class Slide extends React.Component {

Slide.propTypes = {
/**
* A single child content element. The component used as a child must be able
* to hold a ref.
* A single child content element.
*
* ⚠️The component used as a child [must be able to hold a ref](/guides/composition#children).
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
*/
children: PropTypes.element,
/**
Expand Down
7 changes: 1 addition & 6 deletions packages/material-ui/test/integration/MenuList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,14 @@ import { assert } from 'chai';
import { spy } from 'sinon';
import MenuList from 'packages/material-ui/src/MenuList';
import MenuItem from 'packages/material-ui/src/MenuItem';
import RootRef from 'packages/material-ui/src/RootRef';
import { createMount } from 'packages/material-ui/src/test-utils';

function FocusOnMountMenuItem(props) {
const listItemRef = React.useRef();
React.useLayoutEffect(() => {
listItemRef.current.focus();
}, []);
return (
<RootRef rootRef={listItemRef}>
<MenuItem {...props} tabIndex={0} />
</RootRef>
);
return <MenuItem {...props} ref={listItemRef} tabIndex={0} />;
}

function assertMenuItemTabIndexed(wrapper, tabIndexed) {
Expand Down
2 changes: 1 addition & 1 deletion pages/api/button-base.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ It contains a load of style reset and some focus/ripple logic.
| <span class="prop-name">centerRipple</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the ripples will be centered. They won't start at the cursor interaction position. |
| <span class="prop-name">children</span> | <span class="prop-type">node</span> | | The content of the component. |
| <span class="prop-name">classes</span> | <span class="prop-type">object</span> | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. |
| <span class="prop-name">component</span> | <span class="prop-type">element type</span> | <span class="prop-default">'button'</span> | The component used for the root node. Either a string to use a DOM element or a component.<br>[Needs to be able to hold a ref](/guides/composition#caveat-with-refs). |
| <span class="prop-name">component</span> | <span class="prop-type">element type</span> | <span class="prop-default">'button'</span> | The component used for the root node. Either a string to use a DOM element or a component.<br>⚠️ [Needs to be able to hold a ref](/guides/composition#caveat-with-refs). |
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
| <span class="prop-name">disabled</span> | <span class="prop-type">bool</span> | | If `true`, the base button will be disabled. |
| <span class="prop-name">disableRipple</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the ripple effect will be disabled. |
| <span class="prop-name">disableTouchRipple</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the touch ripple effect will be disabled. |
Expand Down
2 changes: 1 addition & 1 deletion pages/api/click-away-listener.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ For instance, if you need to hide a menu when people click anywhere else on your

| Name | Type | Default | Description |
|:-----|:-----|:--------|:------------|
| <span class="prop-name required">children&nbsp;*</span> | <span class="prop-type">element</span> | | The wrapped element. |
| <span class="prop-name required">children&nbsp;*</span> | <span class="prop-type">element</span> | | The wrapped element.<br>⚠️The component used as a child [must be able to hold a ref](/guides/composition#children). |
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
| <span class="prop-name">mouseEvent</span> | <span class="prop-type">enum:&nbsp;'onClick'&nbsp;&#124;<br>&nbsp;'onMouseDown'&nbsp;&#124;<br>&nbsp;'onMouseUp'&nbsp;&#124;<br>&nbsp;false<br></span> | <span class="prop-default">'onMouseUp'</span> | The mouse event to listen to. You can disable the listener by providing `false`. |
| <span class="prop-name required">onClickAway&nbsp;*</span> | <span class="prop-type">func</span> | | Callback fired when a "click away" event is detected. |
| <span class="prop-name">touchEvent</span> | <span class="prop-type">enum:&nbsp;'onTouchStart'&nbsp;&#124;<br>&nbsp;'onTouchEnd'&nbsp;&#124;<br>&nbsp;false<br></span> | <span class="prop-default">'onTouchEnd'</span> | The touch event to listen to. You can disable the listener by providing `false`. |
Expand Down
7 changes: 7 additions & 0 deletions pages/api/root-ref.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ filename: /packages/material-ui/src/RootRef/RootRef.js
import RootRef from '@material-ui/core/RootRef';
```

⚠️⚠️⚠️
If you want the DOM element of a Material-UI component check out
[/getting-started/faq/#how-can-i-access-the-dom-element](FAQ: How can I access the DOM element?)
first.

This component uses `findDOMNode` which is deprecated in React.StrictMode.

Helper component to allow attaching a ref to a
wrapped element to access the underlying DOM element.

Expand Down
2 changes: 1 addition & 1 deletion pages/api/slide.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ It uses [react-transition-group](https://github.com/reactjs/react-transition-gro

| Name | Type | Default | Description |
|:-----|:-----|:--------|:------------|
| <span class="prop-name">children</span> | <span class="prop-type">element</span> | | A single child content element. The component used as a child must be able to hold a ref. |
| <span class="prop-name">children</span> | <span class="prop-type">element</span> | | A single child content element.<br>⚠️The component used as a child [must be able to hold a ref](/guides/composition#children). |
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
| <span class="prop-name">direction</span> | <span class="prop-type">enum:&nbsp;'left'&nbsp;&#124;<br>&nbsp;'right'&nbsp;&#124;<br>&nbsp;'up'&nbsp;&#124;<br>&nbsp;'down'<br></span> | <span class="prop-default">'down'</span> | Direction the child node will enter from. |
| <span class="prop-name">in</span> | <span class="prop-type">bool</span> | | If `true`, show the component; triggers the enter or exit animation. |
| <span class="prop-name">timeout</span> | <span class="prop-type">union:&nbsp;number&nbsp;&#124;<br>&nbsp;{ enter?: number, exit?: number }<br></span> | <span class="prop-default">{ enter: duration.enteringScreen, exit: duration.leavingScreen,}</span> | The duration for the transition, in milliseconds. You may specify a single timeout for all transitions, or individually with an object. |
Expand Down