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

Add ability to the user to filter lists by date added #5334

Merged

Conversation

Evazisouli
Copy link
Contributor

Closes #4267

Adding a new feature for sorting the Reading Log "Want to read" list of users by the date they have added their books.

Technical

Two filters added when being in the "Want to read" list, one for descending order and one for ascending.

Testing

  1. Visit the Reading Log "Want to read" list
  2. Two filters should appear, "Newest to Oldest" and "Oldest to Newest".
  3. By clicking "Newest to Oldest" the books on the right should be sorted by the date the user has logged them in his list from newest logged to oldest logged.
  4. By clicking "Oldest to Newest" the books on the right should be sorted by the date the user has logged them in his list from oldest logged to newest logged.

Screenshot

image

image

image

Stakeholders

@mekarpeles, @jamesachamp

@cdrini cdrini self-assigned this Jun 21, 2021
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! This is going to be super useful :) A few code quality/UX feedback for you! Also, let's make the default going forward "Date added (newest)" instead of whatever random order the DB has these in :P

openlibrary/core/bookshelves.py Outdated Show resolved Hide resolved
openlibrary/templates/account/books.html Outdated Show resolved Hide resolved
openlibrary/plugins/upstream/account.py Outdated Show resolved Hide resolved
openlibrary/plugins/upstream/account.py Outdated Show resolved Hide resolved
openlibrary/plugins/upstream/account.py Outdated Show resolved Hide resolved
openlibrary/templates/account/books.html Outdated Show resolved Hide resolved
@cdrini cdrini added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Jun 28, 2021
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fixes! Looking good :) One small UX fix, and then it looks like there are also some code formatting issues: https://github.com/internetarchive/openlibrary/pull/5334/checks?check_run_id=2928114170#step:8:16 . These are mostly menial things, like adding new line characters. Once that's done, we should be good to merge!

@@ -65,6 +67,17 @@ <h1 style="display:inline">

$if owners_page:
<a href="$ctx.path/stats" title="$('View stats about this shelf')">$_('Stats')</a>

<span class="mytools-tools"><img src="/images/icons/icon_sort.png" alt="$_('Sorting by')" style="margin-right:10px;" width="9" height="11">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, my typo; my bad :P

Suggested change
<span class="mytools-tools"><img src="/images/icons/icon_sort.png" alt="$_('Sorting by')" style="margin-right:10px;" width="9" height="11">
<span class="mybooks-tools"><img src="/images/icons/icon_sort.png" alt="$_('Sorting by')" style="margin-right:10px;" width="9" height="11">

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah we'll also want to move this a little lower; just before the macros.Pages section:

image

@cdrini
Copy link
Collaborator

cdrini commented Jun 28, 2021

Oh, this is also up on testing.openlibrary.org now, if you want to take it for a spin 👍

Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! Tested on testing:

  • Bar appears on all three types of shelves
  • shelves now appear with most recent at top
  • Switch sort order works
  • Paging works when sort enabled

@cdrini cdrini merged commit 77d98b3 into internetarchive:master Jun 28, 2021
@cdrini
Copy link
Collaborator

cdrini commented Jun 28, 2021

Context: I don't usually merge stuff on the weekends :P , but @Evazisouli and @STAMATIOSL have a course deadline on Monday that they're trying to meet. And they've been so friendly and polite and diligently followed all our community rules, and been patient and understanding while getting their environments up and running, and now created this wonderfully useful feature! So wanted to make sure they met their deadline :) Best of luck, @Evazisouli, @STAMATIOSL!

@jimchamp jimchamp removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Apr 2, 2024
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.

Add ability for a user to filter lists by date added
4 participants