gilbarbara / react-joyride

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

Fix support of floaterProps.getPopper #926

Closed andreash82 closed 6 months ago

andreash82 commented 12 months ago

Fixes #875

Changed order of props passed to Floater to avoid overriding mandatory internal stuff by provided floaterProps, chained internal function to call of provided floaterProps.getPopper.

Extending PropTypes was required due to lint settings.

codesandbox-ci[bot] commented 12 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 2983ec9287f63a133f9a0ed16c6ed0fc26b11611:

Sandbox Source
React Configuration
elastic-varahamihira-6md2d2 Issue #875
gilbarbara commented 11 months ago

Hey @andreash82

The issue you mentioned is using floaterProps on the component, not the step. How are they connected?

Also, can you add tests for this new behavior?

andreash82 commented 11 months ago

How are they connected?

The floaterProps on the Joyridecomponent are passed through to the component Step (as documented under Step -> Common Props Inheritance), so i just fixed the part inside the Step component where it had an actual effect due to incorrect property order (combined with property expansion) on <Floater ... getPopper={this.setPopper} ... {...step.floaterProps}> .

The call of floaterProps.getPopper inside Step's setPopper was added so that the passed function is called at all (and not ignored) with the new property order.

Also, can you add tests for this new behavior?

The "hard" part, since floaterProps aren't covered at all yet. A task for my weekend ;)

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
No Duplication information No Duplication information

gilbarbara commented 6 months ago

Fixed in 2.7.2 Thanks