Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add top offset up to the body #960

Closed
wants to merge 2 commits into from

Conversation

sergei-deriv
Copy link

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
Copy link

codesandbox-ci bot commented Oct 26, 2023

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 80c4752:

Sandbox Source
React Configuration

@gilbarbara
Copy link
Owner

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
Copy link
Author

sergei-deriv commented Oct 26, 2023

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

Wrong.place.mov

@gilbarbara
Copy link
Owner

@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. :)

@sonarqubecloud
Copy link

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
Copy link

@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
Copy link
Owner

Fixed in 2.7.2
Thanks

@gilbarbara gilbarbara closed this Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants