Skip to content
New issue

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

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

Already on GitHub? # to your account

syncing speaker notes to speaker window after figwheel reload #15

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

oliyh
Copy link
Contributor

@oliyh oliyh commented Feb 6, 2023

Hi,

Reveal.js seems to support reloading of speaker notes but this wasn't working in my reveal-cljs project after a figwheel reload (I guess the reveal method of hot reloading is less incremental than figwheel).

This change does two things:

  1. Prevents figwheel from unnecessarily opening a websocket in the speaker slides window
  2. Posts a message to the speaker notes window after the reload with the new notes content

NB The message post was reverse-engineered from the Reveal notes plugin code and doesn't use an exposed API, so it's not as robust as I'd like, but I couldn't find any other way to do it.

Thanks

@oliyh
Copy link
Contributor Author

oliyh commented Feb 7, 2023

Apologies, this is not good for merging yet

  • it attempts to open the speaker window when it's not already open
  • it refers to figwheel which won't be present in the deployment build

I'll attempt to fix both these issues and let you know

@n2o
Copy link
Owner

n2o commented Feb 7, 2023

Hi, thanks for your input :-)

Please let me know if I can help you any further to solve this issue.

@oliyh
Copy link
Contributor Author

oliyh commented Feb 7, 2023

Hi,

I've pushed an update which solves both problems and also doesn't rely on internals from revealjs anymore to work, so it's much cleaner.

Thanks!

@oliyh
Copy link
Contributor Author

oliyh commented Feb 8, 2023

Hi,

Let me know if you're happy with this or would like any further explanation.

Cheers

@n2o
Copy link
Owner

n2o commented Feb 8, 2023

Looks great, thanks :-)

@n2o n2o merged commit b2e111f into n2o:main Feb 8, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants