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

revealjs_backgroundTransition attribute has no effect #263

Closed
akosma opened this issue Jun 5, 2019 · 10 comments
Closed

revealjs_backgroundTransition attribute has no effect #263

akosma opened this issue Jun 5, 2019 · 10 comments

Comments

@akosma
Copy link
Contributor

akosma commented Jun 5, 2019

The following Asciidoc file contains a revealjs_backgroundTransition attribute, yet the generated HTML does output backgroundTransition: 'fade' in the configuration instead.

:revealjs_theme: white
:revealjs_backgroundTransition: none

= White

Title

[background-color="black"]
== Black

TIP: This is a tip

== White

White slide

Built with the simplest possible command: asciidoctor-revealjs slides.adoc using the following software versions:

$ asciidoctor-revealjs --version
Asciidoctor 2.0.9 [https://asciidoctor.org]
Runtime Environment (ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-linux]) (lc:UTF-8 fs:UTF-8 in:UTF-8 ex:UTF-8)

The source Asciidoc and generated HTML are attached: slides.zip

@akosma akosma changed the title revealjs_backgroundTransition has no effect The revealjs_backgroundTransition attribute has no effect Jun 5, 2019
@akosma akosma changed the title The revealjs_backgroundTransition attribute has no effect revealjs_backgroundTransition attribute has no effect Jun 5, 2019
@mojavelinux
Copy link
Member

That's because of this issue: asciidoctor/asciidoctor#509

In brief, attributes cannot contain uppercase characters. You can write them that way, but the AsciiDoc processor forces them to lowercase. Therefore, the code needs to be changed so that it looks for revealjs_backgroundtransition not revealjs_backgroundTransition. (That way, you can write it however you want and it will still work).

@mojavelinux
Copy link
Member

Btw, there are a number of settings that are affected by this:

  • revealjs_controlsTutorial
  • revealjs_controlsLayout
  • revealjs_controlsBackArrows
  • revealjs_fragmentInURL
  • revealjs_showNotes
  • revealjs_autoPlayMedia
  • revealjs_autoSlide
  • revealjs_autoSlideStoppable
  • revealjs_autoSlideMethod
  • revealjs_defaultTiming
  • revealjs_mouseWheel
  • revealjs_hideAddressBar
  • revealjs_previewLinks
  • revealjs_transitionSpeed
  • revealjs_backgroundTransition
  • revealjs_viewDistance
  • revealjs_parallaxBackgroundImage
  • revealjs_parallaxBackgroundSize
  • revealjs_parallaxBackgroundHorizontal
  • revealjs_parallaxBackgroundVertical

None of those are going to work as the code stands now.

@bentolor
Copy link
Contributor

I'm currently desperately trying to get my presentation finished and this bug made me heavy headaches. Many attributes are silently failing and I was totally puzzled unless I noted that it was always attributes with casing in in and I remembered that Github issue I saw some time ago…

@mojavelinux I'm unsure which way to go:

  • should I open a MR changing all asciidoctor attribute refs to purely lowercase or would this be wrong?
  • or should the code somewhere start using the referred sanitize_attribute_name which IMHO would fix that problem?

@mojavelinux
Copy link
Member

I would go with the first option. Case doesn't mean anything in AsciiDoc attributes, so there's never any reason to use it.

If you submitted a PR, I would expect it could get merged quickly because this feature is broken as it stands.

@bentolor
Copy link
Contributor

Thanks for the lightning-fast reply! I opened #267

@bentolor
Copy link
Contributor

P.S. I'm using my local patched version for now, so I'm good for now. No release pressure…

@obilodeau
Copy link
Member

This was fixed with the just-released 2.0.1. Can you confirm that the fixed version works for you?

@obilodeau
Copy link
Member

Closing. This was confirmed elsewhere as fixed. Feel free to re-open.

@akosma
Copy link
Contributor Author

akosma commented Dec 20, 2019

I confirm it works perfectly well now. Thanks a lot!

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

No branches or pull requests

4 participants