Add OPDS/Atom Feed link in HTML head #882

juuz0 · 2023-02-05T12:02:43Z

Fixes #832

codecov · 2023-02-05T12:07:14Z

Codecov Report

Base: 71.95% // Head: 71.95% // No change to project coverage 👍

Coverage data is based on head (6b8f9aa) compared to base (fa80be8).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #882   +/-   ##
=======================================
  Coverage   71.95%   71.95%           
=======================================
  Files          54       54           
  Lines        3748     3748           
  Branches     2100     2100           
=======================================
  Hits         2697     2697           
  Misses       1049     1049           
  Partials        2        2

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

juuz0 · 2023-02-05T12:09:49Z

I think this is what was needed here (using Want My RSS extension on firefox - there was no inbuilt way to check which i know of). It links to the atom feed with correct filters. Working!

kelson42 · 2023-02-05T12:12:59Z

@juuz0 Thank you, I will give a try

kelson42 · 2023-02-05T12:19:27Z

@juuz0 What you have done looks good to me from a user perspective. That is what I was expecting.

juuz0 · 2023-02-05T13:55:51Z

@kelson42 Awesome! I don't why the CI is failing though, is it a common problem to current-libkiwix or just this PR?

(As a sidenote, how did you check the RSS feed? Is there an inbuilt feature in firefox/other browser?)

kelson42 · 2023-02-05T14:07:21Z

@kelson42 Awesome! I don't why the CI is failing though, is it a common problem to current-libkiwix or just this PR?

Yes, a problem with the package building. Nothing related to your PR.

(As a sidenote, how did you check the RSS feed? Is there an inbuilt feature in firefox/other browser?)

I checked the HTML and know how this feature should work. I have also a proper browser which displays an icon in the searchbar.

juuz0 · 2023-02-06T00:48:47Z

Added image @kelson42
It's hidden in mobile version because I don't know the correct design to put it (might need discussion here if it's desirable)

I also did a rebase so please remember that while pulling :>

static/templates/index.html

static/skin/index.js

static/templates/index.html

mgautierfr

Code seems good to me.
(Edit: But I have missed the wrong usage of the api, I agree with @veloman-yunkan here, better using /catalog/v2/entries)

However, I think the icon is a bit to lower, I would have put it at the top-right of the top bar.
But I will not block for that if there is a consensus against me.

kelson42 · 2023-02-06T10:07:18Z

However, I think the icon is a bit to lower, I would have put it at the top-right of the top bar.

Yes, top-right corner would be better IMO too.

juuz0 · 2023-02-06T23:48:01Z

However, I think the icon is a bit to lower, I would have put it at the top-right of the top bar.

Done.

Unrelated to this issue but https://github.com/kiwix/libkiwix/blob/main/static/skin/index.js#L22 is also using the old API, should that be changed too?

kelson42 · 2023-02-07T06:06:24Z

Unrelated to this issue but https://github.com/kiwix/libkiwix/blob/main/static/skin/index.js#L22 is also using the old API, should that be changed too?

yes please

static/templates/index.html

kelson42

It works fine but few changes necessary to be polished

static/resources_list.txt

static/skin/index.css

static/skin/index.js

static/templates/index.html

juuz0 · 2023-02-08T01:20:53Z

Rebased on main

veloman-yunkan · 2023-02-08T12:59:38Z

static/skin/index.js

-        document.querySelector('#feed_link').href = `${root}/catalog/v2/entries?${filteredParams.toString()}`;
+        const feedLink = `${root}/catalog/v2/entries?${filteredParams.toString()}`;
+        document.querySelector('#feed_link').href = feedLink;
+        document.querySelector('#feedLink').href = feedLink;
    }


I see in the current naming of the OPDS feed link ids a risk that any future links will be named feedlink, FeedLink, etc :)

I was worried about it too :P but kelson's recommendation so I let it pass.
What do you think is a better name?
@kelson42 @veloman-yunkan

We should keep only version of feedLink or feed_link to have the naming consistant.
But we should have a id for the link in the head as head_feed_link (or headFeedLink) and feed_link (or FeedLink).

headFeedLink then :)

mgautierfr

We are good on my side.

You could have set the head id to headFeedLink in the first commit to avoid the change in the second commit. But I will not block for that.

mgautierfr · 2023-02-09T11:33:12Z

@juuz0 Please rebase on master so we can merge.

This change adds a <link> element in the head node of welcome page. Browsers with extensions for RSS will show a sign to navigate to the feed. The link changes based on current set filters.

Added an image of rss logo on the welcome page which links to the RSS feed with current filters

juuz0 · 2023-02-09T15:21:49Z

@mgautierfr Done.

mgautierfr · 2023-02-09T15:43:46Z

@juuz0 Thanks.

Comment from @kelson42 has been addressed. So I think we are free to merge without its review.

juuz0 requested a review from kelson42 February 5, 2023 12:03

kelson42 requested a review from veloman-yunkan February 5, 2023 12:12

juuz0 force-pushed the rssFeed branch from 54ddecf to 6e5989b Compare February 6, 2023 00:45

veloman-yunkan requested changes Feb 6, 2023

View reviewed changes

static/templates/index.html Outdated Show resolved Hide resolved

static/skin/index.js Outdated Show resolved Hide resolved

static/templates/index.html Outdated Show resolved Hide resolved

mgautierfr requested changes Feb 6, 2023

View reviewed changes

juuz0 force-pushed the rssFeed branch from 6e5989b to 63df742 Compare February 6, 2023 23:42

juuz0 requested review from veloman-yunkan and mgautierfr February 6, 2023 23:45

veloman-yunkan requested changes Feb 7, 2023

View reviewed changes

static/templates/index.html Outdated Show resolved Hide resolved

static/templates/index.html Outdated Show resolved Hide resolved

kelson42 requested changes Feb 7, 2023

View reviewed changes

juuz0 force-pushed the rssFeed branch 2 times, most recently from 6012dd0 to 01a9f07 Compare February 8, 2023 01:09

juuz0 requested review from veloman-yunkan and kelson42 February 8, 2023 01:13

veloman-yunkan reviewed Feb 8, 2023

View reviewed changes

kelson42 changed the title ~~Add RSS Feed extension in head~~ Add OPDS/Atom Feed extension in HTML head Feb 8, 2023

kelson42 changed the title ~~Add OPDS/Atom Feed extension in HTML head~~ Add OPDS/Atom Feed link in HTML head Feb 8, 2023

juuz0 force-pushed the rssFeed branch from 01a9f07 to 91762ed Compare February 9, 2023 07:52

juuz0 requested a review from veloman-yunkan February 9, 2023 07:52

mgautierfr approved these changes Feb 9, 2023

View reviewed changes

veloman-yunkan approved these changes Feb 9, 2023

View reviewed changes

juuz0 added 2 commits February 9, 2023 20:47

Add RSS Feed extension in head

e3a211e

This change adds a <link> element in the head node of welcome page. Browsers with extensions for RSS will show a sign to navigate to the feed. The link changes based on current set filters.

Add specific link for Kiwix RSS Feed

6b8f9aa

Added an image of rss logo on the welcome page which links to the RSS feed with current filters

juuz0 force-pushed the rssFeed branch from 91762ed to 6b8f9aa Compare February 9, 2023 15:21

mgautierfr merged commit 29519df into main Feb 9, 2023

mgautierfr deleted the rssFeed branch February 9, 2023 15:43

kelson42 mentioned this pull request Feb 19, 2023

Internationalization of kiwix-serve's frontend #846

Merged

Popolechien mentioned this pull request May 30, 2023

Deploy latest version of Kiwix-server with OPDS feed kiwix/web#215

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OPDS/Atom Feed link in HTML head #882

Add OPDS/Atom Feed link in HTML head #882

juuz0 commented Feb 5, 2023

codecov bot commented Feb 5, 2023 •

edited

Loading

juuz0 commented Feb 5, 2023

kelson42 commented Feb 5, 2023

kelson42 commented Feb 5, 2023

juuz0 commented Feb 5, 2023

kelson42 commented Feb 5, 2023

juuz0 commented Feb 6, 2023 •

edited

Loading

mgautierfr left a comment •

edited

Loading

kelson42 commented Feb 6, 2023

juuz0 commented Feb 6, 2023 •

edited

Loading

kelson42 commented Feb 7, 2023

kelson42 left a comment

juuz0 commented Feb 8, 2023

veloman-yunkan Feb 8, 2023

juuz0 Feb 8, 2023

mgautierfr Feb 8, 2023

juuz0 Feb 9, 2023

mgautierfr left a comment

mgautierfr commented Feb 9, 2023

juuz0 commented Feb 9, 2023

mgautierfr commented Feb 9, 2023

Add OPDS/Atom Feed link in HTML head #882

Add OPDS/Atom Feed link in HTML head #882

Conversation

juuz0 commented Feb 5, 2023

codecov bot commented Feb 5, 2023 • edited Loading

Codecov Report

juuz0 commented Feb 5, 2023

kelson42 commented Feb 5, 2023

kelson42 commented Feb 5, 2023

juuz0 commented Feb 5, 2023

kelson42 commented Feb 5, 2023

juuz0 commented Feb 6, 2023 • edited Loading

mgautierfr left a comment • edited Loading

Choose a reason for hiding this comment

kelson42 commented Feb 6, 2023

juuz0 commented Feb 6, 2023 • edited Loading

kelson42 commented Feb 7, 2023

kelson42 left a comment

Choose a reason for hiding this comment

juuz0 commented Feb 8, 2023

veloman-yunkan Feb 8, 2023

Choose a reason for hiding this comment

juuz0 Feb 8, 2023

Choose a reason for hiding this comment

mgautierfr Feb 8, 2023

Choose a reason for hiding this comment

juuz0 Feb 9, 2023

Choose a reason for hiding this comment

mgautierfr left a comment

Choose a reason for hiding this comment

mgautierfr commented Feb 9, 2023

juuz0 commented Feb 9, 2023

mgautierfr commented Feb 9, 2023

codecov bot commented Feb 5, 2023 •

edited

Loading

juuz0 commented Feb 6, 2023 •

edited

Loading

mgautierfr left a comment •

edited

Loading

juuz0 commented Feb 6, 2023 •

edited

Loading