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

Update list of reveal.js config options #590

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

moble
Copy link

@moble moble commented Mar 2, 2021

These lines suggest that more options are actually passed through. I took the descriptions from the reveal documentation. (Note that the website is currently down, so I just looked through the src.)

[These lines](https://github.com/damianavila/RISE/blob/ebfcb5760823787e632b4a0c19681829474d1710/classic/rise/static/main.js#L539-L540) suggest that more options are actually passed through.  I took the descriptions from the reveal documentation.  (Note that the website is currently down, so I just looked through the [src](https://github.com/reveal/revealjs.com/blob/master/src/config.md).)
@damianavila
Copy link
Owner

@moble, thanks for the PR but even when some other options are passthrough, modifying some of them could be problematic (https://github.com/damianavila/RISE/blob/master/classic/rise/static/main.js#L110), this is why we do not advertise them. @parmentelat, maybe we need to prevent passing through some of these options... thoughts?

@parmentelat
Copy link
Collaborator

hiya everythone

well, given that, as the Python motto has it, we're all consenting adults here, I'd rather leaver this open, but indeed a few words of warning about the possible negative effects of tweaking them could be quite helpful
this being said I would have a hard time trying to phrase this in any specific way...

@damianavila
Copy link
Owner

but indeed a few words of warning about the possible negative effects of tweaking them could be quite helpful

I would be OK with that!

this being said I would have a hard time trying to phrase this in any specific way...

Me too 😉 maybe @moble has some words for us!

@moble
Copy link
Author

moble commented Mar 8, 2021

Me too 😉 maybe @moble has some words for us!

I'm afraid I'm more lost than ever, now. I would appreciate some direction on which options will/should work.

@damianavila
Copy link
Owner

I'm afraid I'm more lost than ever, now. I would appreciate some direction on which options will/should work.

I guess we are essentially asking you to add to your PR some note that warns the user that things could be wrong if they tweak those additional parameters (the ones you added).

@parmentelat parmentelat force-pushed the master branch 2 times, most recently from 5e2531e to 6d81041 Compare September 17, 2023 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants