codingteam / loglist

Reincarnation of the famous service
https://loglist.xyz/
MIT License
7 stars 4 forks source link

Link to RSS feed from the main page #158

Closed Minoru closed 8 years ago

Minoru commented 8 years ago

Originally I wanted to use the RSS icon as it's more prominent, but after 2 (two!) hours of trying to make it scale properly with the other <li>, I gave up. If any CSS wizards want to take over this, I'm happy to oblige.

Close #153

rexim commented 8 years ago

I'd suggest to keep things simple. If it looks fine on page I'm ok with mergin that. Let me check it locally.

rexim commented 8 years ago

So, if I understand correctly this change should look like this:

RSS

Won't the user accidentally click on the RSS instead of add a new quote all the time?

I'm actually thinking about a fixed positioned RSS icon button at the right bottom corner that follows the user's view while they scroll:

Fixed RSS Icon

@ForNeVeR @Minoru What do you guys think?

Minoru commented 8 years ago

Well, putting it on the margin doesn't look good to me—the page becomes "unbalanced". Maybe placing it next to pagination links is better? If user got through the first page, presumably they like our quotes enough to consider a subscription.

rexim commented 8 years ago

@Minoru sounds good to me.

ForNeVeR commented 8 years ago

Personally I'd place it in the footer, near the copyright info. Usually it's the place for all sorts of technical links, and I'd expect to see it there.

RSS link near the pagination links is also okay.

ForNeVeR commented 8 years ago

Also, @Minoru, fell free to contact me if you want to scale or position some shit with CSS, I have the necessary experience.

Minoru commented 8 years ago

How do you like it now?

@ForNeVeR, if you can get an image to scale to text's height in this gist, we'll be able to use an icon in the footer. But I'm pretty happy with the page as it is and am not entirely convinced that icon would look better.

rexim commented 8 years ago

@Minoru jsfiddle is much more convenient for such thing than gist https://jsfiddle.net/y9tfrstk/

ForNeVeR commented 8 years ago

I'm on it.

ForNeVeR commented 8 years ago

Well, guys, there's a simple CSS metric that matches the current font size. It's called em. height: 1em pretty much does what you want, check this jsfiddle: https://jsfiddle.net/y9tfrstk/3/

ul.menu li img {
  height: 1em;
  vertical-align: text-bottom; // looks nice this way
}
Minoru commented 8 years ago

R-right… Now I feel kinda stupid for only thinking of HTML height attribute and not of CSS :(

Anyway, thanks @ForNeVeR; the PR is updated with your idea.

ForNeVeR commented 8 years ago

@Minoru should we still consider it as a work in progress? Or should we start the review & merge process?

Minoru commented 8 years ago

Right; thanks. I think it's ready to be reviewed.

rexim commented 8 years ago

@ForNeVeR I have some technical difficulties right now. Could you please take this PR?

ForNeVeR commented 8 years ago

Okay, I'll try to review that tonight.

rexim commented 8 years ago

Ah! I'm on vacation and totally forgot that everyone else still works! :D Sorry. I guess will be able to review that in a couple of hours once I get my problems resolved.

ForNeVeR commented 8 years ago

First of all, I've built the site with these patches, and it looks pretty good. All the functionality works.

ForNeVeR commented 8 years ago

Overall it looks good, but we need the answer to my question.

ForNeVeR commented 8 years ago

Looks nice, thank you very much. I'll merge and deploy that tonight.