add rss button in navbar

UtkarshVerma / hugo-dream-plus

:rainbow: An upgraded version of the Hugo "Dream" theme with tons of new features.

http://dream-plus-posts.netlify.com

MIT License

68 stars 66 forks source link

add rss button in navbar #22

Closed thomas-louvigne closed 6 years ago

thomas-louvigne commented 6 years ago

i have also delete trailing white space

UtkarshVerma commented 6 years ago

@thomasluquet Thanks for the PR. Is there any demo site for your patches? I can't check your patches currently because I'm can't access my PC.

thomas-louvigne commented 6 years ago

Yes : https://serene-sierra-72019.herokuapp.com/

UtkarshVerma commented 6 years ago

@thomasluquet I saw your site. While I do like the idea of providing a link to RSS feed through icons, there are some issues:

RSS feed isn't needed by most of the common internet users so providing a big icon for it doesn't seem right.
Placing it alongside Posts and Archives isn't right as they don't go together. There must be a sort of distinction between the RSS button and the rest of the buttons. What comes to my mind currently is biasing the feed button to the right of the top pane or something a bit more subtle.

thomas-louvigne commented 6 years ago

RSS feed isn't needed by most of the common internet users so providing a big icon for it doesn't seem right.

Yes, you can activate or desactivate in you config file ( enableRSS = true). You can set it at false by default.

Placing it alongside Posts and Archives isn't right as they don't go together. There must be a sort of distinction between the RSS button and the rest of the buttons. What comes to my mind currently is biasing the feed button to the right of the top pane or something a bit more subtle.

Good idea. i can try to do it.

UtkarshVerma commented 6 years ago

@thomasluquet Yep, disabling it by default would be good. Also good luck with the RSS button placement. Looking forward to it. :+1:

thomas-louvigne commented 6 years ago

Done !

UtkarshVerma commented 6 years ago

@thomasluquet Great! I checked it right now and it looks as it should on the home page. However it still isn't properly aligned in the post pages.

thomas-louvigne commented 6 years ago

W3C recommand to use 2 space for indentation , not tabulation. https://www.w3schools.com/html/html5_syntax.asp Are you OK to switch to 2 spaces-indetation ? I can correcting all template if you need in a next PR.

UtkarshVerma commented 6 years ago

@thomasluquet I did prefer tabbing instead of spacing but since W3C recommends it, I think spacing would be a better choice. You can make a separate PR for it. As for this PR, it seems to be correct, but I'll check the code once again before the merge.

thomas-louvigne commented 6 years ago

@UtkarshVerma : i think it works now with tabulation

UtkarshVerma commented 6 years ago

@thomasluquet Remove the tab/spacing related commits from this PR. I did ask you to submit another PR for it. It is difficult to track changes this way.

UtkarshVerma commented 6 years ago

@thomasluquet Lots of merge conflicts kept popping up, so instead of merging this PR, I just added the feature myself. Therefore closing this PR. You may reopen it if the need arises.