search

gilbarbara / react-joyride

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

Use first visible instance for target query #439

Closed nuthinking closed 5 years ago

nuthinking commented 5 years ago

I have 2 targets for the same step which are visible interchangeably depending on the page size (media query). I would expect that by default the first visible instance of the target query would be picked up.

Expected behavior

The beacon appears close to the only visible instance of .my-button

Actual behavior

The beacon still tries to find the first instance in the dom of .my-button and because this can't be located visually the beacon appears on the top-left corner.

React version

16.2

React-Joyride version

2.0.0-15

Browser name and version

Chrome 70 / Mac

nuthinking commented 5 years ago

Looks like getElement function in the dom module could be a bit more sophisticated. Would you accept such PR?

nuthinking commented 5 years ago

Made PR for both Joyride and Floater

I know it's extra computation but I believe it's pretty essential for responsive website/apps.

gilbarbara commented 5 years ago

I think this will add more surface to bugs than help, in most cases.

Maybe it's better to conditionally render only what you need for the current screen size to avoid extra DOM computation anyway.