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

Remove usage of React.findDOMNode() #189

Merged
merged 7 commits into from
Nov 15, 2018
Merged

Conversation

zhusee2
Copy link
Contributor

@zhusee2 zhusee2 commented Nov 13, 2018

Purpose

As of React 16.6, ReactDOM.findDOMNode() is added to list of deprecated API.
But even before that, usage of the API has been advised to be prevented.

This PR removes calls to the listed API from anchored() HOC mixin, and instead introduces an approach that requires you to manually set refs to both anchor element and wrapped element.

The anchored(Component) HOC now only accept an HTMLElement node for the anchor prop it's taking.
Also it now passes an additional nodeRef prop to the wrapped component, which is a React.createRef() to collect ref to the DOM node that should be positioned to anchor.

Example

Configuring wrapped component

function Component({ placement, arrowStyle, style, nodeRef }) {
    return (
        <div ref={nodeRef} className={placement} style={style}>
            <div className="arrow" style={arrowStyle} />
            content body
        </div>
    );
}
const AnchoredComponent = anchored(options)(Component);

setting anchor

The anchored(Component) HOC only renders if the anchor prop is a valid DOM node.
So please be sure to render it after the ref to anchor is set.

class Example extends React.Component {
    static propTypes = {
        show: PropTypes.bool.isRequired,
    };

    anchorRef = React.createRef();

    render() {
        return (
            <div>
                <div className="anchor" ref={this.anchorRef} />
                {this.props.show && (
                    <AnchoredComponent anchor={this.anchorRef.current} />
                )}
            </div>
        );
    }
}

@zhusee2 zhusee2 added this to the Version 2 milestone Nov 13, 2018
@zhusee2 zhusee2 self-assigned this Nov 13, 2018
@zhusee2 zhusee2 requested a review from hsunpei November 13, 2018 08:56
@cjies cjies mentioned this pull request Nov 13, 2018
8 tasks
@zhusee2 zhusee2 changed the base branch from feature/adapt_portal to develop November 14, 2018 03:32
Copy link
Contributor

@hsunpei hsunpei left a comment

Choose a reason for hiding this comment

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

Nice code with clear examples 🥇 🚢

@zhusee2 zhusee2 merged commit 97303d4 into develop Nov 15, 2018
@zhusee2 zhusee2 deleted the feature/dont_find_dom_node branch November 15, 2018 09:04
@zhusee2 zhusee2 mentioned this pull request Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants