openzim / gutenberg

Scraper for downloading the entire ebooks repository of project Gutenberg
https://download.kiwix.org/zim/gutenberg
GNU General Public License v3.0
130 stars 37 forks source link

Added more search capability, bookshelves, and UI improvements #102

Closed vlee1776 closed 4 years ago

vlee1776 commented 4 years ago

1) Added search by title 2) Add bookshelf page where you can search by bookshelf 3) Individual bookshelf pages 4) Downloaded covers 5) Included covers on the homepage as thumbnails

kelson42 commented 4 years ago

@vlee1776 Thank you for your PR. Could you please fix the problems reported by code factor? We might also ask you to split your PR, like I have written weeks ago, big PRs are really difficult to review/merge.

vlee1776 commented 4 years ago

Thank you for the feedback @kelson42. We'll fix the problems from code factor ASAP. As for the splitting of the PR, please let us know how it would best to split it up if it is neccessary.

rgaudin commented 4 years ago

Let me take a look first and I'll let you know if it needs to be broken down.

vlee1776 commented 4 years ago

You could also see our changes on the zim here http://54.161.135.106/kiwix/gutenberg_en_selection_2020-02/A/Home.html

eshellman commented 4 years ago

Missing covers are not handled properly. If a book doesn't have a cover, the detail page is OK but in the book list, it appears as a broken image. Search for Jerry T. Bonnel for instance.

Note that there is now a cover for every book in Project Gutenberg. These were added starting in January and should have been 100% by the end of February.

rgaudin commented 4 years ago

Missing covers are not handled properly. If a book doesn't have a cover, the detail page is OK but in the book list, it appears as a broken image. Search for Jerry T. Bonnel for instance.

Note that there is now a cover for every book in Project Gutenberg. These were added starting in January and should have been 100% by the end of February.

OK good to know, so maybe it was just a glitch in your ZIM. 👍

Hariharan-V commented 4 years ago

Thank you for the feed back @rgaudin . Regarding

  • title search is global. When you select a bookshelf, it apparently just changes the list of book you're browsing. The title search returns results from outside the bookshelf. That's unexpected.
  • author search is global. Same as for title.

would it be better to have no filters on top of bookshelf or should we just make it so that it filters on the displayed items?

vlee1776 commented 4 years ago

Thanks again @rgaudin for the feedback.

  • Search by title feature should be enabled only using a parameter. You understand this can't scale so one would cherry-pick it.

In order to do this we can add a flag to gutenberg2zim with the option for title search.

  • Nb of records to show doesn't work on bookshelf UI. It has no effect.

Would you mind double checking it? I believe this is working when we tried on our end.

  • Missing covers are not handled properly. If a book doesn't have a cover, the detail page is OK but in the book list, it appears as a broken image. Search for Jerry T. Bonnel for instance.

We can add an alternate no cover image, just in case for the book list.

  • Bookshelves feature should be enabled using a parameter. This should be optional.

Same as search by title.

  • Bookshelves UI is odd. It doesn't look like the rest of the UI, feels unfinished. I think you should reuse what exists for books. My first thought was actually to get rid of that page and use a select box in the main page but looking at the database, I see that there are at least 300 bookshelves, so that would not be of much use.

We can change the color scheme and padding. Would that be enough to have a similar feel as the rest of the UI? We tried to accomodate with 300 bookshelves with the pagination.

  • how does that integrates with multiple languages?

We started looking into this. Could we get some advice/help into how this works and how we can integrate with it?

  • what are the changes in js/tools.js ? SInce you (your text editor) changed the quoting on the file, it's impossible to know if you've made any change to this file. Just submit your changes, if any.

We can move our changes into the original file.

rgaudin commented 4 years ago
  • Search by title feature should be enabled only using a parameter. You understand this can't scale so one would cherry-pick it.

In order to do this we can add a flag to gutenberg2zim with the option for title search.

Yes.

  • Nb of records to show doesn't work on bookshelf UI. It has no effect.

Would you mind double checking it? I believe this is working when we tried on our end.

Seems to be working fine, might have been mislead by the few number of bookshelves in your example zim.

  • Missing covers are not handled properly. If a book doesn't have a cover, the detail page is OK but in the book list, it appears as a broken image. Search for Jerry T. Bonnel for instance.

We can add an alternate no cover image, just in case for the book list.

Would be safer, yes.

  • Bookshelves feature should be enabled using a parameter. This should be optional.

Same as search by title.

Yes

  • Bookshelves UI is odd. It doesn't look like the rest of the UI, feels unfinished. I think you should reuse what exists for books. My first thought was actually to get rid of that page and use a select box in the main page but looking at the database, I see that there are at least 300 bookshelves, so that would not be of much use.

We can change the color scheme and padding. Would that be enough to have a similar feel as the rest of the UI? We tried to accomodate with 300 bookshelves with the pagination.

Yes, it's just that the rest is centered and somewhat unified while this one is not. I'm concerned about a simple list with a large number of bookshelves though. I'm afraid it might defeat the purpose of browsing. Could be a later addition but you might want to look into different designs, like a mosaic maybe?

  • how does that integrates with multiple languages?

We started looking into this. Could we get some advice/help into how this works and how we can integrate with it?

There are a number of pre-generated JS files that contains list of articles based on sorting and combinations. Take a look at tools.js to see what are the existing combinations.

  • what are the changes in js/tools.js ? SInce you (your text editor) changed the quoting on the file, it's impossible to know if you've made any change to this file. Just submit your changes, if any.

We can move our changes into the original file.

Please do.

rgaudin commented 4 years ago

@vlee1776 I see some new commits ; let me know when you want me to take another look at it.

vlee1776 commented 4 years ago

Will do, still working on some changes.

aschlumpf commented 4 years ago

@rgaudin Quick question about tools.js -- do you guys use a code formatter? I believe these changes happened because I have Prettier configured for my editor, which is a popular code formatter for JavaScript. If you have formatting rules, please let us know so we can apply them to our code and more easily find our changes. If not, we will have to manually dig to rediscover the exact changes we made.

Edit: I wrote comments on the code we changed/added. I'm not sure if this is a sufficient way for doing the PR, so let me know if you'd rather take a different approach.

rgaudin commented 4 years ago

Thank you @aschlumpf for the effort. Would you mind taking the extra step of reseting the tools.js file and reapplying your changes so that we can check and keep an history of what happened?

AFAIK, there's no code formatting for this file but once we get to merge this, we could have a new, separate commit to set the formatting to something standard that would ease new contributions.

vlee1776 commented 4 years ago

Here is an updated link w/the requested ui changes and handling of languages for bookshelves. http://pgzim.ebookfoundation.org/kiwix/gutenberg_de-fr_all_2020-04/A/Home.html Please view in an incognito or private browser

rgaudin commented 4 years ago

Thanks for all the changes and fixes. Now that we've merged your formatting PR, can you please resolve the tools.js conflict so we can get this merged-in?

aschlumpf commented 4 years ago

Conflicts resolved.