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

Paginate the list of bills #480

Merged
merged 1 commit into from
Feb 20, 2020
Merged

Conversation

zorun
Copy link
Collaborator

@zorun zorun commented Oct 1, 2019

This is a work-in-progress proposal to paginate the list of bills. The main motivation is that I have 1095 bills, and the list of bills is starting to be slow to display!

Before investing time in design etc, I would like to first gather some feedback on the idea itself.

By default, the first page is displayed (most recent bills). Older bills can be displayed with the page URL parameter:

$URL/project/?page=42

Documentation for this feature in flask-sqlalchemy is here:

https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.BaseQuery.paginate
https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.Pagination

Some things to improve:

  • decide on the number of bills to display on each page (20 items by default, which is probably fine)
  • decide what happens when fetching an out-of-range page. Currently it just displays an empty table, but we can also return a 404.
  • maybe add a link to the previous and next page above the table?
  • better style for the list of pages
  • implement the same thing in the API! (would be useful for MoneyBuster, it's also really slow to synchronise with 1000 bills)

Here is a screenshot, with only 2 items per page for testing:

screenshot demo

@almet
Copy link
Member

almet commented Oct 2, 2019

Thanks for starting working on this :-)

On the things to improve :

decide on the number of bills to display on each page (20 items by default, which is probably fine)

I would go for 100 bills displayed by default.

decide what happens when fetching an out-of-range page. Currently it just displays an empty table, but we can also return a 404.

404 seems to be the sane thing to do.

maybe add a link to the previous and next page above the table?

I wonder if we should try to use autoscroll here, in order to load the bills as they're needed?

better style for the list of pages

Yes! (But I'm bad at this :()

implement the same thing in the API! (would be useful for MoneyBuster, it's also really slow to synchronise with 1000 bills)

I wonder if paginating would make it faster? I believe it would just add some overhead here, don't you think? But, how to enhance the API is probably an issue by itself. We could probably enhance how we synchronize, but that's a bunch of changes.

I've been working (with @Natim, who's a contributor here) on the Kinto project, which is basically an JSON HTTP API, made to be efficient. We might want to borrow some principles we've implemented there :

  • Pagination
  • How Timestamps are used to ease the syncing (and avoid downloading again stuff that's already been retrieved).

@almet
Copy link
Member

almet commented Oct 14, 2019

Hey @zorun, hope everything's going well. Are you okay with this, or do you need any help?

@almet
Copy link
Member

almet commented Feb 13, 2020

Hey. This seems stalled. I believe that until we have some news from @zorun we should close the pull request (it's been more than 3 months)

@zorun
Copy link
Collaborator Author

zorun commented Feb 17, 2020

@almet yeah sorry, didn't have time to work on this.

I just rebased and pushed, with a few changes:

  • 100 bills per page by default
  • 404 page when requesting an out-of-range page
  • add a "Page:" string in front of the page numbers

This should be ready for merging, unless we want to have a configuration option to choose the number of bills per page.

@zorun zorun changed the title WIP: paginate the list of bills Paginate the list of bills Feb 17, 2020
@almet
Copy link
Member

almet commented Feb 17, 2020

Neat! You can run black on web.py to fix the travisCI error.

I believe that if there is no pagination, the section is still shown, can we change this?

We display 100 bills on each page.  We only show previous/next buttons (at
the top of the view) and the list of pages (at the bottom) if there are
more than one pages.

This uses built-in pagination support from Flask-SQLAlchemy:

  https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.BaseQuery.paginate
  https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.Pagination
@zorun
Copy link
Collaborator Author

zorun commented Feb 17, 2020

Right, it should be better now.

I have also improved the rendering using Bootstrap classes:

ihm-paginate

For the screenshot I displayed just 1 item per page, this is of course not the case for the final commit (100 items per page).

@almet does it sound good? I am not a designer but bootstrap makes a lot of things easier :)

@almet
Copy link
Member

almet commented Feb 20, 2020

yeah, it's great, thanks!

@almet almet merged commit 9378694 into spiral-project:master Feb 20, 2020
Jojo144 pushed a commit to Jojo144/ihatemoney that referenced this pull request Mar 21, 2020
We display 100 bills on each page.  We only show previous/next buttons (at
the top of the view) and the list of pages (at the bottom) if there are
more than one pages.

This uses built-in pagination support from Flask-SQLAlchemy:

  https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.BaseQuery.paginate
  https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.Pagination
@zorun zorun deleted the paginate_bills branch April 26, 2020 22:12
@zorun zorun mentioned this pull request Oct 19, 2021
TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this pull request Mar 2, 2024
We display 100 bills on each page.  We only show previous/next buttons (at
the top of the view) and the list of pages (at the bottom) if there are
more than one pages.

This uses built-in pagination support from Flask-SQLAlchemy:

  https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.BaseQuery.paginate
  https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.Pagination
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.

2 participants