search

ionelmc / python-darkslide

Generate HTML5 slideshows from markdown, ReST, or textile
http://ionelmc.github.io/python-darkslide/
Apache License 2.0
97 stars 22 forks source link

Add wide16x9 theme #19

Closed epmoyer closed 4 years ago

epmoyer commented 4 years ago

Adds a wide16x9 theme per issue #11.

Tox did not run without error for me (before the change), but it does run enough to generate the example slide decks (in dist/examples/). It runs as well now (post-change) as it did for me before (pre-change) :)

Note: I changed the "default theme" link in the slide footer from . to index.html so that the link to the default theme would work when opening the example deck files locally in a browser (rather than browsing them from a web server). I believe the index.html supports both cases.

I've been using this new theme (with some local style customizations) to develop a complex~30+ slide deck with lots of content/images/etc and it has behaved robustly.

Default: default

Wide: wide

Wide Expose: wide_expose

Wide Presenter: wide_presenter

Wide Next Slide: wide_next

Wide Notes: wide_notes

ionelmc commented 4 years ago

I wonder if having a way to specify the base theme would be better. It would support a wide-layout with all the themes. What do you think? It would mean new configuration and command line options tho ...

epmoyer commented 4 years ago

I like that idea a lot. Specifying a base theme would allow people to easily migrate decks using their existing (local, custom) themes to widescreen by simply declaring the new base. It would also give a super-clean way to add a new aspect ratio if/when that becomes desirable.

ionelmc commented 4 years ago

You also interested in implementing?

Another idea, maybe even better: have a switch for wide-screen. Press w key you get widescreen mode or something along those lines. This would be ideal as you might now always get same screen for a presentation.

epmoyer commented 4 years ago

My experience authoring both a bunch of 4:3 decks and now a big (30+ slides) 16:9 deck is that one ends up doing a fair bit of content shuffling in one's markdown to make sure that the content fits on the slides (and also, for me, to maximize my use of the space I have available). A deck made for 4:3 will work ok in wide, but if you prepare a deck intending to use wide then it is likely to overrun 4:3 (pictures may be too wide, and text will often run off the top/bottom). Personally I would always author a presentation for a specific target aspect ratio, so I'd never use an "on the fly" mode switch option (and having it arguably opens up the opportunity to accidentally switch to 4:3 during a presentation.

Not sure about implementation. I haven't looked into the code base really other than the css / html (but I'm very experienced in Python, so that's not an issue). Are there existing issues with tox or does it finish/pass for you? Right now tox fails for me, but I haven't dug into why. I noticed that the automated Travis build failed as well, so guessing it's not just me? It's harder to take on a "real" code change if not starting from a test suite that passes.

ionelmc commented 4 years ago

Just the publishing fails (i suppose because it's a pr). The content for layout argument sounds pretty sensible. What errors you get from tox? Gist/pastebin?

epmoyer commented 4 years ago

Here's a gist of running tox.

https://gist.github.com/epmoyer/3f51923c028a23d4b80e68407912f161

I've spent zero time trying to parse/resolve the errors. Seems like it is barfing on testing against multiple python versions, but I wasn't even sure the problem was mine (since it was failing on the unmodified repo clone) so I did no digging. I work in Anaconda (Conda) with a 3.7 base and don't use venv for anything I normally do, and haven't really used tox much.

ionelmc commented 4 years ago

Looks like you have some broken pythons installed (lacking certain stdlib modules). Either way looks like py27 is passing so no big deal, just run tox -e py27 and ignore the rest.

epmoyer commented 4 years ago

py27 was failing during dependency installation; looks like hashlib imports failing. Still getting the same errors when I run tox -e py27.

Not your fault I imagine; python packaging / deployment is the worst :) It shouldn't be so hard to replicate tests on another machine, but invariably is.

Still, will take me some mucking about to sort out; at the moment I am in crunch mode to get this presentation finished today, so I won't be able do dig in until early next week. Maybe I can get over the hump by running in a clean 2.7 Conda environment, or running this build/test on a linux vm.

epmoyer commented 4 years ago

Let's figure out the "requirements" for the new option. I didn't see a good lower case option so I was thinking upper case -B with the form:

-B BASE, --base=BASE

Where BASE (for the moment) can be one of ["default", "wide16x9"]

It doesn't make sense to me to allow someone to pass a path and specify their own custom "base", since they can effectively achieve any customization goals by creating their own custom theme (directory) and overriding any/all of the base there.

The default (when -B is not given, or -B default is given) would be the 4:3 base, so the new release wouldn't break anyone's existing deck build scripts.

I think the dist/examples build should then create decks for the 4 existing styles in both bases (so, 8 decks total), and the footer links in the example decks should link to all 8 decks.

What do you think?

epmoyer commented 4 years ago

Okay, not ready to merge yet, but I pushed an "experimental" command line option change.

Open issues:

I changed the footer to reference all 8 decks (which required a little re-formatting so all the names would fit on the 4:3 slide), and changed tox to build all 8 decks.

default_slide_with_base_option

epmoyer commented 4 years ago

Also, do you believe that printing works currently? I never updated print.css to support 16x9, and presumably I should, but I've been experimenting with Safari/Chrome/Firefox and I can't get the 4:3 slide deck to print properly; I'd need to figure out how to do that before I can change/validate print.css for 16:9. Is there some trick or mode change or something special I should be doing to print, or is a regular File->Print supposed to work properly? Chrome comes closest to working for me, but the pagination is wrong whether I print to portrait or landscape.

ionelmc commented 4 years ago

I've only got printing to work in chrome cause only chrome has an useful (preview feature) pdf export output for printing. It's kinda wonky yes, you have to press the print button when certain display options are set (can't remember for sure but I think you have to be in expose mode to make the page breaks work properly).

epmoyer commented 4 years ago

Ok, as of the latest commit (6718cee) the PR is a "release candidate". Let me know what you think.

Cheers!

ionelmc commented 4 years ago

I wondered for a bit if the theme mod could also be applied to situations where default ain't the base theme, but then again, maybe it wouldn't be compatible anyway with other themes that start with brand new css.

Anyway, do I understand it correctly - wide16x9 is a copy of default's css with changes right? How come you changed the option to "mod" from "base", since it's technically a base theme?

epmoyer commented 4 years ago

Anyway, do I understand it correctly - wide16x9 is a copy of default's css with changes right?

Correct, but to be precise it contains modified copies of only those default files which needed to change, which ended up being 3 of the 4 css files.

How come you changed the option to "mod" from "base", since it's technically a base theme?

I changed it to mod from base because the new directory wide16x9/ is not a stand-alone base but rather contains modifications to the default/ base.

The end result (of using --mod when generating a deck) behaves like a "new" base theme, but really it is applying a modification to the default base theme. If the option were called --base then wide16x9/ should really be a directory containing a full set of base files, (i.e. the same 6 files in default/). But only 3 files require changes to implement wide screen (base.css, print.css, and screen.css), so the 3 other files would just be exact copies of the versions in default/, and that would make maintaining the code harder in the future (because if anyone needed to make changes to the other 3 files (base.html, slides.js, or theme.css) then they would have to make those changes to both copies (i.e. the one in default/and the one in wide16x9/).

That is the trade-off I was referring to in my post about the DRY Principle two days back. The DRY (Don't Repeat Yourself) Principle says that one wants to avoid repeating code (or data) because it makes maintainability more difficult.

The real vulnerability (to maintainability) of violating DRY isn't so much the effort associated with maintaining (future) changes in two places, but the risk associated with making (future) changes to one copy and (accidentally) neglecting to change the other.

epmoyer commented 4 years ago

Just checking in. I can go either way on the above. If you prefer the simplicity of --base over the maintainability of --mod then I will switch it. Your call.

ionelmc commented 4 years ago

So I'm including this as is, and I've added what I had in mind in the master branch. It may be bit broken but it has all the right bits: not repeating styling in the mod and some refactoring on the internals. I've removed the copy_theme option since it was broken anyway with the mod support.

ionelmc commented 4 years ago

Also, now with few changes there could be support for multiple "mods".

ionelmc commented 4 years ago

@epmoyer please give the master a try, I'd like a bit of testing before cutting the new release.

epmoyer commented 4 years ago

@ionelmc I am kicking the tires today. I like what you did with the css cascading; it completely didn't occur to me to override just the styles that needed modification; that is a much cleaner approach.

epmoyer commented 4 years ago

@ionelmc I did a bunch of testing (including regenerating my fairly complex wide slide deck with the new master) and I can successfully build an identical deck with a few issues:

Old behavior:

bash-3.2$ darkslide --version
darkslide 5.1.0
bash-3.2$ darkslide slides_PROCESSED.md -t not_a_real_theme --destination=presentation_UNRESIZED.html
Error: Theme not_a_real_theme not found or invalid
bash-3.2$

New behavior (master 455045b):

bash-3.2$ python -m darkslide slides_PROCESSED.md -t not_a_real_theme --destination=presentation_UNRESIZED.html
Adding   'slides_PROCESSED.md' (markdown)
Generated file: presentation_UNRESIZED.html
bash-3.2$

Overall this is an awesome release candidate; I'm very happy with where it landed. Kudos!

ionelmc commented 4 years ago

Did you also use the copy-theme feature by any chance?

epmoyer commented 4 years ago

I don't think I ever used --copy-theme. It's vaguely possible I used it at some point to get the original copy of default/ that I modified to create my custom theme, but more likely I just copied the default/ theme from my local pull of the git repo.

ionelmc commented 4 years ago

Well I fixed the missing errors and did a check for base.html. I'd like to make the templates use inheritance and have blocks for footer/header/whatever (that will make your custom template be future proof) but maybe someone else who has custom template could contribute that feature :P

epmoyer commented 4 years ago

:) Sounds good to me. Thanks!

ionelmc commented 4 years ago

Btw ... I did have a custom footer in one of my templates, but I just used slide markup, eg: https://blog.ionelmc.ro/presentations/things-to-try-with-a-tracer/slides.rst