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

Improve Bevy news feed #1073

Merged
merged 11 commits into from Mar 6, 2024

Conversation

TrialDragon
Copy link
Member

@TrialDragon TrialDragon commented Mar 3, 2024

Adds a standard feed link to the footer of the page and adds the authors variable to the news pages so that the news feed will have attribution to the authors of articles. It is currently using the rss icon provided by feather icons: https://github.com/feathericons/feather/blob/main/icons/rss.svg

image
image

Closes #1072

A note: we can't use the authors variable in place of our current extra.author variable because it isn't available to templates. It's only for feeds as far as I know.

@TrialDragon TrialDragon changed the title Improve Bevy news RSS feed Improve Bevy news feed Mar 3, 2024
Technically we're using atom rather than RSS, minor technical difference.
@mockersf
Copy link
Member

mockersf commented Mar 4, 2024

I don't think RSS is used enough to get a place in the header

From #1072 (comment), this shouldn't be needed. We could add a title in the html header link.

@BD103
Copy link
Member

BD103 commented Mar 4, 2024

I'm not a fan of the bright orange RSS logo; it clashes a lot with the current website design. What if we used a black + white variant that could use Bevy's theme?

I don't think RSS is used enough to get a place in the header

What if the RSS logo was only shown on the News page and at the bottom of posts? That way it is out-of-the-way, but still discoverable. Additionally a website footer could be made, which could include the RSS link.

@TrialDragon
Copy link
Member Author

I don't think RSS is used enough to get a place in the header

From #1072 (comment), this shouldn't be needed. We could add a title in the html header link.

We already have the atom link in the HTML head and that feature, AFAIK, isn't supported by default in any of the modern browsers (that article you mentioned in the issue is from almost 20 years ago and contains outdated info regarding browser support) and requires external extensions and tooling. I can try to find some place to put it at https://bevyengine.org/news/, but that'll take a bit more effort to make it look good.

I'm not a fan of the bright orange RSS logo; it clashes a lot with the current website design. What if we used a black + white variant that could use Bevy's theme?

That can work, yeah. There should be some freely available feed icon for use in that color scheme, or we can just make one ourselves if need be.

@mockersf
Copy link
Member

mockersf commented Mar 4, 2024

What if the RSS logo was only shown on the News page and at the bottom of posts? That way it is out-of-the-way, but still discoverable. Additionally a website footer could be made, which could include the RSS link.

Sounds good 👍

It's less intrusive than sticking the feed icon in the header and the site could use a footer.
It looks better on the site and modern. It's from the feather icons set: <https://github.com/feathericons/feather>
@TrialDragon
Copy link
Member Author

Updated it to use a neutral colored svg and reside in a website footer.

I forgot to `git add -A`, only `git commit -a`
sass/components/_footer.scss Outdated Show resolved Hide resolved
Copy link
Member

@mockersf mockersf left a comment

Choose a reason for hiding this comment

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

looks good!

Copy link
Member

@BD103 BD103 left a comment

Choose a reason for hiding this comment

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

I'm not sure if I like the footer in its current form, but I'm not really sure how to improve it.

I'm going to mark this as approved, but in the future I think it would be good to revisit this and see if we can rethink what goes in to the footer. (Maybe important links / contact info?)

The width and height can just be set on the elements themselves.
@TrialDragon TrialDragon added the S-Ready-For-Final-Review Ready for a maintainer to consider for merging label Mar 5, 2024
@TrialDragon
Copy link
Member Author

I'm not sure if I like the footer in its current form, but I'm not really sure how to improve it.

I'm going to mark this as approved, but in the future I think it would be good to revisit this and see if we can rethink what goes in to the footer. (Maybe important links / contact info?)

Agreed. I considered adding more, but that would require a lot of design and was out of scope for this specific PR (tbh, switching to the footer was already toeing the line of the scope imo even tho I did go with it). I'll open up an issue (or maybe a discussion?) if this merged so that the footer can have design decisions made and then be improved.

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

I've resisted the footer for awhile but I do think it is probably inevitable as we expand the site (and run out of space in the headerbar).

I did change the icons to grayscale as I don't think the colors were a good fit.

I removed the "author" and "authors" redundancy by embracing "authors". I also made it default to Bevy Contributors (and removed the explicit definitions in those cases), although now that I'm thinking about it, that would probably break the RSS attribution in those cases. I'll add those lines back.

@cart cart enabled auto-merge March 6, 2024 00:41
@cart cart added this pull request to the merge queue Mar 6, 2024
@TrialDragon
Copy link
Member Author

I removed the "author" and "authors" redundancy by embracing "authors". I also made it default to Bevy Contributors (and removed the explicit definitions in those cases), although now that I'm thinking about it, that would probably break the RSS attribution in those cases. I'll add those lines back.

I did not realize that would work, otherwise I would've done that back when we updated to Zola 0.18.0. authors was documented only as something that can be set in front matter and not in templates (I guess I have something to look into and contribute to the Zola documentation now); I thought it was just like the weight variable

Merged via the queue into bevyengine:main with commit 6ddb3c8 Mar 6, 2024
10 checks passed
@TrialDragon TrialDragon deleted the 1072_improve_bevy_news_rss_feed branch March 6, 2024 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-News C-Enhancement New feature or request C-Usability S-Ready-For-Final-Review Ready for a maintainer to consider for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make RSS feed more prominent
4 participants