reveal / notes-server

The reveal.js Speaker Notes Server Plugin
MIT License
38 stars 12 forks source link

added arg options and QR code generation #4

Closed sashokbg closed 3 months ago

sashokbg commented 1 year ago

Hello, I am proposing some upgrades to your great tool :)

image

Example running with args:

npx reveal-notes-server \ 
  --presentationDir=./src \
  --presentationIndex=/presentation.html \
  --hostname=192.168.17.137

Please let me know what you think and if things look ok I will add also the README part etc.

Best Regards, Alex

sashokbg commented 10 months ago

Hello, any update on this PR ?

sashokbg commented 9 months ago

Hello @hakimel are you interested in the upgrades I am proposing ? Can you please take a look at the PR ?

hakimel commented 9 months ago

The additional options to make this more configurable is great. I don't like the idea of having a QR code overlaid on top of the presentations by default though. Could you change that so it's opt-in? It could either be switched on via config/arg or a keyboard shortcut.

If you want to add keyboard shortcut there's an Reveal.addKeyBinding API method that registers the shortcut so that it shows up in the reveal.js help overlay.

Reveal.addKeyBinding( {
    keyCode: 81,
    key: 'Q',
    description: 'Show speaker notes QR code'
}, () => {
    console.log('show qr code')
} )

Sorry for taking forever to get back on this, @sashokbg.

sashokbg commented 9 months ago

Hello @hakimel you have a valid point, I will make it configurable. But maybe leave it on by default ? Just to make it clear the QR code disappears once we are connected and it can be closed by clicking on it (there is a small text under explaining this).

PS: No worries about taking some time for this topic - we all have lots of work, projects and family :)

hakimel commented 9 months ago

I didn't realize the code hides automatically when the notes window connects. With that in mind I'm fine with leaving this on by default, as long as there's a way to disable it :)

sashokbg commented 9 months ago

Hello @hakimel I have added a client option that can disable the QR code. I also updated the Readme md

Let me know what you think. Have a nice weekend

sashokbg commented 3 months ago

Hello @hakimel a small reminder about this feature :) Does it look good to you ?

hakimel commented 3 months ago

Thanks for adding the config option! This has been merged but I'm going to make a minor tweak. Will update here shortly.

hakimel commented 3 months ago

I've made a few changes in 7afcdf41083923b5e36c9ba9c4ead5fa2e15e949:

Hope that makes sense. Thanks again for the PR!

sashokbg commented 3 months ago

Thank you for your help @hakimel. Could you increment and publish the new version to npm please ?

hakimel commented 3 months ago

Sure, just published 0.2.0 https://www.npmjs.com/package/reveal-notes-server