Skip to content

Pagination #82

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.

Sign up for GitHub

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

Jump to bottom
Merged
marktheunissen merged 5 commits into from
Jun 30, 2013
Merged

Pagination #82

marktheunissen merged 5 commits into from
Jun 30, 2013

Conversation

@pifantastic
Copy link
Member

@pifantastic pifantastic commented Jun 29, 2013

Closes #35

@pifantastic
Backend support for pagination. Front-end support TODO.
128b6eb
@pifantastic
Support past and future pagination.
cc339ea
@pifantastic
Merge branch 'master' into issue-35
8151fb5 
* master:
  Update Changelog.
  Protect against empty posts. I think we need to check for whitespace in postBoxDisabled. But oh well.
  Allow submitting a post with ctrl + enter.
  Config change to 5 sec
  Remove old config stuff
  Port Mark's config changes over. Remove now unused hbs module.
  Use require for template.
  Use require.js
  Adding more formats
  Fix config handling, provide poll interval
  Linkify all urls in a post.
  Use 'listen' instead of 'host'.
  Listen to localhost only by default, but make it configurable.

Conflicts:
	config.sample.json
	src/lib/config/config.go
	static/index.html
	static/js/app/core.js
@pifantastic
Functional 'load more' button.
cd47de0
@pifantastic
Comments.
b11b4c0
@Fil
Copy link

Fil commented Jun 30, 2013

Is it useful to say I've tested it and it works?

@marktheunissen
Copy link
Member

marktheunissen commented Jun 30, 2013

@Fil Yes, thanks for that.

@pifantastic This works great and is an effective stop-gap. Nice work. I had some thoughts about the limitations of this approach and I'll put them here in case others are interested.

If more than PageSize posts are made between consecutive polling requests (3 seconds by default), they won't be fetched. We could change the poll to use 'after' and pass a highwater mark, but there is no good method of doing this given our environment. For example, imagine this simplified representation:

G - 190
F - 189
E - 188
D - 187
C - 186
B - 185
A - 184

We have posts with IDs of A..G, and timestamps 184..190. The list is sorted by timestamp. If we ask for all posts after ID G, but 10 new posts arrive with timestamp 189, we'll never get them on the frontend. If we use timestamp, we have the same problem, asking for all posts after 190 will fail to retrieve anything that arrives in the past. Doing a hard refresh of the page will collect all missing data, of course.

However, I think that this is fine for now. We can set the PageSize to 50. If someone is getting more than 50 posts every 3 seconds, they're being spammed.

With inotify support, this problem goes away. I've opened a new issue for that:

#84

marktheunissen added a commit that referenced this pull request Jun 30, 2013
@marktheunissen
Merge pull request #82 from vole/issue-35
c5d8bc1 
Pagination
@marktheunissen marktheunissen merged commit c5d8bc1 into master Jun 30, 2013
@marktheunissen marktheunissen deleted the issue-35 branch June 30, 2013 16:20
@Fil
Copy link

Fil commented Jul 1, 2013

Pagination is broken on the personal profile. Possibly because filtering on user is done after pagination.

@pifantastic
Copy link
Member Author

pifantastic commented Jul 1, 2013

D'oh. I never thought to check there @Fil. Good point. Does it just return everyone's posts?

Should be an easy fix.

@marktheunissen
Copy link
Member

marktheunissen commented Jul 1, 2013

@Fil, I did test it and it worked for me. What are the exact steps to reproduce?

@marktheunissen
Copy link
Member

marktheunissen commented Jul 1, 2013

Ah ok, so if you go to your profile but the frontend hasn't loaded any of your own posts, you won't see anything.

Every time you hit 'load more', it's loading the next 50 posts, which may or may not include your own posts - so if you have 4 posts in the next 50, then those 4 appear.

Correct behavior is to query for 50 of only your own posts.

@Fil
Copy link

Fil commented Jul 1, 2013

I tried again with a fresh clone from https://github.com/vole/vole.git

"My profile" lists 4 posts; "load more" loads another 6; then another 11;
then 0…

@Fil
Copy link

Fil commented Jul 1, 2013

exactly

@pifantastic
Copy link
Member Author

pifantastic commented Jul 1, 2013

Thanks @Fil for finding stuff like this so quickly 👍

@Fil
Copy link

Fil commented Jul 1, 2013

There are only two buttons to try :)

@pifantastic
Copy link
Member Author

pifantastic commented Jul 1, 2013

We will send you a complimentary live vole when we hit version 1.0

@marktheunissen
Copy link
Member

marktheunissen commented Jul 6, 2013

I'm working on this now.

marktheunissen added a commit that referenced this pull request Jul 8, 2013
@marktheunissen
Merge pull request #95 from vole/82-follow-up
fc2b447 
#82 Follow up
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Pagination support.

4 participants

@pifantastic @Fil @marktheunissen