Skip to content

Conversation

AndrewMusgrave
Copy link
Member

@AndrewMusgrave AndrewMusgrave commented Jun 17, 2019

WHY are these changes introduced?

Last change before we can enable strict mode 🎉

Warning: findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of StyledComponent which is inside StrictMode. Instead, add a ref directly to the element you want to reference.

WHAT is this pull request doing?

Luckily none of our uses require findDOMNode to find a child element without any context so we can replace them with refs.

@BPScott BPScott temporarily deployed to polaris-react-pr-1696 June 17, 2019 22:07 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1696 June 17, 2019 22:09 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1696 June 18, 2019 19:00 Inactive
bulkActionButtonNode.getBoundingClientRect().width) ||
0;
const width = (this.bulkActionButton
.current as HTMLButtonElement).getBoundingClientRect().width;
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 cast is to increase codecov. We don't optionally render the button so this ref will always exist in componentDidMount lifecycle

});
});

function FocusTestWrapper({children, ...props}: Discard<Props, 'root'>) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Wrapper component that passes a root to Focus

@AndrewMusgrave AndrewMusgrave added this to the v4.0.0 milestone Jun 18, 2019
@AndrewMusgrave AndrewMusgrave changed the title [v4] Removed all useage of findDOMNode [v4] Removed all usage of findDOMNode Jun 18, 2019
@AndrewMusgrave AndrewMusgrave added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Jun 18, 2019
@AndrewMusgrave AndrewMusgrave marked this pull request as ready for review June 18, 2019 19:35
@AndrewMusgrave AndrewMusgrave requested a review from dleroux June 18, 2019 19:36
Copy link
Contributor

@dleroux dleroux left a comment

Choose a reason for hiding this comment

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

As per usual, looks great, works great 🎉 🚢

@AndrewMusgrave AndrewMusgrave merged commit 827510e into version-4.0.0 Jun 18, 2019
@AndrewMusgrave AndrewMusgrave deleted the remove-finddomnode branch June 18, 2019 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖Skip Changelog Causes CI to ignore changelog update check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants