jdan / cleaver

30-second slideshows for hackers
http://jdan.github.io/cleaver
MIT License
4.03k stars 305 forks source link

Advanced templating using remote templates from github repos? #58

Closed iamstarkov closed 10 years ago

iamstarkov commented 11 years ago

I developed Bochar and now I think that exists more efficient way to customize Cleaver. What do you think about method use to setup custom themes from github (user/repo or organization/repo)?

$ cleaver use Shower/Ribbon
$ cleaver use Shower/Bright

And resetted to default theme like this

$ cleaver use default
iamstarkov commented 11 years ago

What fo you think about it?

jdan commented 11 years ago

Do these correspond to stylesheets, templates, or both? I want to keep everything in one document (no command-line options), and we could possibly add support for remote templates (specified by a URL) to the style and template fields.

Does that sound like a good solution?

iamstarkov commented 11 years ago

Do these correspond to stylesheets, templates, or both?

Both

I don't think that separate fields are good idea because it's complicated way. I like one field theme corresponding to repo with strictly defined structure of files like this: layout.mustache, styles.css and script.js which will be replace files fromt default theme

iamstarkov commented 11 years ago

What do you think about it?

jdan commented 11 years ago

I think it's a good idea. Would solve my issues with #56 as well.

iamstarkov commented 11 years ago

nice, I will try to implement this.

I hope that this feature will help Cleaver became even more popular due to the ability to use any theme

jdan commented 11 years ago

I'll be working on this today - I have a few ideas I want to try out :) I'll update this issue with my progress

jdan commented 11 years ago

Put a lot of work into this last night :) Check out the themes branch, where you can add a themes property to the metadata.

You can either put a folder, URL, or a github repo (username/repo)

theme: jdan/cleaver-github

style, template, and layout are all still supported, and will only be overwritten if the theme contains those files. I'm thinking it should be the other way around, where style, template, and layout will overwrite theme.

Anyway, I'd appreciate if you checked it out and reported any weird behavior. I'll add some documentation before merging.

iamstarkov commented 11 years ago

you are awesome! I will check it out in few hours

iamstarkov commented 11 years ago

at first sight all works perfectly

iamstarkov commented 11 years ago

Now I will try out my custom theme

iamstarkov commented 11 years ago

I was wrong, I forgot to run Cleaver with themes branch

iamstarkov commented 11 years ago
  1. I have Cleaver cloned and initialized (npm i) in /Users/vstarkov/projects/cleaver also I check out themes branch.
  2. Cleaver-github cloned repo here: /Users/vstarkov/projects/cleaver-github
  3. My presentation is here: /Users/vstarkov/projects/SHRI
  4. Settings
    • Setting's string №1: themes: ../cleaver-github
    • Setting's string №2: themes: jdan/cleaver-github

Running Cleaver like this (in one folder with presentation): ../cleaver/bin/cleaver lection3.md

I got some strange error:

!! ENOENT, open '/Users/vstarkov/projects/cleaver//lection3.md'

When I removed themes: jdan/cleaver-github nothing changes.

Also ../cleaver/bin/cleaver lection3.md worked fine when I checkout cleaver to master branch — I tested my previous PR in this way and it worked.

iamstarkov commented 11 years ago

How do you test your development version of Cleaver?

jdan commented 11 years ago

Thanks for catching this :) I patched it in 60f1de4746de6f7e445f1e20217e14fb14e2c11a.

There are currently no unit tests, which is a bummer. I will be working on them over the next few days.

iamstarkov commented 11 years ago

I have already seen your commit in my github feed. =)

seems working.

Continue to testing =)

iamstarkov commented 11 years ago

Are you sure that property name is themes? themes not working opposite to working theme property's name

iamstarkov commented 11 years ago

Local theme is working

Remote theme is working

jdan commented 11 years ago

Themes is just the branch name, theme is actually property.

iamstarkov commented 11 years ago

Yep, I understand, but I was confused with this statement in your previous messages:

themes: jdan/cleaver-github
iamstarkov commented 11 years ago

Now I will try to implement custom theme =)

iamstarkov commented 11 years ago

If I use nonexistent path Cleaver silently degrades to default theme with no error — I think this behaviour is potentially error-prone, because of no information given to user and user will wrongly think that all is ok

jdan commented 11 years ago

It's a trade-off, because username/repo looks like a path which probably doesn't exist. I first check that as a path, and then if the path doesn't exist, I assume it's a github path.

I'm not sure what error I would display, because I can't tell if the user wants a path or a github repository.

iamstarkov commented 11 years ago

Maybe something like this:

$ cleaver lection3.md
Note! Cannot use custom theme defined in slideshow settings {theme_value}. Degrading to default theme.

This message will give enough information about situation in which Cleaver is forced to use default theme

iamstarkov commented 11 years ago

I found issues, resolved them and sent #60 PR:

  1. Cleaver styles conflicts with custom theme styles [fixed]
  2. {{author.url}} and {{author.name}} are accessible from author's slide but in custom theme it's can be required in any place. [fixed]
  3. Also themes can has own design requirements and it's be usefull to give freedom of passing any defined in md-file variables to theme scope. [fixed]
  4. Custom themes also can require custom JavaScript so maybe {{navigation}} should be renamed to more common {{JavaScript}} [fixed]

Also, I take out script.js from slides.mustache and passed it into layout.mustache because it would be done by most of developers and this movement help Cleaver to avoid many silly issues about not working {{script}}

PS. With all of these changes Cleaver will never got new issues about adding any new option or about theme improvement. It will make the world better and give you more spare time.

iamstarkov commented 11 years ago

Also, In my implementation Cleaver trying to use style.css and script.js from custom theme without appending them to default styles and scripts because custom themes is separate and self-contained entity.

May be some boolean appending options can be added to themes to solve this potential issue

iamstarkov commented 11 years ago

You can try matmuchrapna/cleaver-ribbon to see some ribbon magic.

iamstarkov commented 11 years ago

with new changes your theme jdan/cleaver-github was broken — I sent PR with updates

iamstarkov commented 11 years ago

Some shortcuts to copy-past:

theme: matmuchrapna/cleaver-ribbon
theme: jdan/cleaver-github
iamstarkov commented 11 years ago

What do you think about all of these changes?

jdan commented 11 years ago

They look good! I'll merge tonight and hopefully I'll get themes out soon.

jdan commented 11 years ago

60 is merged. I will add an error message if no data was returned when searching for a theme. Let me know if you find anything else :) Right now this looks great and I'm pretty happy with how it's turning out.

iamstarkov commented 11 years ago

I will try it out today evening

iamstarkov commented 11 years ago

I have two other needs for easy and flexible theming:

  1. Can we overwrite agenda and author templates with custom theme? Maybe author template can be incapsulated in slideshow templates?
  2. Can we make author slide optional even if user defined author field in md-file? Because custom theme can not require author slide (author information used in out-sildes template) or in some cases this slide need to bee first.
iamstarkov commented 11 years ago

After some hours I think that we can remove author.mustache from Cleaver and delegate responsibility for this slide to themes. What do you think about it?

iamstarkov commented 11 years ago

Also maybe shortcut metadata to to some of short synonyms (metadata, meta, options, o)?

var layoutData = {
    slideshow: slideshow,
    title: title,
    encoding: encoding,
    style: style
    style: style,
    author: this.metadata.author,
    script: script,
    metadata: this.metadata,
    meta: this.metadata,
    options: this.metadata,
    o: this.metadata,
};

Which should me more nice to use:

    {{metadata.company}}
    {{meta.company}}
    {{options.company}}
    {{o.company}}

I prefer meta or even o to be shorter and easier to type

jdan commented 11 years ago

I will likely change the functionality of agenda and author slides to insert Markdown into the slices array, that way they will be rendered exactly like other slides.

As for a renaming, I'll play around with it and see if anything works.

jdan commented 11 years ago

Thanks again for testing this out, I hope to polish it up today or tomorrow.

iamstarkov commented 11 years ago

I will likely change the functionality of agenda and author slides to insert Markdown into the slices array, that way they will be rendered exactly like other slides.

If I understand you right, there will be no way to markup of author slide?

iamstarkov commented 11 years ago

My congratulations with first thousand of stars on github! :clap:

jdan commented 11 years ago

Thanks, @matmuchrapna :)

I just added a feature to the themes branch which will, by default, append styles to the built-in styles. This is because most themes, in my opinion, will be small style changes - which I feel don't merit the entire stylesheet to be rewritten.

If you want to completely override style.css and script.js, you will need to add a file called settings.json like the following:

{
  "override": true
}

The layout and slide templates will still be overwritten. Let me know what you think.

iamstarkov commented 11 years ago

Firstly, overrideshould be override_css to not mislead developers.

It is very good to give choice. Also every stick has two ends and we need to wisely choose default value. In our case choice based on theme's stability and possible developer's comfort (while while developing smart themes customising default theme).

Why I am talking about theme's stability? When I was implementing this feature I thought that developer who do not want to replace entire theme will copy Cleaver default styles and append his own styles to default ones. This way allows not to depend from Cleaver default theme's update, which can broke custom theme's stylesheet — so full rewriting will be more stable solution. Also rewriting way will interfere with themes, which only adding some customising lines.

Anyway, we don't know which type of themes community will like and develop more: full overridden themes or smart theme which will override only some css details.

I think full overridden themes would be more popular (also this default value will give some more stability).

iamstarkov commented 11 years ago

Polite ping =)

jdan commented 11 years ago

Slowly getting there - been a busy week.

iamstarkov commented 10 years ago

Also, today I learn that node in the core require path to be prefixed with this statements:

./ — (dot + slash) for relative paths • / — (slash) for absolute paths

I think Cleaver can check this prefixes to find local themes else find github repo — this should be faster

jdan commented 10 years ago

@matmuchrapna Cleaver already does this.

iamstarkov commented 10 years ago

Nice

jdan commented 10 years ago

:confetti_ball:  https://github.com/jdan/cleaver/releases/tag/v0.5.0 :confetti_ball:

iamstarkov commented 10 years ago

It's awesome! :gem: