Remove findDomNode from withHoverAreas by making a render prop component #11407
Conversation
youknowriad
added
the
[Type] Code Quality
There was a problem hiding this comment.
Nice!
I tested this by hovering my mouse over different blocks in the demo post and nothing seems to have regressed.
Left some minor suggestions on how to improve the comprehensibility of WithHoverAreas.
Some unit tests for WithHoverAreas would be ace, but since they're missing in master that's not a blocker.
| { shouldShowMobileToolbar && ( | ||
| <BlockMobileToolbar | ||
| <WithHoverAreas> | ||
| { ( { bindContainer, hoverArea } ) => { |
There was a problem hiding this comment.
Did you consider changing BlockListBlock to use createRef() and then having it share this ref with <WithHoverArea>?
<WithHoverAreas containerRef={ this.blockListRef }>
{ ( hoverArea ) => {
...
} }
</WithHoverAreas>I think this makes it a little clearer that the "container" is something that's passed in to WithHoverAreas.
There was a problem hiding this comment.
Good ideal, I'll try it.
| if ( this.state.hoverArea ) { | ||
| this.setState( { hoverArea: null } ); | ||
| } | ||
| toggleListeners( shouldRemoveEvents ) { |
There was a problem hiding this comment.
I would expect the sole boolean argument of toggleListeners to work the opposite way.
this.toggleListeners( true ); // Adds the listeners
this.toggleListeners( false ); // Removes the listeners| this.onMouseLeave = this.onMouseLeave.bind( this ); | ||
| this.onMouseMove = this.onMouseMove.bind( this ); | ||
| } | ||
| class WithHoverAreas extends Component { |
There was a problem hiding this comment.
The convention we seem to follow is that components beginning with with are HOCs. In other places that we've used components with render props, we've named it something more conventional, e.g. PostTaxonomies.
(HoverArea would be my suggestion here.)
youknowriad
force-pushed
the
remove/find-dom-node-with-hover-areas
branch
from
3caeb8c
to
3f15f77
Compare
Related #11360
This PR removes
findDOMNodefrom thewithHoverAreasHigher Order Component by making it a component with a render prop. (This would be the perfect use-case for a React hook :) )Make sure to view the diff without "whitespace" diff