**Feature:** <Foldable /> component to React hook

[adeelibr]

Summary

Convert Foldable from React class component to React hook.

Related issue

#910

To be tested

Me

• No error or warning in the console on localhost:6060

[TejasQ]

Beautiful, but I'd request one change.

[adeelibr]

@TejasQ can you kindly review again.

[TejasQ]

Unfortunately, the hover state is quite broken on mouse out.

[adeelibr]

I am sorry I don't understand, The mouse state seems to be toggling fine at my end via the deployment link for this PR. What is that I am missing here? 🤔

I find this as the same behavior as on the https://operational-ui.netlify.com/#Foldable demo as well, please correct me where I am wrong 😅 @TejasQ

[TejasQ]

I am sorry I don't understand

No problem at all!

The mouse state seems to be toggling fine at my end

I'm not sure this is true. I think you may not be testing it correctly.

What is that I am missing here? 🤔

Let's clarify with a GIF showing the current master and your PR:

Notice how on master, a "flick" of a draggable header works as expected: the hover state clears. In your PR's demo, moving a cursor away from a draggable header quickly causes the mouseout or mouseleave event to be ignored and thus, the header remains grey even after the cursor has exited the draggable header.

Are you able to reproduce this?

[adeelibr]

Are you able to reproduce this?

Yes thank you, I am on it.

[adeelibr]

Can you kindly review again. @TejasQ 🤞

[TejasQ]

@adeelibr great work! 🎉

Let's get one last review from @fabien0102.

Can you explain how you fixed it?

[adeelibr]

Can you explain how you fixed it?

+ React.useEffect(() => {
+   document.addEventListener("mousemove", handleMouseMove)
+   return () => document.removeEventListener("mousemove", handleMouseMove)
- }, [])
+ })

Removing [] ensures that the event binding does not happen only on componentDidMount & componentWillUnmount life cycle hook, which was previosly causing glitch & inconsistencies.

The current implementation ensures that as soon as we mouse move over the div element the event as binded & as soon as we mouse out the event is removed. Which ensures that handleMouseMove function is called every time + in doing so ensures that an event is immediately discarded when not in use i.e, 'mousemove' in our case here.

[stereobooster]

use effect will run after each render unless you instruct it not to do so with the second param

[TejasQ]

He's right! Amazing!

https://github.com/contiamo/operational-ui/pull/916/files/22dfed4280b30a5ced56656357141e25cb934c4e#diff-0aa69b95e86b2ae76327ba1ce76f15d3R33

[adeelibr]

Having a look at Dan Abramov's at React Conf 2018 https://youtu.be/V-QO-KO90iQ?t=2475 He isn't monitoring that with a second param. For event listeners.

use effect will run after each render unless you instruct it not to do so with the second param

That is okay! I believe, I have tested this. As soon as I mouse over a the "mousemove" event is binded & as soon as I mouse out the "mousemove" is removed from the DOM for that element. & I think this is the correct behavior. We shouldn't listen to an event continuously which might not get used.

@stereobooster @TejasQ

[TejasQ]

@adeelibr let's do a fix in a separate PR.