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

Add rss icon #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add rss icon #37

wants to merge 1 commit into from

Conversation

j-delaney
Copy link

Summary

Adds an RSS icon to the icon library taken from from https://feathericons.com/?query=rss

Motivation

As discussed in Discord, so that we can add an <Icon name="rss"> RSS Feed to the bottom of https://svelte.dev/blog. I figured it would be helpful for people who need a direct link to the RSS feed so they don't need to go looking through the page source

Test plan

Tested on the REPL: You can see this in action at https://svelte.dev/repl/1f1ae8d9d155435f82549f97d9d36a9c?version=3.32.0

@benmccann
Copy link
Member

It's a nice icon. I wonder how it would be used on svelte.dev. Perhaps we should see that to make sure we agree it's the best way to represent it and it will actually be used before we merge it here

@peopledrivemecrazy
Copy link

Been wondering the same, maybe we can add a footer since the site lacks it or we can add along with the header icons (github, discord).

@peopledrivemecrazy
Copy link

Vuejs news has them on every article like this; a static sidebar

image

@j-delaney
Copy link
Author

@benmccann
Screen Shot 2021-01-26 at 9 42 35 PM

I was thinking we could put it at the bottom of the blog listings page like so. (source code)

I like what @peopledrivemecrazy posted and think it would be great for the blog section to have a thing like that, but that would require Svelte to have a newsletter subscribe thing first. I'm a bit hesitant about putting it in the header of every page since it isn't exactly the best interface were someone to click it (just downloads an xml file to their computer)

@peopledrivemecrazy
Copy link

peopledrivemecrazy commented Jan 27, 2021 via email

@benmccann
Copy link
Member

Seems like a good starting point. My main suggestion would be to make the whitespace between the last article and RSS link the same as the whitespace between articles

@j-delaney
Copy link
Author

@benmccann Great idea! Here's what it looks like now:
Screen Shot 2021-01-27 at 8 45 23 AM

I updated my branch as well. I have a question about CSS best practices I'd love advice on. In my branch I set the RSS's div to have a margin-top to match what the h2 blocks in each article has, like so:

.rss {
    margin-top: 3.2rem;
}
 ...
h2 {
    display: inline-block;
    margin: 3.2rem 0 0.4rem 0;
...

Would it be better to put this into a variable or is it okay to hardcode it since it's only be used in 2 places (in the same file)?

@peopledrivemecrazy
Copy link

    .rss {
		margin-top: 3.2rem;
	}

I just tested and it works well

image

@j-delaney
Copy link
Author

@benmccann This is a minor detail, but I like the icon more when the Icon itself has vertical-align: text-top;

With vertical-align: middle (current state of all icons on the site):
Screen Shot 2021-01-27 at 3 27 47 PM

With vertical-align: top:
Screen Shot 2021-01-27 at 3 28 23 PM

What do you think about having that be controlled by a parameter to Icon? I don't think it should be set globally because it makes some usages of icon look funny (like the icon next to "Log in and save" on the REPL page).

@benmccann
Copy link
Member

FYI, we're moving this repo to https://github.com/sveltejs/sites/tree/master/packages/site-kit. That location is currently used only for the SvelteKit docs, but we hope to eventually use it for all docs

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