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

Feature: If RSS is enabled, show RSS icon in menu #63

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

vvzen
Copy link

@vvzen vvzen commented Jun 1, 2024

This PR adds support for showing a tiny RSS svg badge in the top menu in case the user added a menu_item that looks like this:

menu_items = [
    {name = "home", url = "$BASE_URL"},

    # [...]

    # RSS
    {name = "", url = "$BASE_URL/$FEED_FILENAME"},
]

It also requires that the user has a config.toml that contains the following entries, and generate_feed is set to true :

generate_feed = true
feed_filename = "rss.xml"
author = "Your name here"

The icon would look like this:
Screenshot 2024-06-01 at 11 51 14 AM

A live example can be seen here: https://valerioviperino.me

@vvzen vvzen force-pushed the feature/show-rss-icon-in-menu branch from 28619fa to 5f7ce19 Compare June 1, 2024 09:56
@pawroman
Copy link
Owner

pawroman commented Jun 5, 2024

Hey, thanks for the feature contribution.

Personally, I'd prefer it to be one of the feather icons, since it's just fewer bytes, and also MIT licensed :)

<svg viewBox="0 0 24 24" width="24" height="24" stroke="currentColor" stroke-width="2" fill="none" stroke-linecap="round" stroke-linejoin="round" class="css-i6dzq1"><path d="M4 11a9 9 0 0 1 9 9"></path><path d="M4 4a16 16 0 0 1 16 16"></path><circle cx="5" cy="19" r="1"></circle></svg>

It would also be great if there was a configuration toggle for:

  • Enabling the icon (should be disabled by default, not to break backwards compat)
  • The icon's color

What do you think? Would you be interested in adding these?

@pawroman pawroman added the enhancement New feature or request label Jun 5, 2024
@vvzen
Copy link
Author

vvzen commented Jun 5, 2024

Oh, I didn't know about that OSS icon, that's much better!

As for the other things, sounds good! I'll have a look as soon as I have a bit of free time.

@vvzen
Copy link
Author

vvzen commented Jun 6, 2024

If I use something like

<svg xmlns="http://www.w3.org/2000/svg" width="22" height="22" viewBox="0 0 32 32" fill="none" stroke="#ee802f" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-rss">
<g transform="translate(0,5)">
    <path d="M4 11a9 9 0 0 1 9 9"></path>
    <path d="M4 4a16 16 0 0 1 16 16"></path>
    <circle cx="5" cy="19" r="1"></circle>
</g>
</svg>

it looks like this:
Screenshot 2024-06-06 at 10 39 11 PM

is that okay?

Otherwise I can keep it white and slightly bigger, which is what your original snippet does!

@vvzen
Copy link
Author

vvzen commented Jun 6, 2024

As for enabling the icon, right now one would need to add this to the menu_items :

menu_items = [
    # RSS
    {name = "", url = "$BASE_URL/$FEED_FILENAME"}
]

so it's opt-in already, in a way. Unless you prefer a dedicated config entry!
As for the color, it should be pretty easy to make it configurable and just inject the rss_icon_color as a variable into the svg itself.

@pawroman
Copy link
Owner

pawroman commented Jun 7, 2024

is that okay?

yeah, nice!

3 things I would add on top of it (in order of priority):

  1. The svg element should have a CSS class or id so that it's easy to customize the size, and other things.
  2. It would also be great to document how to add the RSS feed icon in the README. E.g. it's not obvious that menu items with url = "$BASE_URL/$FEED_FILENAME" are going to be converted into a neat RSS icon in the menu.
  3. It would be great to add rss_icon_color as a configurable, so it's easy to change without involving custom CSS.

@vvzen
Copy link
Author

vvzen commented Jun 8, 2024

I've done a first pass on those features!
1 - > 6dfa410, I've used id="rss-icon"
2 -> c84d03c
3 -> 6181259, added it to the extra section of the config.toml

For 3) I think it might make more sense to have rss_icon_color to use the same logic that you're using for accent_color and background_color, but I need to take a deeper look at how you've implemented those! It would make it easier for people to just have the RSS icon match the current palette of the theme.

Let me know what's next!

@vvzen vvzen force-pushed the feature/show-rss-icon-in-menu branch from f1b4c1d to c84d03c Compare June 9, 2024 09:26
Copy link
Owner

@pawroman pawroman left a comment

Choose a reason for hiding this comment

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

Hey, thanks for addressing the comments, and sorry for the late review. Unfortunately, to merge this we need to address breaking changes to feed generation added in recent PRs. Please take a look.

{% endif -%}

<a type="application/{{feed_type}}" title="RSS" href="{{ get_url(path=config.feed_filename) | safe }}">
<svg id="rss-icon" xmlns="http://www.w3.org/2000/svg" width="22" height="22" viewBox="0 0 32 32" fill="none" stroke="{{rss_icon_color}}" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-rss">
Copy link
Owner

Choose a reason for hiding this comment

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

We should give credit to feather icons https://feathericons.com/ and include their LICENSE file in the repo https://github.com/feathericons/feather/blob/main/LICENSE as LICENSE-Feather.md (similar to Hack fonts).

Copy link
Author

Choose a reason for hiding this comment

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

Added in 5017bf6

README.md Outdated
Comment on lines 356 to 360
generate_feed = true
author = "[email protected]"

# Use `rss.xml` for RSS feeds and `atom.xml` for ATOM.
feed_filename = "atom.xml"
Copy link
Owner

Choose a reason for hiding this comment

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

This needs to be updated to reflect changes introduced in #71 and #73

<a href="{{ item.url | replace(from="$BASE_URL", to=config.base_url) | safe }}" target="_blank" rel="noopener noreferrer">{{ item.name | safe }}</a>

<!-- RSS -->
{%- set is_rss = item.url == "$BASE_URL/$FEED_FILENAME" -%}
Copy link
Owner

Choose a reason for hiding this comment

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

We use

{%- if feed is containing('rss') %}

elsewhere now (see #71), would be good to keep it consistent.

Copy link
Author

@vvzen vvzen Aug 18, 2024

Choose a reason for hiding this comment

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

To do that, I'd need to do it inside the {%- for feed in config.feed_filenames %} loop, but this specific if is guarding against a different thing: whether or not the current item refers to the RSS icon or not (we don't care about RSS or ATOM at this stage). If it is referring to RSS, then we can add the svg icon and also add the relevant <a> element depending if it's ATOM or RSS. Does that make sense ?

Comment on lines 52 to 57
{%- if config.generate_feed %}
{%- if config.feed_filename == "rss.xml" %}
{% set feed_type = 'rss+xml' %}
{%- else %}
{% set feed_type = 'atom+xml' %}
{% endif -%}
Copy link
Owner

Choose a reason for hiding this comment

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

Needs to be updated, see other comments for details.

@vvzen
Copy link
Author

vvzen commented Aug 18, 2024

Hey @pawroman , no problem. I noticed these new changes in Zola 19.1 as well (getzola/zola#2566) - I'll give it ago once I have a bit of free time.

vvzen added 9 commits August 18, 2024 14:36
This adds support for showing a tiny RSS svg badge in the top menu in
case the user added a menu_item that looks like this:
`{name = "", url = "$BASE_URL/$FEED_FILENAME"}`

This also requires that the user has a config.toml that contains the
following entries:

generate_feed = true
feed_filename = "rss.xml"
author = "your name here"
There's some css rules for which prepending the name of the menu item as
set in the config causes the <a> element to appear shifted down compared
to the other <a> elements of the menu. I might have a look later, but
for now this enables only the appeareance of the RSS icon, discarding
entirely the name of the link.
At some point this might follow the same logic used by `accent_color`
and `background_color`, for consistency.
NOTE: This breaks compat with previous previous versions since Zola made a
breaking change in 0.19 and imho it's not worth trying to support previous
versions too.
@vvzen vvzen force-pushed the feature/show-rss-icon-in-menu branch from 13fca2f to 8fc4001 Compare August 18, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants