search

gilbarbara / react-joyride

Create guided tours in your apps
https://react-joyride.com/
MIT License
6.62k stars 519 forks source link

Add top offset up to the body #960

Closed sergei-deriv closed 6 months ago

sergei-deriv commented 8 months ago

Issue: when the next step is out of the screen viewport tooltip is in the wrong place.

How it was before:

image

How it looks now:

image
codesandbox-ci[bot] commented 8 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 80c4752fbeeec52d246bdf07a4e86e1bd4285f15:

Sandbox Source
React Configuration
gilbarbara commented 8 months ago

Hey @sergei-deriv Before considering this PR, I need to see this behavior happening to ensure it can't be fixed with CSS alone. Can you create a codesandbox with a minimal reproducible example?

sergei-deriv commented 8 months ago

Hey @sergei-deriv Before considering this PR, I need to see this behavior happening to ensure it can't be fixed with CSS alone. Can you create a codesandbox with a minimal reproducible example?

Thank you for your fast reply, really appreciate it. You can see the error on your own site in tab scroll (https://react-joyride.com/scroll). I'll try to record the video when sometimes your library highlights wrong elements for scrolling parents when element is outside of viewport

https://github.com/gilbarbara/react-joyride/assets/120570511/ce2393c2-3e77-4dfe-b603-74bf553cacc5

gilbarbara commented 8 months ago

@sergei-deriv I'm still getting the same behavior with your changes.

https://github.com/gilbarbara/react-joyride/assets/31954/039f4c1b-279c-422b-8524-9bbb39bcc794

By the way, I didn't test the dom utilities on purpose. Using JSDOM to test element size and offset increases the coverage, but it's completely fake. :)

sonarcloud[bot] commented 8 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Barash777 commented 8 months ago

@sergei-deriv I'm still getting the same behavior with your changes.

react-joyride-scroll.mp4 By the way, I didn't test the dom utilities on purpose. Using JSDOM to test element size and offset increases the coverage, but it's completely fake. :)

Hi @gilbarbara. I thought PR didn't pass the validation because of coverage decrease and by this reason I've added tests :) About the same behaviour, it's strange and in this case what exactly I've fixed and why it works for my case)) Please, don't close the PR, I'll try to find a solution for this case too and add changes here. Thanks

gilbarbara commented 6 months ago

Fixed in 2.7.2 Thanks