search

schacon / showoff

moved to puppetlabs/showoff!
2.41k stars 13 forks source link

Let's start a discussion #184

Open burtlo opened 12 years ago

burtlo commented 12 years ago

A few weeks ago I started look at ways I could improve upon Showoff by adding:

I think after that I went off the deep end trying to clean up and refactor the monolith sinatra class that contained all the parsing and rendering logic. At some point I questioned the usefulness of a showoff.json file and replaced with a showoff file which contained a DSL that allowed you to define sections and slides.

The README describes in far greater detail this implementation.

Final Note

This pull request is unreasonable in size, scope, and change. It is likely near impossible to visibly inspect it to gain a sense of it. I am absolutely not sure if there is a home for this abomination. However, I felt like freeing this Frankenstein...

goncalossilva commented 12 years ago

Wow.

I really think we should join efforts into improving showoff (vs forking it). Thanks for all this!

First of all, I couldn't agree more regarding GitHub's flavored markdown. It's the way to go, imo. I also like the stuff you did with static page & PDF generation. And finally, great work shrinking showoff.rb!

Having said this, I see a few issues with this pull request:

Let's discuss this a bit more!

alexch commented 12 years ago

FYI I'm kinda doing some of this already with deck.rb, which is based on a cleaner JS presentation layer and an arguably cleaner and better tested Ruby tool. http://github.com/alexch/deck.rb

(I'm not using full GFM but I'd love to be, and I'm inlining a lot of the JS and CSS too. Also it should be drop-in-compatible with showoff since I support showoff.json files so I'd love for more people to just give it a try and tell me what breaks.)

burtlo commented 12 years ago

@goncalossilva this is to start a conversation, so don't feel too bad about the work I have done on the fork coordinating with your work. Thanks for taking a look at it and your feedback.

Supporting showoff.json

I am fairly certain that I could build an adapter to parse the previous showoff.json format.

I think the new format, though Ruby, is a little easier to read and possibly to understand for non-developers. There are overall less symbols { , }, [, ], : in the new format.

README

That of course would change if you are seriously considering this monstrous body of work.

Missing Features

Markdown Engine Configurability

I likely have broken the markdown engine configurability because I have never navigated this minefield before and when it came time to get Github Flavored Markdown (GFM), I simply opted for a solution that I found that worked. Namely using Redcarpet with Pygments.rb. Perhaps there is a way to restore that functionality

Presenter View and Presentation View

That functionality still exists and is working but is indeed missing from the documentation I composed.

Presentation Templates

I'm not sure what you mean by presentation templates? Are those the classes that can be applied to slides? The ability to request hostname:9090/onepage or hostname:9090/pdf?

burtlo commented 12 years ago

Alright, both the server and the static generation will look for a showoff then the original showoff.json.

Also I took a moment to make the server and the static generation use the full filepath provided and not simply a directory.

So you could do something like:

$ bin/showoff server examples/showon
$ bin/showoff server examples/custom.json

Which will add those files to the list of files to search for when creating the presentation.

burtlo commented 12 years ago

@goncalossilva let's talk about what you would love to see in a series of smaller pull requests:

I am running into some problems pushing this functionality to Heroku because of some python issues. I'll see what I can do to make this work.

  • Multi-column layout

This was something that I was working on in the last few commits.

  • Updated Example Presentation

I wanted to update the example presentation so that it actually show-cased Showoff and the majority of it's features.

  • RSpec test suite

How do you feel about having an RSpec test suite backed with Guard?

  • Reduced showoff command

I removed a large set of the showoff commands so that showoff could focus ensuring high quality of core commands. I also made the showoff command work when simply given a directory or filepath.

goncalossilva commented 12 years ago

@burtlo In parts:

Markdown Engine Configurability I'll try to take a deeper look into this in the near future.

Presentation Templates I'm talking about this: https://github.com/schacon/showoff#templates

RSpec test suite Any test suite is better than no test suite :p

The rest seems fine. Your PR breakdown looks good to me. After ironing out the questions about the MD engines and the templates, I think we're good to go.

I think this is the best approach:

These are big changes. I'm strongly in favor of most of them. Let's see if @schacon can weigh in on this!

burtlo commented 12 years ago

Oh templates! I must have forked before templates. I'll work on how that would integrate with the work I've done.

goncalossilva commented 12 years ago

There were a few commits after that one :|

Everything after 580c02595bbc422844d0769e95214f6f3411ba4c (https://github.com/schacon/showoff/commits/master?page=2).

burtlo commented 12 years ago

Looking over the current template implementation, I'm not sure why the templates aren't simply using erb. Would you be opposed to using ERB?

<div style='background-color: red;' id='<%= id %>' class='slide <%= slide_classes %>' data-transition='<%= transition %>'>
  <div class='content <%= content_classes %>' ref='<%= reference %>'>
    <%= content_as_html %>
  </div>
</div>
goncalossilva commented 12 years ago

Not at all. Did you check out the rest of the commits?

burtlo commented 12 years ago

Yes, I'm looking them over.

burtlo commented 12 years ago

I had a question about the full-page slide class.

I took it to mean, from the description, that it should take all images on the slide and center and enlarge them:

full-screen-in-my-branch

In the current implementation it looks like the following:

full-screen-original

However, full-page does definitely sound like it should break out of the slide mold and show it full-width.

goncalossilva commented 12 years ago

Hum. Tough choice. In my opinion "full-page" doesn't mean "enlargement", but this is just my opinion. What about having them both with different names?

PS. Off the top of my head, I just remembered Android dev. When using images, there are various types of scale types (http://developer.android.com/reference/android/widget/ImageView.ScaleType.html)... we're basically discussing if "full-page" means CENTER_INSIDE or CENTER.

burtlo commented 12 years ago

@goncalossilva I'll return it to the latter option, as the enlargement wasn't great, just what I thought of initially when I saw the text.

I think I have included most of the fixes and pull requests from the past three months; except for the multiple markdown support. I will take a look at that latter this evening.

burtlo commented 12 years ago

To further clean up the project, I have started to tackle the javascript. I have been trying to create a more modular system with classes that employ pub/sub.

https://github.com/burtlo/showoff/blob/js-refactor/lib/public/js/showoff.js

I am not done with it yet, but getting pretty close.

burtlo commented 11 years ago

I took a break and have now returned to working on these changes. I believe I am going to fork this into a new gem.

alexch commented 11 years ago

I agree that showoff's JS needs work, but instead, I've been using deck.js and it's great; it's fully unit tested with Jasmine and with my deck.rb wrapper gem, it's mostly backwards-compatible with showoff (e.g. understands showoff.json files).

https://github.com/alexch/deck.rb

burtlo commented 11 years ago

@alexch I think deck.rb is still missing some fundamental things but I would love to continue this conversation further.

alexch commented 11 years ago

@burtlo So how about you make a "deck.rb is still missing some fundamental things" issue at http://github.com/alexch/deck.rb/issues and we can continue this conversation further :-)