[TouchRipple] Remove findDOMNode usage #14825
Conversation
Details of bundle changes.Comparing: a5b8011...b11f755
|
| > | ||
| {this.state.ripples} | ||
| </TransitionGroup> | ||
| <span className={clsx(classes.root, className)} ref={this.container} {...other}> |
There was a problem hiding this comment.
Spreading to the span follows the API docs:
Any other properties supplied will be spread to the root element (native element).
-- https://material-ui.com/api/touch-ripple/#props
It's only accessible via TouchRippleProps in ButtonBase. The PR that introduced this prop mentioned "changing color of the ripple" as a use case. Sounds like the native element is only needed for that not the TransitionGroup
| {this.state.ripples} | ||
| </TransitionGroup> | ||
| <span className={clsx(classes.root, className)} ref={this.container} {...other}> | ||
| <TransitionGroup component={null} enter exit> |
There was a problem hiding this comment.
https://reactcommunity.org/react-transition-group/transition-group#TransitionGroup-prop-component
If TransitionGroup would forward refs or offer some innerRef we could avoid that nesting. I feel way more comfortable not exposing the TransitionGroup though. Making this part of our public API created a tight coupling that forces us to continue using it even though react-transition-group is not strict mode compliant.
| assert.strictEqual(wrapper.name(), 'TransitionGroup'); | ||
| assert.strictEqual(wrapper.props().component, 'span'); | ||
| assert.strictEqual(wrapper.hasClass(classes.root), true); | ||
| it('should render a span', () => { |
There was a problem hiding this comment.
TransitionGroup is not part of the public API
eps1lon
force-pushed
the
feat/TouchRipple/no-findDOMNode
branch
from
ed930a6 to
b11f755
Compare
| import { StandardProps } from '..'; | ||
|
|
||
| export type TouchRippleProps = StandardProps< | ||
| TransitionGroup.TransitionGroupProps, | ||
| React.HTMLAttributes<HTMLElement>, |
There was a problem hiding this comment.
Documentation > Code i.e. type definitions were wrong.
Use ref on the DOM element type instead of
findDOMNode(this).