search

jasonkuhrt / react-popover

A smart popover component for React
600 stars 240 forks source link

fix: popover glitch, tip jumps #125 #132

Open AvraamMavridis opened 7 years ago

AvraamMavridis commented 7 years ago

This tries to fix the https://github.com/littlebits/react-popover/issues/125

On popoverResize event is firing even if the size of the popover haven't really changed (due on the late attachement of the tip to the containerEl), the popoverResize re-resolvePopoverLayout causing the jumping glitch. You can see that the event is firing even in the first toggle, and the diff of the height is exactly the size of the tip.

fix-glitch

This tries to fix it by ensuring that the resize is at least > than the tipSize.

AvraamMavridis commented 7 years ago

I would like to test/evaluate on it a bit more, so if anyone else has the issue and can npm -i directly from the fork in their project to test locally and give feedback, would be much appreciated.

Kerumen commented 7 years ago

With your fix:

kapture 2017-09-13 at 19 03 16

Without:

kapture 2017-09-13 at 19 05 49

I have a bug in both cases the initial position of the popover seems off.

AvraamMavridis commented 7 years ago

@Kerumen thx for the feedback, I will check it more in deep.

Kerumen commented 7 years ago

@AvraamMavridis I reported the initial layout position bug here: https://github.com/littlebits/react-popover/issues/62#issuecomment-329233980

jasonkuhrt commented 7 years ago

@AvraamMavridis Thanks so much for your contribution! This has been a long outstanding rough edge that I'd like to see get fixed. I don't have time to help out on this PR at the moment but if you get this into a perfected state I would be more than happy to try and get this merged.