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

Collections.all only lists first page created with pagination, not others #253

Closed
edwardhorsford opened this issue Sep 25, 2018 · 17 comments
Assignees
Milestone

Comments

@edwardhorsford
Copy link
Contributor

edwardhorsford commented Sep 25, 2018

Pages made with pagination don't all get added to collections.all. Only the first page created appears.

Similarly, if I apply a tag foo to the pages, collections.foo only contains a single item - even though i'm generating more than 100 pages with pagination and this tag.

This causes me some issues as I don't know how to easily list / iterate over the pages created with pagination.

@edwardhorsford
Copy link
Contributor Author

As a workaround I tried adding a default tag paged to the pagination template. However it seems like only the first page made by pagination gets the added to the the paged collection.

{{collections.paged | length}} returns 1, when in my case it should return 100.

@zachleat
Copy link
Member

zachleat commented Oct 7, 2018

Hmm this was by design but maybe it isn’t right.

If you are using pagination to provide numeric paged links to a very large data set the current behavior makes sense. But if you’re using pagination to provide individual distinct pages of a collection of data, this certainly makes less sense.

HMM.

@zachleat zachleat added the needs-discussion Please leave your opinion! This request is open for feedback from devs. label Oct 7, 2018
@zachleat
Copy link
Member

zachleat commented Oct 7, 2018

This was previously discussed at #76 (comment)

@edwardhorsford
Copy link
Contributor Author

I'm very much in the camp of using it to provide a page per thing. It's a limitation as I don't think it's otherwise easy to get access to the data you'd need to know about which pages have been created - or at least makes the code more complex as you have to separately iterate through each thing you do pagination on, and if you use collections.all, filter for the first item in that paginated set.

Similar for wanting to list pages in site-maps - which will be incomplete right now.

Conversely, it seems each collection you add gets listed in collections.all, even if they're not creating a page.

Currently I'm using pagination it for a page per tag, per image asset, and per date. It's super useful - a great feature.


@zachleat Slightly buried in my report is another (related) possible bug that if you add tags to the pagination template, the pages generated with pagination don't show up under that tag's collection.

@zachleat zachleat added bug and removed needs-discussion Please leave your opinion! This request is open for feedback from devs. labels Dec 21, 2018
@zachleat
Copy link
Member

zachleat commented Dec 21, 2018

After mulling it over sufficiently long, I'm on board with this. I'm filing as a bug because it doesn't currently work for Object based paginations but it might be called a feature request too. I think this should be a pagination option that can be changed on a per template basis. Thanks for pushing back on this @edwardhorsford.

@edwardhorsford
Copy link
Contributor Author

Super, glad to hear it!

Totally on board with it being an opt-in feature or whatever. To me it makes a lot of sense for dynamic pages, but not so much sense for paginated groups of things.

--

Related: will tags applied in paginated templates get applied to the pages? they aren't currently.

@zachleat
Copy link
Member

zachleat commented Dec 21, 2018

Absolutely @edwardhorsford, this feature will apply to all collections.

@zachleat zachleat self-assigned this Jan 8, 2019
@zachleat
Copy link
Member

zachleat commented Jan 8, 2019

Spent a little bit of time looking at this, just fair warning it’s gonna take a bit of work to get it going and isn’t going to make it in to 0.7.0. I am still very much on board with the idea.

zachleat added a commit that referenced this issue Jan 10, 2019
zachleat added a commit that referenced this issue Feb 11, 2019
zachleat added a commit that referenced this issue Feb 11, 2019
@zachleat zachleat added this to the 0.7.2 milestone Feb 11, 2019
@zachleat
Copy link
Member

Complete. This will be included with 0.7.2. Opt-in with:

---
pagination:
  addAllPagesToCollections: true
---

This will likely be the default in 1.0.

@zachleat
Copy link
Member

Major version default change filed at #407.

zachleat added a commit to 11ty/11ty-website that referenced this issue Feb 11, 2019
zachleat added a commit that referenced this issue Feb 11, 2019
@edwardhorsford
Copy link
Contributor Author

Complete. This will be included with 0.7.2. Opt-in with:

---
pagination:
  addAllPagesToCollections: true
---

This will likely be the default in 1.0.

So happy for this! Thanks a million!

zachleat added a commit that referenced this issue Feb 12, 2019
…xes `templateContent` missing in a few places.
zachleat added a commit that referenced this issue Feb 19, 2019
@zachleat
Copy link
Member

zachleat commented Feb 19, 2019

@edwardhorsford Would love some help testing this fix out, if you want to point your package.json to use the git repo master branch a la https://docs.npmjs.com/files/package.json#git-urls-as-dependencies

@edwardhorsford
Copy link
Contributor Author

@zachleat I'm still on 0.6 (started a new job recently which has occupied my time) - but will try to test out shortly.

Any particular areas to focus on? For tag pages, I know how many tags I have so should be able to easily see if the right number of pages exist, etc.

@edwardhorsford
Copy link
Contributor Author

edwardhorsford commented Feb 24, 2019

@zachleat Might take me a bit of time to test after all.

Turns out I was on 0.5.4 and versions since then don't compile correctly. Some of my site is rendered fine, but some is rendering out html. Weirdly I'm not getting any errors in the terminal.

screen shot 2019-02-24 at 22 38 41

The problem seems to start with v6.0 - so I'll have a look through what changed to try to work it out. If you have any ideas I'd be grateful.

EDIT: it looks like my main content block inserted in my templates is being escaped. I'm using {{ content | safe }} as per the @11ty docs so unsure why this is, or what changed. If I add autoescape: false to my nunjucks config it renders ok - but obviously am wary about disabling long term.

@edwardhorsford
Copy link
Contributor Author

@zachleat I didn't resolve the above issue, so have disabled autoescape on my site for now.

0.71 works for me, but 0.72 doesn't.

I get the following error:
Error: Could not resolve pagination key in template data: collections.byAspectRatio[0]

I'm indexing in to [0] because of #277. Now that that bug is fixed, I'll try removing. Still - I don't see why it's broken here. Perhaps an issue with arrays?

@edwardhorsford
Copy link
Contributor Author

@zachleat Just a note to say that after removing all the [0] related to #277, collections.all seems to be correctly show all my paginated pages.

I suspect there might be a bug though - I don't see why my previous data format of collections being inside an array would now stop working. I may try making a reduced test case to see.

Still having an issue with all output being escaped. I've disabled globally for now until I find a solution.

@zachleat
Copy link
Member

@edwardhorsford Are you using Nunjucks? I don’t see any autoescape changes in the release notes there hmm https://github.com/mozilla/nunjucks/releases

Seems to still be on by default (as it has always been?):
https://mozilla.github.io/nunjucks/api.html#autoescaping
https://mozilla.github.io/nunjucks/templating.html#autoescaping

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

No branches or pull requests

2 participants