Kiwix Serve welcome page dynamic and subset loading (OPDS based) #530
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,3 +4,4 @@ subprojects/googletest-release* | |
| *.class | ||
| build/ | ||
| .vscode/ | ||
| builddir/ | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| (function() { | ||
| const root = $(`link[type='root']`).attr('href'); | ||
| let isFetching = false; | ||
| const incrementalLoadingParams = { | ||
| start: 0, | ||
| count: viewPortToCount() | ||
| }; | ||
|
|
||
| function queryUrlBuilder() { | ||
| let url = `${root}/catalog/search?`; | ||
| url += Object.keys(incrementalLoadingParams).map(key => `${key}=${incrementalLoadingParams[key]}`).join("&"); | ||
| return url; | ||
| } | ||
|
|
||
| function viewPortToCount(){ | ||
| return Math.floor(window.innerHeight/100 + 1)*(window.innerWidth>1000 ? 3 : 2); | ||
| } | ||
|
|
||
| function getInnerHtml(node, query) { | ||
| return node.querySelector(query).innerHTML; | ||
| } | ||
|
|
||
| function generateBookHtml(book) { | ||
| const link = book.querySelector('link').getAttribute('href'); | ||
| const title = getInnerHtml(book, 'title'); | ||
| const description = getInnerHtml(book, 'summary'); | ||
| const id = getInnerHtml(book, 'id'); | ||
| const iconUrl = getInnerHtml(book, 'icon'); | ||
| const articleCount = getInnerHtml(book, 'articleCount'); | ||
| const mediaCount = getInnerHtml(book, 'mediaCount'); | ||
|
|
||
| return `<a href='${link}' data-id='${id}'><div class='book'> | ||
| <div class='book__background' style="background-image: url('${iconUrl}');"> | ||
| <div class='book__title' title='${title}'>${title}</div> | ||
| <div class='book__description' title='${description}'>${description}</div> | ||
| <div class='book__info'>${articleCount} articles, ${mediaCount} medias</div> | ||
| </div> | ||
| </div></a>`; | ||
| } | ||
|
|
||
| async function loadAndDisplayBooks() { | ||
| isFetching = true; | ||
| fetch(queryUrlBuilder()).then(async (resp) => { | ||
| const data = new window.DOMParser().parseFromString(await resp.text(), 'application/xml'); | ||
| const books = data.querySelectorAll('entry'); | ||
| let bookHtml = ''; | ||
| books.forEach((book) => {bookHtml += generateBookHtml(book)}); | ||
| document.querySelector('.book__list').innerHTML += bookHtml; | ||
| incrementalLoadingParams.start += books.length; | ||
| if (books.length < incrementalLoadingParams.count) { | ||
| incrementalLoadingParams.count = 0; | ||
| } | ||
| isFetching = false; | ||
| }); | ||
| } | ||
|
|
||
| async function loadSubset() { | ||
| if (!isFetching && | ||
| incrementalLoadingParams.count && | ||
| window.innerHeight + window.scrollY >= document.body.offsetHeight | ||
| ) { | ||
| loadAndDisplayBooks(); | ||
| } | ||
MananJethwani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| window.addEventListener('resize', async () => { | ||
| incrementalLoadingParams.count = incrementalLoadingParams.count && viewPortToCount(); | ||
| loadSubset(); | ||
| }); | ||
|
|
||
| window.addEventListener('scroll', loadSubset); | ||
|
|
||
| window.onload = async () => { | ||
| loadAndDisplayBooks(); | ||
| } | ||
| })(); | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,66 +1,96 @@ | ||
| <!DOCTYPE html> | ||
| <html xmlns="http://www.w3.org/1999/xhtml"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <title>Welcome to Kiwix Server</title> | ||
| <script | ||
| type="text/javascript" | ||
| src="{{root}}/skin/jquery-ui/external/jquery/jquery.js" | ||
| ></script> | ||
| <script | ||
| type="text/javascript" | ||
| src="{{root}}/skin/jquery-ui/jquery-ui.min.js" | ||
| ></script> | ||
| <link | ||
| type="text/css" | ||
| href="{{root}}/skin/jquery-ui/jquery-ui.min.css" | ||
| rel="Stylesheet" | ||
| /> | ||
| <link | ||
| type="text/css" | ||
| href="{{root}}/skin/jquery-ui/jquery-ui.theme.min.css" | ||
| rel="Stylesheet" | ||
| /> | ||
| <style> | ||
| body { | ||
| background: radial-gradient(#eeeeee 15%, transparent 16%) 0 0, | ||
| radial-gradient(#eeeeee 15%, transparent 16%) 8px 8px, | ||
| radial-gradient(rgba(255, 255, 255, 0.1) 15%, transparent 20%) 0 1px, | ||
| radial-gradient(rgba(255, 255, 255, 0.1) 15%, transparent 20%) 8px 9px; | ||
| background-color: #e8e8e8; | ||
| background-size: 16px 16px; | ||
| margin-left: auto; | ||
| margin-right: auto; | ||
| max-width: 1100px; | ||
| } | ||
| .book__list { | ||
| text-align: center; | ||
| } | ||
| .book { | ||
| display: inline-block; | ||
| vertical-align: bottom; | ||
| margin: 8px; | ||
| padding: 12px 15px; | ||
| width: 300px; | ||
| border: 1px solid #ccc; | ||
| border-radius: 8px; | ||
| text-align: left; | ||
| color: #000; | ||
| font-family: sans-serif; | ||
| font-size: 13px; | ||
| background-color: #f1f1f1; | ||
| box-shadow: 2px 2px 5px 0px #ccc; | ||
| } | ||
| .book:hover { | ||
| background-color: #f9f9f9; | ||
| box-shadow: none; | ||
| } | ||
| .book__background { | ||
| background-repeat: no-repeat; | ||
| background-size: 48px 48px; | ||
| background-position: top right; | ||
| } | ||
| .book__title { | ||
| padding: 0 55px 0 0; | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| font-size: 18px; | ||
| color: #0645ad; | ||
| line-height: 1em; | ||
| } | ||
| .book__description { | ||
| padding: 5px 55px 5px 0px; | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| font-size: 15px; | ||
| line-height: 1em; | ||
| } | ||
| .book__info { | ||
| color: #777; | ||
| font-weight: bold; | ||
| font-size: 13px; | ||
| line-height: 1em; | ||
| } | ||
| </style> | ||
| <script type="text/javascript" src="{{root}}/skin/index.js" async></script> | ||
| </head> | ||
| <body class="kiwix"> | ||
| <div class="kiwix"> | ||
| <div class="book__list"></div> | ||
| </div> | ||
|
|
||
|
Comment on lines
-49
to
-57
There was a problem hiding this comment. If we don't use the There was a problem hiding this comment. I agree on this point I was going to open a separate issue for this, but if you want it removed from cpp side I will create a new PR and add that part in some time. There was a problem hiding this comment. Why the next time ? I don't "want" to remove the cpp code. I want to have the template and the cpp in sync. There was a problem hiding this comment. by remove, I meant that only😅.... I will bring them in sync. |
||
| <div id="kiwixfooter">Powered by <a href="https://kiwix.org">Kiwix</a></div> | ||
| </body> | ||
| </html> | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing this ?
OPDS stream list zim file a user can download. If we remove the
remote(true)filter, we will return zim file without url to download from.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgautierfr kelson discussed this point earlier in the PR and we agreed that we should show all zim files and show a disabled download button(and a tooltip) for those zim files that can't be downloaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that ODPS stream is already used (and designed for) native client which list what zim files user can download.
By definition, the user can download a zim file if they have a associated url (they are "remote").
If you remove this filter, you break the API for existing client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgautierfr Can you please be specific? I'm not aware of anything broken. If OPDS should not list anymore books with URL, it should list all books. This is necessary for the welcome page and this is more logical because the URL is only one attribute beetween others of the book. If at some point we need to filter on URL them we should add a new filter, but AFAIK this is not needed to the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kiwix-desktop uses OPDS stream to propose zim file to download to the user. If OPDS stream now return one zim file without "url" attribute, kiwix-desktop will display to book to the user but he will not be able to download it.
(I've checked and kiwix-desktop reapply this filter internally before displaying books to the user, but other implementation (kiwix-android, kiwix-ios)) may not do the same).
(And this is maybe hypotical anyway as most (if not all) zim files in library.kiwix.org have a url, but if someone use a local opds stream, it will be broken)
This is a OPDS API break, we cannot change the content returned by a stream like that.
We may want to change this behavior or not (we can extend the catalog/v2 to handle this feature and use catalog/v2), but if we change it we have to be careful about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgautierfr @kelson42 instead of creating a completely new endpoint wouldn't it be better if I just introduce a new search parameter that will check if we need all zim files and changes filter accordingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgautierfr Kiwix-Desktop works perfectly, so there is no problem, there is no API break, there is no documentation to update. What you are talking about seems to be a potential problem if for some reason library.kiwix.org would deliver an OPDS stream with books without URLs. An "yes" I confirm, this is the resonsability of the person running the kiwix-serve to have a
library.xmlwith URL if he wants that these books are downloadable. What is concretly wrong here or broken?