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

PR: Add atom feed support #136

Merged
merged 2 commits into from
Jul 24, 2018
Merged

PR: Add atom feed support #136

merged 2 commits into from
Jul 24, 2018

Conversation

dalthviz
Copy link
Member

@dalthviz dalthviz commented Jul 9, 2018

Following #126

Needs the no-prune option to work (otherwise the feed.xml is deleted)

A preview:

imagen
imagen

@dalthviz dalthviz self-assigned this Jul 9, 2018
@dalthviz dalthviz requested a review from ccordoba12 July 9, 2018 19:41
@ccordoba12
Copy link
Member

Thanks @dalthviz, it looks good!

However, I don't like that RSS link is placed next to the main navigation links. What if we put it like this

Spyder | Blog RSS         Home Blog Docs

with RSS in the same font as Home Blog Docs but with a smaller size?

@dalthviz
Copy link
Member Author

A new preview:

imagen

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @dalthviz!

Let's merge this one to see how it looks.

@ccordoba12 ccordoba12 merged commit cf05abe into spyder-ide:develop Jul 24, 2018
@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Jul 24, 2018

The formatting/look of the link element itself is fine but I don't think I like the position personally, as it is in a much too central and prominent location for something relatively few are likely to use, and seems out of place right next to the homepage logo in the sidebar. It would be much better off either all the way to the right of the navbar (Home, Docs, etc) or in the footer, IMO.

@ccordoba12
Copy link
Member

ccordoba12 commented Jul 24, 2018

It would be much better off either all the way to the right of the navbar (Home, Docs, etc)

That was @dalthviz original proposal, but I didn't like it.

or in the footer, IMO.

I think it'd be completely lost in the footer.


Since I think it looks nice where it is, I'm going to merge it to production right now.

@CAM-Gerlach
Copy link
Member

I think it'd be completely lost in the footer.

Well, I don't disagree, but if that's the case then I'm afraid all our social media/etc. links are as well. The more pressing reason is that the footer is supposed to be common among all sites, while RSS is only for the blog, and we'd have to make a place for it there somewhere.

That was @dalthviz original proposal, but I didn't like it.

Why not? Back when sites commonly had RSS links, this was the standard convention for sites that put it on their top bar from what I recall; never right next to the logo. Of the many peer blogs I found, even including the relatively small number on Planet SciPy that are still active (perhaps a half dozen or so have any posts in the past 2 1/2 months), only two had RSS feed links visible somewhere: one had it in the footer (one of the Planet SciPy individual blogs) and one in the very right of the top bar, as an icon (Rstudio).

However, especially if you are insistent on the current position, the [RSS icon](https://en.wikipedia.org/wiki/File:Feed-icon.svg/ or some monochrome/simplified variant like on Rstudio would look far better just to the right of the "Spyder | Blog" header image than a random bit of text as is now.

@CAM-Gerlach
Copy link
Member

Actually, come to think of it having it as a simple icon rather than text is much more important than the position either way, since I can see why you wouldn't want it to the right of the existing text as-is either, as we want it to look very distinct from those navigation links; conversely, having it as an icon looks a lot cleaner and more purposeful in either position than having the text link either spot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants