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

Feature: autogenerate agenda based on titles of slides #28

Closed
wants to merge 8 commits into from

Conversation

AlphaGit
Copy link
Contributor

@AlphaGit AlphaGit commented Sep 1, 2013

I thought it would be cool to have cleaver autogenerate for you a slide with an agenda (or a table of contents) of the slides that you had in your presentation.

So I went ahead and took the liberty of implementing it.

Feel free to provide any criticism or improvement, I would love to make my contributions useful.

@jdan
Copy link
Owner

jdan commented Sep 1, 2013

Hm, I'm unsure about this indent/unindent business. examples/basic.md generates the following agenda:

  • Cleaver 101
    • A first look at quick HTML presentations
      • A textual example
      • A list of things
        • A fourth level title

Which isn't really how the presentation flows. Each of those items (the second item being removed because it's a subtitle) should have the same level on the agenda. I don't quite see where nesting fits in.

I like this feature as a "table of contents" of sorts, though.

@AlphaGit
Copy link
Contributor Author

AlphaGit commented Sep 1, 2013

I think "table of contents" is a more appropriate name, now that you mention that.

As for the nesting, the ones that currently seem out of hand are the title and subtitle. Do you see any usefulness in having hierarchy at different levels? I personally do. The other three items ("A textual example", "A list of things" and "A fourth level title") have in fact different levels.

Let me know what you think. If you think it would be more useful in some other way, let me know. The bulk of the work is done, so I believe modifying it should not be a big deal.

Thanks for taking a look at it. =)

@jdan
Copy link
Owner

jdan commented Sep 1, 2013

Just from a slideshow perspective, I imagine an "agenda" or "table of contents" listing each slide title as a bullet point, without nesting and without including the title slide.

One potential use-case could be title slides acting as higher-level bullet points with all normal slides inside of them, but that seems more confusing than it needs to be.

  • Intro
    • Where we are
    • 2012 Sales
  • Where we're going
    • 2013 Sales (Q3)
    • New products
  • Credits
    • Salesman of the Month

As opposed to:

  • Where we are
  • 2012 Sales
  • 2013 Sales
  • New Products
  • Salesman of the Month

Pretty rough example, but it shows how slides shouldn't really have different levels of importance. Section "dividers" don't really need any presence in an agenda.

@AlphaGit
Copy link
Contributor Author

AlphaGit commented Sep 2, 2013

(Not sure if commits generate a notification. Just in case.)

I made the modifications based on what you mentioned. I believe that the code is much simpler now and feel more comfortable about it. Plus, it aligns better with what you had in mind.

var titles = [];

for (var i = 0; i < slices.length; i++) { // for each slice
var lines = slices[i].split(/(\n|\r)+/); // get every line independently of others
Copy link
Owner

Choose a reason for hiding this comment

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

Just curious, why do we need to loop through every line in each slice? Shouldn't we just assume the slide title is on the first line? (If it isn't, it won't render properly anyway)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very good question: I assumed that not only it may not be the first line, but a slide could actually have several titles. Maybe level 4, 5 and 6 titles could be part of the slide rather than the header? I didn't want to make a decision so I thought that going through each of them gave the flexibility for templates and layouts to change without breaking the agenda functionality.

Copy link
Owner

Choose a reason for hiding this comment

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

Hm, I see, so this is where the nesting stuff comes in. For simplicity's sake, I say we assume titles are on the first line. If we play with this feature some more and find that nested sections would be useful, we can always add it to a future release.

Copy link
Owner

Choose a reason for hiding this comment

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

Keep in mind that individuals can always manually create their own fancy agenda slide if they need that sort of functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I'll make the change right away.

@jdan
Copy link
Owner

jdan commented Sep 2, 2013

Thanks for pinging me, since pushes don't send out notifications. I appreciate you having an open mind to your changes, and I'm really looking forward to getting this merged - I think it's awesome.

Assuming the title of the slide (if any) will always be the first line of the slide.
@AlphaGit
Copy link
Contributor Author

AlphaGit commented Sep 2, 2013

There, it looks even simpler now (which is good). Thanks for the compliment, I hope I'm not coming as pushy at all (no pun intended). I believe this library is great and had some ideas -- I'm willing to change my original view so that it goes along with the roadmap and helps others as well.

@jdan
Copy link
Owner

jdan commented Sep 2, 2013

Made some slight modifications and merged in 6d43a7e. Thanks! ⭐ I also went ahead and filed #29 based on this if you want to take a stab at it :)

@jdan jdan closed this Sep 2, 2013
@jdan
Copy link
Owner

jdan commented Sep 2, 2013

Your fix landed in c004a8b, gracias :)

@AlphaGit
Copy link
Contributor Author

AlphaGit commented Sep 2, 2013

Es un placer. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants