JohnCoene / cicerone

🏛️ Give tours of your Shiny apps
https://cicerone.john-coene.com
Other
187 stars 7 forks source link

Opening/Closing an element during a cicerone guide causes guide to disappear. #62

Open yogat3ch opened 4 months ago

yogat3ch commented 4 months ago

Hi @JohnCoene , Is there an input that monitors which step the guide is on or when it concludes to chain guides together? A full description of the issue is in this video: https://www.loom.com/share/5add4592b8b74c93a787985b80522560?sid=ea2f62f2-6b2c-498b-aad7-6e368a3f964e

yogat3ch commented 4 months ago

By the looks of this I think I can chain two together by monitoring this input: https://github.com/JohnCoene/cicerone/blob/da81fedad5af915baf4eb747dc081281583a219d/srcjs/exts/cicerone.js#L37

Update: I have not had any luck observing that input, it doesn't seem to fire. I think it might be a namespacing issue?

Update: I forked cicerone, put a debugger in to take a look at what that id was at the line above, and managed to figure out the packer compilation for the JS but it required an npm update. The _cicerone_next input is now firing because I changed the guide id to use the ns of the module the input is observed in, but it doesn't seem to fire on every step? It doesn't appear to update on some steps

JohnCoene commented 4 months ago

Do the events listed here help at all?

https://cicerone.john-coene.com/guide/events

yogat3ch commented 4 months ago

Do the events listed here help at all?

https://cicerone.john-coene.com/guide/events

Thank you for pointing me to the documentation @JohnCoene!

input$ciceroneId_cicerone_next and input$ciceroneId_cicerone_previous which are triggered every time the user presses “next” or “previous”

It says this, but I'm not actually finding that to be true. ciceroneId_cicerone_next does not fire on some steps. If I'm providing a custom on_next argument, is that going to override the assignment of that input?

If so, we should probably modify the JS of on_next to combine the user supplied JS with the on_next function logic that assigns the input rather than overwriting it?

JohnCoene commented 4 months ago

Indeed, that's how it should work, I can try a fix, in the meantime you will have to set the input value yourself I'm afraid :(

yogat3ch commented 4 months ago

@JohnCoene I have a decent understanding of how everything is put together in cicerone after spending some time with it yesterday. Do you have the bandwidth to review a PR to upgrade the entire package to use the new version of driver.js if I do the upgrade? It's been out for some years now and has a good bit more functionality that is missing from the 0.8.9 version.

JohnCoene commented 4 months ago

Sure, always happy to do that!

I was just looking at it here

yogat3ch commented 4 months ago

@JohnCoene 😄 That's awesome, that's exactly what I was doing last night! Do you want me to commit directly to that branch or branch off of it for my contributions?

JohnCoene commented 4 months ago

Feel free to take the branch that suits you best, you'll be the one working on this. Also add yourself to the DESCRIPTION as contributor before your PR :)

yogat3ch commented 4 months ago

@JohnCoene Ok will do!

yogat3ch commented 4 months ago

Hi @JohnCoene,

63 is up for your review!

Update 2:45p ET: I realized there's a little more to do it, so you can hold of on review

Update 4:38p ET: All done and ready for review again!