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

:customcss: attribute for easy per-presentation CSS #85

Merged
merged 6 commits into from
Sep 8, 2016
Merged

Conversation

obilodeau
Copy link
Member

Check the documentation commit for details.

@mojavelinux
Copy link
Member

I like the capability, but we're changing the meaning of the linkcss attribute attribute here. Therefore, I think we should use a different attribute name. We could align with the deck.js converter and use the customcss attribute instead (or perhaps revealjs_customcss). If empty, it resolved to asciidoctor-reveal.css. If non-empty, the value provided is used.

@obilodeau
Copy link
Member Author

obilodeau commented Sep 5, 2016

Sounds fair. I'll go with customcss (instead of revealjs_customcss) since this is not related to an upstream feature.

@obilodeau obilodeau changed the title :linkcss: allows for easy per-presentation CSS to be inlined :customcss: attribute for easy per-presentation CSS Sep 5, 2016
@mojavelinux
Copy link
Member

👍

@cmoulliard
Copy link
Member

Don't forget to also update the jade template !

@obilodeau obilodeau modified the milestone: 1.0 Sep 6, 2016
@obilodeau
Copy link
Member Author

@cmoulliard as I said in #63, I won't do the jade templates. I love asciidoc[tor] and I love sharing about infosec with nice HTML presentations but I don't have enough time to learn properly ruby and javascript plus slim and jade/pug. I already feel my contributions are not idiomatic to a ruby programmer (being a Python/Perl "programmer") and I don't want to double that mess. Someone needs to step in to do the jade thing or we need to switch to a ruby/javascript accessible template engine.

@obilodeau
Copy link
Member Author

@mojavelinux I implemented what we discussed and added a test case. From my perspective, this is ready to go.

@@ -65,6 +65,10 @@ html lang=(attr :lang, 'en' unless attr? :nolang)
document.write( '<link rel="stylesheet" href="#{revealjsdir}/css/print/' + ( window.location.search.match( /print-pdf/gi ) ? 'pdf' : 'paper' ) + '.css" type="text/css" media="print">' );
- unless (docinfo_content = docinfo :header, '.html').empty?
=docinfo_content
- if attr? :customcss and (attr :customcss).empty?
Copy link
Member

Choose a reason for hiding this comment

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

Here's my preferred way of writing this:

- if attr? :customcss
  link rel='stylesheet' href=((customcss = attr :customcss).empty? ? 'asciidoctor-revealjs.css' : customcss)

I'd also be open to:

- if attr? :customcss
  - if (customcss = attr :customcss).empty?
    link rel='stylesheet' href='asciidoctor-revealjs.css'
  - else
    link rel='stylesheet' href=customcss

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 noticed that you love the ternary operator :)

Copy link
Member Author

Choose a reason for hiding this comment

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

The test/assignment is very interesting, I'll try to remember that trick!

Copy link
Member

Choose a reason for hiding this comment

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

I love it to death. It helps cut down on a lot of lines of code and useless assignments. Combined with the assignment in the conditional, I can express meaning much more succinctly.

@mojavelinux
Copy link
Member

I already feel my contributions are not idiomatic to a ruby programmer (being a Python/Perl "programmer")

I think you're doing a fine job. We're learning together. Besides, we use a unique Ruby style in this project anyway, and I kind of like that.

@cmoulliard
Copy link
Member

Even if you can't maintain the templates in jade, it is important to list somewhere the changes applied to slim and/or jade.

@obilodeau obilodeau merged commit a49dd09 into master Sep 8, 2016
@obilodeau obilodeau deleted the linkcss branch October 2, 2016 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants