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

Move postsPerPage to be a theme-level config option #8131

Closed
ErisDS opened this issue Mar 11, 2017 · 5 comments · Fixed by #8149
Closed

Move postsPerPage to be a theme-level config option #8131

ErisDS opened this issue Mar 11, 2017 · 5 comments · Fixed by #8149
Assignees
Labels
feature [triage] New features we're planning or working on

Comments

@ErisDS
Copy link
Member

ErisDS commented Mar 11, 2017

Ghost 1.0 is all about fixing things that annoy us or get in our way 😁

One of those things, is the "Posts per Page" setting in the admin panel. It tells a blog how many posts its allowed to output on a particular page. However, it makes little sense for this to be a global blog-level setting because this is presentation logic.

Presentation logic belongs in a theme, and ideally, it should be themes that control how much content appears on a page, in order to suit that themes particular layout. E.g. a 2 column theme might want an even number of pieces of content.

The version of this we want to ship for Ghost 1.0 involves adding a new concept of theme config via the package.json file.

Example:

"name": "casper",
"version": "1.3.6",
 "theme": {
        "ppp": 2
    }

As a next step, we're considering adding frontmatter to templates, so that this (and any other theme config we come up with) can be set on a template-level basis.

UPDATE: the spec has been updated:

```
{
"name": "casper",
"version": "1.3.6",
...rest of package.json...
 "config": {
        "posts_per_page": 2
    }
}
```

AND you can access this property inside a theme with:

`{{@config.posts_per_page}}`


@ErisDS ErisDS added the themes label Mar 11, 2017
@ErisDS ErisDS self-assigned this Mar 11, 2017
@kirrg001
Copy link
Contributor

+1 for postsPerPage

@ErisDS
Copy link
Member Author

ErisDS commented Mar 13, 2017

@JohnONolan I've got this implemented and as a result have a couple of bits of the spec that I need a little bit of clarity around, mostly to do with naming-things.

Thing 1 - package.json

In package.json, I'm adding a new object like this:

"name": "casper",
"version": "1.3.6",
...more stuff...
 "theme": {
        "ppp": 2
    }

Ideally I'd like to have some sort of top-level key to contain the theme's config here as that will keep my implementation minimal and easy, assuming we will come up with more things to put here, however I guess theme doesn't make that much sense, so perhaps it should be config.

Secondly, I've called the setting ppp just cos that was fast to type. To be consistent with the rest of package.json, if we use multiple words we should camelCase. Any idea on what you preferred name for this config option is?

Thing 2 - handlebars

Back in Ghost 0.7.3 we added both permalinks and the post per page setting available to the theme via the @blog global.

E.g. {{@blog.permalinks}} and {{@blog.posts_per_page}}
Note: handlebars/theme API uses snake_case.

Now that posts per page is no longer a global blog setting, should it be removed from here? I made a bunch of assumptions, assumed we'd want it to become part of a new @theme global and implemented it as {{@theme.posts_per_page}} but I can also put it back. These changes are trivial.

Thing 3 - the default

It's currently set to 5, which is what it was before. Every blog that USED to have a setting with a different number will lose that configuration in Ghost 1.0. We could implement some really complex migration, but that seems totally unnecessary seeing as 1.0 is meant to have breaking changes.

TLDR;

  • what should the object in package.json look like? what: {what: 5}}
  • what should the handlebars helper look like? {{@what.what}}
  • what should the default be? (it's 5 right now)

This was referenced Mar 13, 2017
@JohnONolan
Copy link
Member

  • config: { postsPerPage: x}
  • @config.postsPerPage
  • 5

?

@ErisDS
Copy link
Member Author

ErisDS commented Mar 14, 2017

Consistency clash: package.json uses camelCase, our theme API uses snake_case. I 100% agree that whatever we pick it makes sense to make them consistency. However, when it comes to which standard to break, I would much, much, much, much rather break general consistency with package.json than with the theme API.

So can I raise you:

  • config: { posts_per_page: x}
  • @config.posts_per_page
  • 5

@JohnONolan
Copy link
Member

fine w/ me

@ErisDS ErisDS added the feature [triage] New features we're planning or working on label Mar 14, 2017
kirrg001 pushed a commit that referenced this issue Mar 14, 2017
closes #8131

- Remove ppp from default-settings.json
- Remove ppp from meta (unused?\!)
- ✨ Basic concept of theme config
- use theme config ppp setting
- ✨ Make @config.posts_per_page helper available
- rather than @blog.posts_per_page, we now have @config.posts_per_page
- 🚨 Test updates
- Adding TODO note
kirrg001 added a commit to kirrg001/Casper that referenced this issue May 31, 2017
refs TryGhost/Ghost#8131, refs TryGhost/gscan#40

- we would like to show a recommendation for theme validation
- this recommendation could look like "package.json `config.posts_per_page` is recommended. Default is 5`
aileen pushed a commit to TryGhost/Casper that referenced this issue Jun 1, 2017
refs TryGhost/Ghost#8131, refs TryGhost/gscan#40

- we would like to show a recommendation for theme validation
- this recommendation could look like "package.json `config.posts_per_page` is recommended. Default is 5`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature [triage] New features we're planning or working on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants