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

Themed #16

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Themed #16

wants to merge 5 commits into from

Conversation

simensen
Copy link
Member

Update to Bootstrap 3 and moved all of the templates (the "theme") to an external theme package (sculpin/bootstrap-3-blog-theme) to showcase the new theme functionality.

There might still be some hiccups, but I think that most of the functionality that was originally available in the blog skeleton are still intact.


To create a theme, create a directory named sources/themes/{vendor-name}/{theme-name}. This is treated as sort of a virtual root on top of source. So you can put things in there like _layouts and _views and they'll behave the same way as they do in source. If a file exists in source directly it will be used before those in source/themes.

To reference external theme resources (assets like css or js) use the theme_path() twig function. For example, if theme_path("assets/css/style.css") is called and it exists in source/assets/css/style.css it will be used instead of the version in the theme.

assets/css/style.css should probably be standardized. In this case, we can use @import to fix things. For example:

@import url('../../themes/sculpin/bootstrap-3-blog-theme/assets/css/style.css');

This will ensure that your local project's css can inherit from a theme's style.css and build on it.

To change theme, app/config/sculpin_kernel.yml needs to be modified. Change the sculpin_theme.theme configuration:

sculpin_theme:
    theme: sculpin/bootstrap-3-blog-theme

To ship a Sculpin theme as a standalone package, use the sculpin-theme package type. It is not currently recommended to publish Sculpin themes on Packagist.

Here is the Bootstrap 3 Blog Theme: https://github.com/sculpin/bootstrap-3-blog-theme

To use it, configure sculpin.json to point to the repository directly.

{
    "repositories": [
        {
            "type": "vcs",
            "url": "git@github.com:sculpin/bootstrap-3-blog-theme.git"
        }
    ],
    "require": {
        "components/bootstrap": "3.0.3",
        "components/font-awesome": "4.0.3",
        "components/jquery": "1.9.1",
        "components/highlightjs": "~7.3.0",

        "sculpin/bootstrap-3-blog-theme": "dev-master"
    }
}

It is not currently recommended for themes to require other external assets like bootstrap or jquery. This should be up to the user to install. This will allow them to install these assets however they want. It is acceptable to note in the installation/README what they can use to easily install these types of things, though.

Update to Bootstrap 3 and moved all of the templates (the "theme") to an
external theme package (sculpin/bootstrap-3-blog-theme) to showcase the new
theme functionality.

There might still be some hiccups, but I think that most of the functionality
that was originally available in the blog skeleton are still intact.
@simensen
Copy link
Member Author

To test this out, make sure you are either running an updated version of sculpin from dev, downloaded a fresh sculpin.phar, or do a self update. Then:

git clone git://github.com/simensen/sculpin-blog-skeleton.git sculpin-blog-themed
cd sculpin-blog-themed
git checkout themed
sculpin install
sculpin theme:list
sculpin generate --watch --server

If all goes well you should see something like this:

image
http://d.pr/i/lYGR

@simensen
Copy link
Member Author

I changed some of the functionality here. I've decided to do this:

{% set stylesheet %}{{ theme_path("assets/css/style.css") }}{% endset %}
{% set theme_stylesheet %}{{ theme_path("assets/css/style.css", true) }}{% endset %}
{% if stylesheet != theme_stylesheet %}
    <link href="{{ site.url }}/{{ theme_stylesheet }}" rel="stylesheet" media="screen">
{% endif %}
<link href="{{ site.url }}/{{ stylesheet }}" rel="stylesheet" media="screen">

I need to think this through a bit more but the goal here is to allow for easier local development by allowing users to not have to tweak their style.css when they switch their theme. This works for a simple case, but I'm not sure how well it works in the case of parent themes...

I think this is going involve adding a few more features to the Sculpin Theme Bundle's Twig extension. Essentially, we should find all css files that actually exist, in order, and then include each of them. This makes a LOT of assumptions so I'm not sure if that is the best way to go...


I experimented with removing components as @mavimo suggested. This is fine and it makes some things nicer... the downside is that the CDN's I am using seem to be doing weird things and get stuck sometimes. It also means local development is no longer an option. Not sure how I feel about it... but it is maybe a nicer solution for people just to get started. They can still install the assets locally in their project if they want to.


Experimenting with addons. Not sure what to call them. Basically, I moved highlight.js to be an addon. There are includes_after_footer and includes_head that you can use to pop new files on and they'll be included in the order they are listed.

If this works out, we should make extensions able to manipulate this so we could ship a highlightjs-sculpin-bundle that could automatically configure itself and expose its includes that way.

@simensen
Copy link
Member Author

Alright, I think I'm close to being ready to merge this.

First, for doing theme development I wanted to try and find a nice way to let people work on stuff without having to change their files. So the new rules are this....

Assuming you have selected a theme named acme/awesome-theme and you create a copy in acme/awesome-theme-dev, it will automatically use the dev version for you without having to update your local files in any way.

So for example, if Sculpin installs into source/themes/awesome-theme, and you want to make changes, you can copy (or clone) into source/themes/awesome-theme-dev and those files will automatically be used instead.


I added a theme_paths Twig function. This returns all actually existing fils for the requested resource. This is most obviously useful when you want to do something like include assets/css/style.css from all of a theme's parent, the theme itself, and local files... but only if they exist.

It will return an empty array if no files are found anywhere. This means the API is pretty easy:

{% for stylesheet in theme_paths("assets/css/style.css") %}
    <link href="{{ site.url }}/{{ stylesheet }}" rel="stylesheet" media="screen">
{% endfor %}

@simensen
Copy link
Member Author

@mavimo FWIW I switched to another CDN for the theme and it works much better. Thanks for the suggestion, I apologize I dismissed it so quickly the other day.

@reinink
Copy link

reinink commented Jan 16, 2014

This is looking great @simensen!

Question, are you seeing source/themes being excluded from a repository via .gitignore? I guess the answer probably depends on whether or not you include the theme as a package, right?

@simensen
Copy link
Member Author

@reinink I imagine source/themes should be excluded by default and if for some reason you want to add a theme manually (i.e., not managed by composer) then you can override that and put your theme into git but keep others out?

I think the point of having composer manage the themes is that you shouldn't check them into vcs. Similar to how for the most part you don't want to have vendor checked into vcs.

The problem I ran into was while trying to do theme development. In that case you might want to actively work on a theme that is under vcs (even if another repo) under source/themes. That is why I worked on the {theme-name}-dev magic.

So in that case, you could develop your theme in its own repo. For me, I did something like this:

 + -- workspaces/
    +-- srcmvn.com/
    | +-- source/
    |   +-- themes/
    |     +-- sculpin/
    |       +-- bootstrap-3-sculpin-theme (installed via embedded composer)
    |       `-- bootstrap-3-sculpin-theme-dev (ln -s workspaces/bootstrap-3-sculpin-theme)
    |
    `-- bootstrap-3-sculpin-theme (live theme development here)

There are still some fiddly bits here. For example, if you do a production build with this configuration it will still use the -dev theme. Not sure what the right thing to do there would be. :)

Anyway, for your purposes, I'd see creating a theme named loep/loep-sculpin-theme. You would require that in a site's sculpin.json. In order to do development on the theme itself with an existing site, you'd want to clone the loep-sculpin-theme somewhere else, and them symlink it as loep-sculpin-theme-dev in source/themes/loep.

This will allow Sculpin to work for everyone else who clones your package (they'll get the actual theme from GitHub in source/themes/loep/loep-sculpin-theme) but if you want to do work on the theme in the context of an actual site, you could clone the theme itself somewhere and just append -dev to the directory.

It works well in my head. :) I need to find out how to document it without confusing people. Would also be nice to get someone working with it to validate my ideas. I think it will work well but I've only been working on one theme so far and I'm admittedly pretty lenient on the hoops I have to jump through since it is awesome (to me) but also it keeps getting better than it was before... doesn't mean it is good, though. ;)

@reinink
Copy link

reinink commented Jan 16, 2014

@simensen Sounds great. I definitely want package up the League's theme, and then pull it in with Composer. This is brilliant.

I now see how the actual theme development (as a separate package) could be a little tricky. I'll have to experiment with that a little.

@reinink
Copy link

reinink commented Jan 17, 2014

@simensen A quick question for you. Is there a way to check if an image exists, and if it does show it, otherwise don't? Basically for an image that there would be no default theme image for, but it could exist in the project itself. Something like this?

{% if path_exists("img/logo.png") %}
    <img src="{{ site.url }}/{{ theme_path("img/logo.png") }}" alt="{{ site.title }} - {{ site.tagline }}">
{% endif %}

@simensen
Copy link
Member Author

@reinink ah, that is a good use case. i almost wrote something like that but wasn't sure what the use case would be. what i think might fit with the existing twig functions would be theme_path_exists:

{% if theme_path_exists('img/logo.png') %}
    <img src="{{ site.url }}/{{ theme_path("img/logo.png") }}" alt="{{ site.title }} - {{ site.tagline }}">
{% endif %}

i was also considering changing things around entirely to do away with theme_path with that sort of usage. basically, it would work like theme_paths and would always return an array, but the array would either have 0 or 1 entries and never multiple.

for example:

{% for path in theme_path('image/logo.png') %}
    <img src="{{ site.url }}/{{ path }}" alt="{{ site.title }} - {{ site.tagline }}">
{% endfor %}

if the path exists, it would return array('path') and if not, it would return array().

i kinda like this syntax but it would add extra boilerplate for everyone so i don't know it would be the best idea.

thoughts?

@reinink
Copy link

reinink commented Jan 17, 2014

@simensen Hmmm, hard to say which would be better. When I see a for loop, I assume multiple results. However, at the same time I like the consistency that would bring between between theme_path and theme_paths. Basically use theme_path if you only want one file, and use theme_paths if you want multiple—but both return an array.

Or you could modify theme_path to simply return false if it doesn't exist. I think that is a pretty natural assumption...in fact that's what I tried first.

When you want one file (a logo for example):

{% if theme_path('img/logo.png') %}
    <img src="{{ site.url }}/{{ theme_path("img/logo.png") }}" alt="{{ site.title }} - {{ site.tagline }}">
{% endif %}

When you want more than one (CSS files are a good use case):

{% for stylesheet in theme_paths("css/styles.css") %}
    <link rel="stylesheet" href="{{ site.url }}/{{ stylesheet }}">
{% endfor %}

Either approach is fine with me. I'd probably lean to second option here, simply because it reads more logically. Also, if someone for some reason wanted a default value set when a path isn't found, they could always do this:

{% if theme_path('img/logo.png') %}
    <img src="{{ site.url }}/{{ theme_path("img/logo.png") }}" alt="{{ site.title }} - {{ site.tagline }}">
{% else %}
    <img src="img/logo.png" alt="{{ site.title }} - {{ site.tagline }}">
{% endif %}

Honestly, I don't see that happening very often...at least I cannot think of any use cases right now.

@simensen
Copy link
Member Author

@reinink Yeah, my main reason for having theme_path always return a result was because it would manifest itself logically in the error logs. For example, {{ theme_page("images/logo.png") }} would fall back to images/logo.png and if someone makes that request and it doesn't exist, they'd see it in their logs pretty easily. On the other hand, if it returns false or null or '', the browser would silently be making requests to /.

Alternately, we could do theme_path('path', true|false) for whether or not it should return null if the path doesn't exist or return the requested resource raw? Default would be to just return the resource... then just need to figure out if it makes more sense to have true or false as the second argument. :)

So I think the options I'm seeing right now:

  • theme_path_exists or theme_check that returns true/false based on whether the resource exists anywhere
  • Change theme_path to always return null/false if the resource doesn't exist anywhere
  • Change theme_path to have an additional argument to return null/false if the resource doesn't exist

@reinink
Copy link

reinink commented Jan 17, 2014

@simensen Good point about the errors. At this point I actually think all three options are decent. I'd probably be inclined to stick with the current functionality and then add theme_path_exists for those who want to do a check. The problem with the extra boolean parameter is that it will be very unclear what it's for when viewing a template.

@simensen
Copy link
Member Author

As of eefe67c80399fea29513a0b99ee82b2d1223be66 theme_path_exists is a thing. It only returns true if a file actually exists, unlike theme_path which will always return a value even if it does not exist on disk.

@ghost
Copy link

ghost commented Feb 15, 2014

bump

@drgomesp
Copy link

@simensen, any updates on this? Is there the possibility of using highligthjs 8.0.0?

@georgiana-gligor
Copy link

bump :-)

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

4 participants