regebro / hovercraft

Make dynamic impressive presentations from text files!
https://hovercraft.readthedocs.org
MIT License
1.49k stars 208 forks source link

Support for Impress 1.0.0 ? #154

Closed 31416r closed 5 years ago

31416r commented 6 years ago

First of all, I would like to thank you all for Hovercraft which is a wonderful tool that I use for all my presentations.

One thing I needed for a long time was the ability to play the presentation automatically as well as a start/stop button available on the slideshow. I noticed that all this has been included in the recent 1.0.0 release of Impress, along with the presenter console plugin and many other improvements.

https://github.com/impress/impress.js/releases/tag/1.0.0

AFAIK, Hovercraft has not yet been adapted to use this last version of Impress, and I think it would be a great step forward. Could it be done anytime soon ? As I can maybe tackle this myself, what would be the steps needed to achieve this ?

Thanks again for Hovercraft !

regebro commented 6 years ago

You can build your own template with v1.0.0, but I don't know if those features would be automatically supported. The easiest way to test is probably to just checkout Hovercraft and replace all copies of impress.js with the new version and see if it works.

It's on my todolist for the next release, but I don't plan releases ahead of time, I do them when I get time, so I don't know when that would be.

31416r commented 6 years ago

Ok, thank you for the reply.

I have already try to simply replace impress.js but it does not bring the new features automatically.

In particular, the autoplay feature is related to the data-autoplay attributes in the #impress div. I think that attribute value should be taken from RST source in the same way that data-transition-duration is, and not set in the generated html if not defined in the source. I guess I could add it to a template dedicated to autoplay slideshows. It would lack flexibility but would constitute usable workaround.

The buttons are included in a new toolbar plugin which require a container div named "impress-toolbar". I guess that can be included statically a new template.

ccamara commented 6 years ago

+1 to this feature.

ccamara commented 6 years ago

@31416r could you please test this PR #170 ? It is working on my computer (toolbar is displayed and I can turn autoplay on and off.

31416r commented 6 years ago

I tried it and it seems to work quite well ! :+1:

I notice two things :

To solve this, it seems that it is sufficient to add this in the template (XSL), just after the other attributes of the impress div :

    <xsl:if test="@data-autoplay">
          <xsl:attribute name="data-autoplay">
                  <xsl:value-of select="@data-autoplay" />
          </xsl:attribute>
    </xsl:if>

Of course, for this to work, you have to include the attribute définition at the start of the RST file. Something like :

:title: My presentation title
:data-transition-duration: 500
:data-autoplay: 10

The problem with this may be then that the slides start to autoplay immediately when loading the page.

31416r commented 6 years ago

As a nice to have, I also think we should be able to specify (in RST)

ccamara commented 6 years ago

Great testing, @31416r . I didn't consider checking the presenter's console.

I have updated PR #170 with some of your comments:

I have not been able to do your other suggestions, as my skills are pretty basic and I am not familiar with how does hovercraft handle fields, but I do believe that your requests are very sensible ones. Maybe if @regebro gave some guidelines I could try to do it.

31416r commented 6 years ago

Great job !

I think you could it would be good to support the autoplay feature of impress 1.0.0. It can be done very simply by adding :

    <xsl:if test="@data-autoplay">
          <xsl:attribute name="data-autoplay">
                  <xsl:value-of select="@data-autoplay" />
          </xsl:attribute>
    </xsl:if>

at line 94 of template.xls.

This affects nothing except when you add the autoplay attribute in the RST (which is not documented at this time). The slides start to autoplay when loading but, I think it should be the expected behavior when you add autoplay within the RST anyway.

ccamara commented 6 years ago

Thanks for your comment, @31416r ! I simply copied-pasted your code (after testing that it works perfectly as expected) in commit 4a579db and added to the PR along with commit 64f535959d1ef4ae28d7385391e08f838dd574ee updating documentation.

PS: I tried to credit you on the commit the best I could, but since I do not have your email I could not link the commit to your github user (or I do not know how to do it)

31416r commented 6 years ago

no prob' for me ;-) Thanks for the job !

yeyeto2788 commented 5 years ago

@ccamara, @31416r hello guys! Since PR #170 is already merged can we close this issue? Or do you guys think that there is something still missing?

31416r commented 5 years ago

That's ok for me !

ccamara commented 5 years ago

I honestly didn't remember about this. Feel free to close it if it has already been merged!

regebro commented 5 years ago

There is something missing, but I don't remember what. I guess I can make new issues when I encounter them.