Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Further improvements to reveal.js plugin configuration. #201

Closed
obilodeau opened this issue Jul 28, 2018 · 5 comments
Closed

Further improvements to reveal.js plugin configuration. #201

obilodeau opened this issue Jul 28, 2018 · 5 comments

Comments

@obilodeau
Copy link
Member

As said in #196, summarizing here:

reveal.js notes plugin doesn't work when loaded from a CDN. It would be preferable if the newly introduced :revealjs_plugins: attribute would override a reveal.js plugin configuration instead of appending to it. This way the notes plugin could be replaced with a local version that should work even if reveal.js is loaded from a CDN.

Original PR provider didn't have time to implement and do the proper test. On my end, I'm going to prioritize other things first but I wanted to track it down.

@a4z
Copy link
Contributor

a4z commented Aug 15, 2018

I think I have an idea how to realize this.
I do not mind caring about tests, but until now I do not know how the testing works for this project.
All this infrastructure is pretty new for me, but looking at the HACKING.adoc the first time now, so I hope the next change comes with tests ;-)
But please note that I am back at work after several weeks holiday and have a very very young daughter at home, so please do not expect results too fast

@a4z a4z mentioned this issue Aug 31, 2018
@a4z
Copy link
Contributor

a4z commented Aug 31, 2018

solution suggestion in PR #204 , please review and share you thoughts

@a4z
Copy link
Contributor

a4z commented Sep 3, 2018

Hi @obilodeau , please review and comment
as in the PR stated, where I am mostly talking with myself, all plugins can now be turned off, or on.

the default behaviour is as it was, tests are passing,

before adding tests for all the toggles, what is, depending on how much work someone wants to invest, pretty much work, (5 flags in all combinations, but we do not need to go that deep), I would like to have feedback if this is an accepted solution.

@obilodeau obilodeau added this to the 1.2.0 milestone Oct 16, 2018
@obilodeau
Copy link
Member Author

The code was merged. Thanks @a4z for your contribution!

@a4z
Copy link
Contributor

a4z commented Oct 30, 2018

Thank you for the awesome improvements you did!
Can't await switching my current slides to the new master, what I will do from next week on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants