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 tag support #48

Merged
merged 10 commits into from
Sep 17, 2017
Merged

Add tag support #48

merged 10 commits into from
Sep 17, 2017

Conversation

divayprakash
Copy link
Contributor

@divayprakash divayprakash commented Jun 16, 2017

Closes #32

Screenshots in comments below

@divayprakash
Copy link
Contributor Author

Markdown and HTML

@divayprakash
Copy link
Contributor Author

Feature Images

@divayprakash
Copy link
Contributor Author

Tags Page

@divayprakash
Copy link
Contributor Author

divayprakash commented Jun 16, 2017

Tags can be added by simply following the Jekyll front-matter convention and adding tags: [comma separated tags, comma separated tags] to the post's front matter.

I used the same theme styles as the a element styled with the button class (ie. <a class="button" ...>) to display a list of tags under a post title/feature image. The tags page is similar to the index page in terms of styling. Thus no styles were added/changed for this feature.

Tags can be disabled by adding exclude: [tags.html] to _config.yml which excludes the file from the Jekyll build. In addition, tags syntax present in a post's front matter would have to be removed.

@rohanchandra

  • Just something to think about, should I also display the post tags on the index.html page in the same location ie. under the post title? I haven't implemented that as of now.
  • Should I hide the tags page from appearing in the header's navigation bar?
  • The type-theme docs would need to be updated for this feature. I understand the change would need to be made to this file. How should we proceed with that?

Copy link
Owner

@rohanchandra rohanchandra left a comment

Choose a reason for hiding this comment

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

Hey @divayprakash, this is awesome!

Just some tweaks to the design and markup:

  1. Move tags to below the content into thefooter of the article.
  2. In terms of design, it'll be best to match the header meta design with the center alignment + text styled with the .meta class (see below)

screenshot

3. Can we make the tags page use a design that's consistent with the index page:

tags

Just something to think about, should I also display the post tags on the index.html page in the same location ie. under the post title? I haven't implemented that as of now.

I think it's best to keep the index free of the tags, as the index in this theme is intended as more of a summary.

Should I hide the tags page from appearing in the header's navigation bar?

Yes, hide the tags page link by default, we can explain in the docs how to add a nav link to the tags page, if users want to include a link to the page (or otherwise include it as an option)

The type-theme docs would need to be updated for this feature. I understand the change would need to be made to this file. How should we proceed with that?

Yep, please fork the repo at https://github.com/rohanchandra/rohanchandra.github.io/ to add your documentation changes 👍 Feel free to make a new heading for tags which includes information the user would need to use this feature (i.e. adding tags to a post, adding an optional navigation link to tags page and disabling the feature).

Thanks for this pull request!

@@ -9,7 +9,15 @@ <h1 class="title">{{ page.title }}</h1>
{% if page.author %} - {{ page.author }}{% endif %}
</p>
</header>
<section class="post-content">{{ content }}</section>
<section class="post-content">
{% for tag in page.tags %}
Copy link
Owner

Choose a reason for hiding this comment

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

Tags would be best placed in a new <footer> HTML element inside the <article> below the post content, rather than within the post content (.post-content)

@divayprakash
Copy link
Contributor Author

@rohanchandra I'll make the changes as you said but it might take a couple of days, just a bit busy over the weekend.

@divayprakash
Copy link
Contributor Author

@rohanchandra

  • index.html is free of tags.
  • Tags page is removed from the header's navigation bar by default.
  • Tags page styling is more consistent with the index page.
  • I've moved the tags on the post pages to a <footer> element at the end of the <article> element after the post-content div.
  • Tags are now centered and styled using the meta class.

For more please see screenshots below.

@divayprakash
Copy link
Contributor Author

tags

@divayprakash
Copy link
Contributor Author

feature
markdown

@divayprakash
Copy link
Contributor Author

In styling the tags page to resemble the index page, I find that the tag names and the post title under that tag are too close in size and difficult to distinguish. An alternate to this could be styling the tag names in the $link-color as below -
tags_alt

@rohanchandra
Copy link
Owner

rohanchandra commented Jun 19, 2017

Looks great! Thank you for making those changes to the tag design and page.

Just two final changes:

  1. Reuse the existing .meta class instead of copying the CSS declarations from .meta
  2. Make the margin-right on the tags.html consistent with the post button margin

In styling the tags page to resemble the index page, I find that the tag names and the post title under that tag are too close in size and difficult to distinguish. An alternate to this could be styling the tag names in the $link-color as below

Try setting the margin-bottom of h1 on the tags page to 0, should help a bit with the visual grouping (see screenshot).

I want to keep $link-color for links, so if you want to add $link-color to the h1, I think it would be best to make the heading an in-page link. e.g. clicking the 'feature-image' heading link would link to ./tags#feature-image

Copy link
Owner

@rohanchandra rohanchandra left a comment

Choose a reason for hiding this comment

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

Comments attached. Thanks!

font-size: 0.9em;
display: inline-block;
text-align: center;
.meta {
Copy link
Owner

Choose a reason for hiding this comment

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

The .meta rule shouldn't be copied and pasted. Can we refactor the .meta rule used in the header instead.

margin-top: 0;
margin-bottom: 0;
}
.meta {
Copy link
Owner

Choose a reason for hiding this comment

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

Reuse the existing .meta class styling

tags.html Outdated
{% for item in (0..site.tags.size) %}
{% unless forloop.last %}
{% capture this_word %}{{ tag_words[item] | strip_newlines }}{% endcapture %}
<a class="button" href="#{{ this_word | cgi_escape }}">
Copy link
Owner

@rohanchandra rohanchandra Jun 19, 2017

Choose a reason for hiding this comment

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

Button margin isn't consistent with that on the post pages. Please adjust the CSS styles for tags.html so it has the same margin-right on the buttons as the post pages.

@divayprakash
Copy link
Contributor Author

Try setting the margin-bottom of h1 on the tags page to 0, should help a bit with the visual grouping (see screenshot).
I want to keep $link-color for links, so if you want to add $link-color to the h1, I think it would be best to make the heading an in-page link. e.g. clicking the 'feature-image' heading link would link to ./tags#feature-image

Done in 5cd1902.

@divayprakash
Copy link
Contributor Author

Reuse the existing .meta class instead of copying the CSS declarations from .meta

Done in 4adf5c8.

@divayprakash
Copy link
Contributor Author

Make the margin-right on the tags.html consistent with the post button margin.

Done in de0ad37.

@divayprakash
Copy link
Contributor Author

This is the final tags.html
1

@divayprakash
Copy link
Contributor Author

@rohanchandra Hey! I'm still making a few changes so just wait until I update this regarding the same. Thanks!

@divayprakash
Copy link
Contributor Author

So I think this is done now, just finished up adding some liquid to the posts template to render tag/tags depending on number of tags and also to not render the footer element containing tags in case the post does not have tags.

I'd love to hear your thought on this, so please let me know! And if you can think of anything else we can add on then I can work that out too.

@rohanchandra
Copy link
Owner

Great work @divayprakash, this looks really good. Can you update the docs for this feature? I'm thinking info on how to add tags to a post / optional navigation link to tags page (or use of tags.html) and disabling the feature would be good to have.

I'll test out your branch for testing and then merge it in after this 👍

@divayprakash
Copy link
Contributor Author

@rohanchandra I'm using this theme for my blog so I've added category support too in the same manner as tags. The equivalent page for listing the various categories and the posts under them is categories.html and looks the same as tags.html. The categories for a post are also rendered in a footer element and look the same as the existing tags implementation. Should I add the same to this PR?

@rohanchandra
Copy link
Owner

@divayprakash What's the difference between a category and tag?

@divayprakash
Copy link
Contributor Author

  • Tags don't change the post url in any manner. However a post in the categories 'general, code' will have the URL /general/code/2017/06/20/foo-bar.html
  • Permalinks don't support tags but do support categories (Source : StackOverflow)

@divayprakash
Copy link
Contributor Author

divayprakash commented Jun 20, 2017

As the categories change the post url, they provide an easy way to organise posts into folders depending on content or whatever else the author chooses.

Instead of placing posts inside of folders, you can specify one or more categories that the post belongs to. When the site is generated the post will act as though it had been set with these categories normally.

Source : Jekyll docs

@rohanchandra
Copy link
Owner

Oh cool I didn't know about that. Feel free to make a separate PR for that.

@rohanchandra
Copy link
Owner

@divayprakash, did you want to write the docs for this feature? If not, I can write them then merge the feature in.

@divayprakash
Copy link
Contributor Author

@rohanchandra Hey! Sorry about the late response but I got caught up in my brother's wedding and then campus placements. I'd be glad to finish up the docs for this feature but it might take me some time. I hope that's okay.

@rohanchandra
Copy link
Owner

rohanchandra commented Jul 13, 2017

@divayprakash No worries 😄! That's totally fine, I'll leave the docs with you

@divayprakash
Copy link
Contributor Author

@rohanchandra I'm extremely sorry for the delay but I won't be able to write the docs after all. If you could go ahead and finish that up then I'll also push the code concerning categories. Cheers!

@rohanchandra
Copy link
Owner

@divayprakash No problem, I'll write up the docs soon.

Just to confirm the things to note are:

  • Add tags to a post by adding tags: [tag_name] to the Markdown
  • Disable the tag index listing by adding exclude: [tags.html] to _config.yml

Is there anything else you think I should include?

@rohanchandra rohanchandra merged commit 6e93e3d into rohanchandra:master Sep 17, 2017
Repository owner deleted a comment from divayprakash Jul 22, 2019
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.

Tag support
2 participants