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

Added support for tags #196

Merged
merged 4 commits into from
Mar 25, 2019
Merged

Added support for tags #196

merged 4 commits into from
Mar 25, 2019

Conversation

pdelaby
Copy link
Contributor

@pdelaby pdelaby commented Nov 29, 2018

I love your theme, so I added the support for one of the default taxonomy in GoHugo : tags.

I also added documentation in the exampleSite, and the theme variants.

I only doubt about the CSS, if anyone have a better idea don't hesitate :-)

@matalo33 matalo33 self-assigned this Dec 21, 2018
@matalo33 matalo33 added the enhancement Improvements to existing features label Dec 21, 2018
@gilbsgilbs
Copy link

I am very interested by this feature. Can I help?

@matalo33
Copy link
Contributor

There's a couple of things if you would like to contribute.

  • Review the code
  • Run the branch on the example site or your own sites and test functionality

I notice @pdelaby added tags to the sample site however when this MR was opened the netlify build was not working. Could you rebase this against master to kick a fresh build? It will render an entirely new copy of the example site along with the changes.

@matalo33 matalo33 added help wanted need user feedback feature New Features and removed enhancement Improvements to existing features labels Mar 13, 2019
@matalo33 matalo33 added this to the v2.3.0 milestone Mar 13, 2019
@gilbsgilbs
Copy link

@matalo33 Thanks. I currently run a rebased version of @pdelaby's work on a private project and it works fine (except that I have to restart the server when I touch to taxonomies, but I think it's a limitation from hugo itself).

Here are some screenshots showing how it looks:
image
image

After clicking on a tag, it looks like this:
image

As far as I'm concerned, I think it looks good overall. If I had anything to say, I would say that the taxonomy page (third screenshot) is not very good looking, but I'm not sure how to improve it. At very least, punctuation is incorrect in english and should be fixed.

I don't have much to say on the implementation itself. It looks correctly written to me however I don't contribute a lot to Hugo themes.

There are obvious typos in the English docs, I'll mention the ones I find. Yet, the french version looks perfect.

tags: ["documentation", "tutorial"]
---

*Learn theme* support one default taxonomy of gohugo : the *tag* feature.

Choose a reason for hiding this comment

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

There's no space before colons in english.


## Configuration

Just add tags to any page :

Choose a reason for hiding this comment

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

There's no space before colons in english.

## Behavior


The tags are displayed at the top of the page, in the order wich they are enterted.

Choose a reason for hiding this comment

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

- wich
+ which
- enterted
+ entered

Not even sure the use of the verb enter is correct here. Maybe “in order of insertion” is slightly better?

<h1>{{.Title}}</h1>
<h1>
{{ if eq .Kind "taxonomy" }}
{{.Kind}} :

Choose a reason for hiding this comment

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

English doesn't have spaces before colons. Not sure how to handle this 🤔 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found no other usage of colons in the theme, except for the title <title>{{ .Title }} :: {{ .Site.Title }}</title>. Maybe we can use double quotes, or some kind of emphasis in order to get rid of the colon?

A good solution could be to use the lang attribute of the html tag, and CSS. Wrapping the colon in a span <span class="colon">:</span> could allow to use something like:

html[lang="fr"] .colon{
    padding-left:0.2em;
}

But even with the fr-fr language code in config.toml, I did not found a way to change the lang attribute in html (which is always en).

Choose a reason for hiding this comment

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

I'd say :: is good enough. Pretty sure I've already seen it somewhere. @matalo33 what do you think?

@@ -0,0 +1,7 @@
<div class="tags">
{{ if .Params.tags }}

Choose a reason for hiding this comment

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

Maybe this should wrap the div so that we don't even display the div when there are no tags.

<div class="tags">
{{ if .Params.tags }}
{{range .Params.tags}}
<a class="tag-link" href="{{ "/tags/" | relLangURL }}{{ . | urlize }}">{{ . }} </a>

Choose a reason for hiding this comment

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

Useless trailing space?

@pdelaby
Copy link
Contributor Author

pdelaby commented Mar 14, 2019

Thanks for the help @gilbsgilbs :-) Sorry for the english typos.

We are using this feature on our projects (with a forked repository for now) for 4 months now, and found no bugs yet. Some insights about it :

  • the tags are case insensitive,
  • you can search the tags, wich is really cool,
  • on the taxonomy page, we miss a feature which woud show the number of pages with the tage ( "my first tag (8)", "my second tag (2)", but I don't know how to implement it.

During edition (hugo server -D), I don't have to restart the hugo server to see taxonomy changes (add/remove a tag). What do you mean by "touch to taxonomies" ?

@gilbsgilbs
Copy link

What do you mean by "touch to taxonomies" ?

It's quite easy to reproduce:

  • Run the server
  • Create a new taxonomy that doesn't already exist
  • Click on the taxonomy link
  • Enjoy the 404
  • Restart the server
  • F5
  • Everything works

As I said, I'm pretty sure hugo is at fault here.

$ hugo version
Hugo Static Site Generator v0.54.0/extended linux/amd64 BuildDate: unknown

@matalo33
Copy link
Contributor

Works very well. I really like the styling of the nametag element on the content pages. I agree that the taxonomy page could use some more work. I am not a designer though :)

Some sample tag pages from other themes:
https://themes.gohugo.io//theme/hugo-theme-jane/categories/themes/
https://themes.gohugo.io//theme/hugo-flex/tags/shortcodes/
https://themes.gohugo.io//theme/hugo-w3-simple/tags/interesting/
https://themes.gohugo.io//theme/mero/tags/markdown

@pdelaby Please could you address the pull request feedback already provided?

we would show the number of pages with the tage ( "my first tag (8)", "my second tag (2)", but I don't know how to implement it.

Some of the examples above include this feature, however we can track this as a future improvement to get the feature released sooner than later.

@matalo33
Copy link
Contributor

Thanks for everything @pdelaby. If you could just get this branch rebased and ready for merge I'm happy to proceed.

@pdelaby
Copy link
Contributor Author

pdelaby commented Mar 25, 2019

Thanks :-)
So, just to be sure, the 4 commits on top of the updated master (upstream/master) ?

@matalo33 matalo33 merged commit 6207c44 into matcornic:master Mar 25, 2019
@matalo33
Copy link
Contributor

My bad, actually I could do this myself! Thanks again

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

Successfully merging this pull request may close these issues.

3 participants