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 reveal, fix scrolling and add transition option #600

Merged
merged 6 commits into from
Jun 5, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion nbconvert/exporters/slides.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from copy import deepcopy
from warnings import warn

from traitlets import Unicode, default
from traitlets import Bool, Unicode, default

from .html import HTMLExporter

Expand Down Expand Up @@ -103,6 +103,21 @@ def _reveal_url_prefix_default(self):
"""
).tag(config=True)

reveal_transition = Unicode('slide',
help="""
Name of the reveal.js transition to use.

The list of transitions that ships by default with reveal.js are:
none, fade, slide, convex, concave and zoom.
"""
).tag(config=True)

reveal_scroll = Bool(False,
help="""
If True, enable scrolling within each slide
"""
).tag(config=True)

require_js_url = Unicode(
"https://cdnjs.cloudflare.com/ajax/libs/require.js/2.1.10/require.min.js",
help="""
Expand Down Expand Up @@ -146,6 +161,8 @@ def from_notebook_node(self, nb, resources=None, **kw):
resources['reveal'] = {}
resources['reveal']['url_prefix'] = self.reveal_url_prefix
resources['reveal']['theme'] = self.reveal_theme
resources['reveal']['transition'] = self.reveal_transition
resources['reveal']['scroll'] = self.reveal_scroll
resources['reveal']['require_js_url'] = self.require_js_url
resources['reveal']['jquery_url'] = self.jquery_url
resources['reveal']['font_awesome_url'] = self.font_awesome_url
Expand Down
2 changes: 1 addition & 1 deletion nbconvert/postprocessors/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class ServePostProcessor(PostProcessorBase):
open_in_browser = Bool(True,
help="""Should the browser be opened automatically?"""
).tag(config=True)
reveal_cdn = Unicode("https://cdnjs.cloudflare.com/ajax/libs/reveal.js/3.1.0",
reveal_cdn = Unicode("https://cdnjs.cloudflare.com/ajax/libs/reveal.js/3.5.0",
help="""URL for reveal.js CDN."""
).tag(config=True)
reveal_prefix = Unicode("reveal.js", help="URL prefix for reveal.js").tag(config=True)
Expand Down
75 changes: 67 additions & 8 deletions nbconvert/templates/html/slides_reveal.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ if( window.location.search.match( /print-pdf/gi ) ) {
/* Overrides of notebook CSS for static HTML export */
.reveal {
font-size: 160%;
overflow-y: scroll;
}
.reveal pre {
width: inherit;
Expand Down Expand Up @@ -116,6 +115,41 @@ if( window.location.search.match( /print-pdf/gi ) ) {
.reveal .progress {
position: static;
}
.reveal .controls .navigate-left,
.reveal .controls .navigate-left.enabled {
border-right-color: #727272;
}
.reveal .controls .navigate-left.enabled:hover,
.reveal .controls .navigate-left.enabled.enabled:hover {
border-right-color: #dfdfdf;
}
.reveal .controls .navigate-right,
.reveal .controls .navigate-right.enabled {
border-left-color: #727272;
}
.reveal .controls .navigate-right.enabled:hover,
.reveal .controls .navigate-right.enabled.enabled:hover {
border-left-color: #dfdfdf;
}
.reveal .controls .navigate-up,
.reveal .controls .navigate-up.enabled {
border-bottom-color: #727272;
}
.reveal .controls .navigate-up.enabled:hover,
.reveal .controls .navigate-up.enabled.enabled:hover {
border-bottom-color: #dfdfdf;
}
.reveal .controls .navigate-down,
.reveal .controls .navigate-down.enabled {
border-top-color: #727272;
}
.reveal .controls .navigate-down.enabled:hover,
.reveal .controls .navigate-down.enabled.enabled:hover {
border-top-color: #dfdfdf;
}
.reveal .progress span {
background: #727272;
}
div.input_area {
padding: 0.06em;
}
Expand Down Expand Up @@ -149,6 +183,19 @@ a.anchor-link {
.rendered_html p {
text-align: inherit;
}
::-webkit-scrollbar
{
width: 6px;
height: 6px;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know CSS can override the scrollbar. Are you sure you want to do that? It seems like the kind of thing that people might get angry about.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have strong intuitions around this, what would the downsides be?

Copy link
Member Author

@damianavila damianavila Jun 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is overriding stuff on the slideshow... and this scroll is there specifically to scroll the slide.
Btw, this gives me the opportunity to make the scroll closer (in look and feel) to other elements... otherwise you get the "system" view which it is not fitting at all the the slide feeling.

::-webkit-scrollbar *
{
background:transparent;
}
::-webkit-scrollbar-thumb
{
background: #727272 !important;
}
</style>

<!-- Custom stylesheet, it must be in the same directory as the html file -->
Expand Down Expand Up @@ -190,8 +237,7 @@ require(
progress: true,
history: true,

theme: Reveal.getQueryHash().theme, // available themes are in /css/theme
transition: Reveal.getQueryHash().transition || 'linear', // default/cube/page/concave/zoom/linear/none
transition: "{{resources.reveal.transition}}",

// Optional libraries used to extend on reveal.js
dependencies: [
Expand All @@ -211,13 +257,26 @@ require(

Reveal.addEventListener('slidechanged', update);

var update_scroll = function(event){
$(".reveal").scrollTop(0);
};

Reveal.addEventListener('slidechanged', update_scroll);
function setScrollingSlide() {
var scroll = {{ resources.reveal.scroll | json_dumps }}
if (scroll === true) {
var h = $('.reveal').height() * 0.95;
var hpx = "" + h + "px";
$('section.present').find('section')
.filter(function() {
return $(this).height() > h;
})
.css('height', hpx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work to use a height like 95%, or even CSS calc (calc(100% - 20px)), rather than doing calculations in JS?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not try to use CSS calc... will take a look in your suggestions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work to use a height like 95%

Nop, it cut the stuff at the very top.

even CSS calc

Neither... in both cases I get the cut off

slide2

It seems I need to specify the the absolute height in pixel to make it work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gah, CSS. It's probably doing the relative height relative to the wrong parent.

One more thing to try: calc(100vh - 20px). vh units are 1% of the height of the viewport, so 100vh should be the full height available in the window.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, vh tested and worked. Pushed a commit fixing it. Thanks for letting learn about this CSS calc stuff!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks along the lines of the kinds of things to get around 'auto' heights in the case of animations. There should be a way to work natively in pixels rather than treating it as a string manipulation problem though. I forget what that is off the top of my head though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed a commit using @takluyver suggested approach which get rid of the awkward string manipulation.

.css('overflow-y', 'scroll')
.css('margin-top', '20px');
}
}

// check and set the scrolling slide every time the slide change
Reveal.addEventListener('slidechanged', setScrollingSlide);

}

);
</script>

Expand Down