search

gilbarbara / react-joyride

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

fix(joyride): fix nested buttons warning #1022

Closed TommyBacco closed 2 months ago

TommyBacco commented 2 months ago

The warning

image

How to reproduce

Since a button cannot be a child of another button, the warning is issued any time a <button /> is used as locale for next, skip or back. So in order for locale prop to accept any ReactNode I think it should wrap that in a div instead of a button or alternatively only wrap in a button locales of type string.

Why is it needed

This could be really useful to use button components from visual element libraries instead on leveraging on css styles only.

Why this is not going to break everything

I ran the react-joyride-demo and the playwright tests

codesandbox-ci[bot] commented 2 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.

gilbarbara commented 2 months ago

Hey @TommyBacco

Changing the button for a div is semantically wrong since screen readers wouldn't identify it correctly. And if you need to use custom components, the tooltipComponent prop exists for this exact reason.

Also, according to the CONTRIBUTING guide, it's always preferable to open an issue first to discuss the idea before wasting time creating a PR.

Thanks for the PR, anyway.