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.

Sign up for GitHub

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

Jump to bottom

Update reveal, fix scrolling and add transition option #600

Merged
merged 6 commits into from
Jun 5, 2017
Merged

Update reveal, fix scrolling and add transition option #600

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d5c50fe
Better per-slide scrolling (defaulting to False but with options to e…
damianavila May 31, 2017
16c843f
Correct docstrings about the transitions and use the slide one as def…
damianavila Jun 1, 2017
acbd41f
Correct docstring for scroll option.
damianavila Jun 1, 2017
8eeeb2f
Get the boolean in the correct way and simplify the function a little…
damianavila Jun 1, 2017
51d5c77
Better (without track) scrolling bar and unified colors for controls,…
damianavila Jun 1, 2017
2364f4c
Use CSS calc instead of awful px based height
damianavila Jun 2, 2017
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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