gohugoio / hugoThemes

A curated directory of Hugo themes
https://themes.gohugo.io/
MIT License
1.77k stars 245 forks source link

Please add castanet to the themes directory #252

Closed mattstratton closed 7 years ago

mattstratton commented 7 years ago

Hey hugo pals!

I would love for you to add https://github.com/mattstratton/castanet to the directory.

The repo is located at https://github.com/mattstratton/castanet

Since it's a podcast, it doesn't work well with the normal examples, but I have a fully functional exampleSite folder, which builds http://sample-castanet.netlify.com/ (as an example of the example, heh)

digitalcraftsman commented 7 years ago

Hi Matt,

thank you for reaching out to us :+1:

To take the last mile please add a thumbnail and a screenshots with the right dimensions to the your theme. Those will be used on the demo site to showcase your theme. You can find the necessary steps in the README.

Currently, the build script parses version numbers as floats but it fails if the float contains two dots. While it's a valid version number it also produces an error when being parsed as a float. Such an error occurs with the theme_version attribute in the theme.toml. I don't know how deeply integrated this version number is in your theme and we should find a workaround for the build script. Would it be possible to remove the attribute temporarily?

Please make sure that you link assets like images always with absURL or relURL. Because the demo is hosted in a subfolder and your using a path relative to the root of the server/host the demo site will return a 404 error for certain images.

Both your custom demo on Netlify and our theme demo have menu items that points to non-existing pages, namely the the contact and resources > Books link.

I hope I don't bother you with this comment :wink:

mattstratton commented 7 years ago

I just updated the thumbnaiil and screenshot...not sure how the wrong ones ended up in there.

My build process isn't dependent upon that version number (I have used it in other, private themes, so it's a habit). I can whack it.

I'll have to check on the images thing; I do some tricky stuff with that value where sometimes the user passes an absolute and sometimes a relative; I'll see what I can do to fix it.

And I'll fix the 404 :)

bep commented 7 years ago

I suspect

theme_version = "0.3.0"

Would be fine.

digitalcraftsman commented 7 years ago

@mattstratton thanks for being so responsive. Just ping me if you think I should take another look

@bep earlier the build script returned the following error:

ERROR 2017/04/27 21:10:02 failed to parse page metadata for castanet.md: (19, 17): cannot have two dots in one float
bep commented 7 years ago

@digitalcraftsman that is not the build script -- it is the TOML parser in Hugo; that is not valid TOML. Making it a string will solve that. If you get many of these you can add a replacement here:

https://github.com/spf13/hugoThemes/blob/master/_script/generateThemeSite.sh#L27

But now getting way off topic.

Very nice theme, btw. I bet useful for lots of people.

mattstratton commented 7 years ago

Good call. I'll fix the TOML! you can tell I'm a yaml person ;)

I'm digging into the code for some other stuff today, so I'll fix everything else and comment here when it's ready to be reviewed again.

digitalcraftsman commented 7 years ago

@bep ah, I didn't noticed that you wrapped the version number with a quotes.

Very nice theme, btw. I bet useful for lots of people.

I can only second that. Hopefully, we see more Hugo-based podcast websites :wink:

mattstratton commented 7 years ago

OK, I think I resolved all the issues. Can you take a look again?

BTW, there are a couple other podcasts that are using a fork of the customized theme I did for Arrested DevOps, and I'm planning to a) port Arrested DevOps to this theme soon-ish, and then convince those other podcasts to switch to Castanet :)

digitalcraftsman commented 7 years ago

Some menu items still link to unintended places.

The name of the website in the top-left of a page is often used as a homelink. But "/" | absURL will indicates that the root of the server / host should be linked. Hence the subfolder will simply be ignored by absURL / relURL. It would point to www.themes.gohugo.io/ instead of www.themes.gohugo.io/theme/castanet/. Just use .Site.BaseURL

Due to the same reason the link to the contact page points to www.themes.gohugo.io/contact/ instead of www.themes.gohugo.io/theme/castanet/contact/. Removing / as prefix from all menu item urls in the config file solves this problem.

And I've to bug you again with this issue :) - /js/castanet-min.js can't be found due to the way it's linked.

mattstratton commented 7 years ago

Yeah, I was using relURL for both the CSS and the javascript, because of build issues with Netlify (it ends up with cross-site scripting because of something with my build setup). I can make those use absURL instead and figure out my Netlify issue later.

mattstratton commented 7 years ago

@digitalcraftsman I think I got it resolved; it's causing issues with my deploy previews on Netlify, but I can fix that myself elsewhere.

digitalcraftsman commented 7 years ago

I'm sorry, but the URLs are still broken in the demo. Whether you use absURL or relURL is not important for the demo. Just do not prefix URLs with /.

As I mentioned in my last comment both template funcs will ignore any subfolder if your URL is / or /path/to/asset.ext. For example, "/" | relURL indicates that the new URL should be relative to the root directory of the server, not relative to the base url. Hence the subfolder will simply be ignored by absURL / relURL. It would point to www.themes.gohugo.io/ instead of www.themes.gohugo.io/theme/castanet/.

It would be the same if you try /path/to/asset.ext | relURL.

I hope you can incorporate this behavior with your own demo on Netlify.

mattstratton commented 7 years ago

Aha! This makes total sense now. Sorry for being challenging with this; I haven't had a use case for my own Hugo implementations for subfolders, so I am dumb about it :)

I think I fixed it, but it's hanging out in a PR right now so that Netlify can help me troubleshoot my build issues with doing it this way. I'll at-mention you when I merge it.

Additional question - since the themes are submodules, does your build pull the latest when you build the gallery site? Just wondering since I'm going to be doing a lot of updates in the next month or so, and it would be grand if the details keep updating :)

digitalcraftsman commented 7 years ago

Sorry for being challenging with this; I haven't had a use case for my own Hugo implementations for subfolders, so I am dumb about it :)

No probem. I'm glad I could help you to resolve the problem.

Each new pushed commits triggers Netlify to do a rebuild of the theme site. The updates of the submodules happen manually. They aren't scheduled but happen mostly at least once a week. Feel free to contact us if you've done a ton of changes and you think the ~1 week update cycle would not reflect the latest state of your theme.

mattstratton commented 7 years ago

I think weekly is totally fine for me...unless I do something major, but mostly I'm in bug-fix, small enhancement mode, and most of those won't reflect the screenshot anyway :)

I just merged the change that I think will fix it - my build problem turns out to be a bug in netfliy and how it generates deploy-preview URLs, so I'm not going to hold this up for that right now. Hopefully this nailed it!

digitalcraftsman commented 7 years ago

That looks much better :wink: But I've to keep you busy:

I checked the pagination of your theme on the homepage. Every link works except those that name the page number in the middle. For those you use absURL as well, but this generates link relative to the root directory. When serving the page on localhost Hugo's absURL template function seems to remove the http://localhost:1313 part of the URL so that /page/2/ remains.

But we already now what happens with such an URL and absURL. Hugo already generates a correct URL. absURL doesn't seem to be a silver bullet in every case.

Btw, for the single pages of an episode absURL should be used for the episode_image.

mattstratton commented 7 years ago

I fixed (locally) the episode_image stuff. I'm not quite sure what your recommendation is regarding the paginator - should I just remove the absURL call in that line? The problem is that it seems to then create it relative to the root, as you mentioned.

Instead of trying to use the magic of absURL, should I try hacking with baseURL do you think?

mattstratton commented 7 years ago

So I tried this:

"{{ $.Site.BaseURL}}{{ .URL }}"

And that works, but it makes an ugly URL; the URL ends up with an extra slash, which the browser can handle, but it's crappy: http://localhost:1313//page/3/

mattstratton commented 7 years ago

Sidenote - going through this exercise is making me a better theme developer! I appreciate your patience and guidance.

mattstratton commented 7 years ago

@jhabdas That doesn't seem to work in a pagination situation; here's a code snippet:

  {{ range $paginator.Pagers }}
  <li class="{{ if eq . $paginator }}active {{ end }}page-item"><a href="{{ .RelPermalink }}" class="page-link hidden-md-down">{{ .PageNumber }}</a></li>
  {{ end }}

That results in can't evaluate field RelPermalink in type *hugolib.Pager

This is the code I'm using to help solve for what @digitalcraftsman mentioned above:

  {{ range $paginator.Pagers }}
  <li class="{{ if eq . $paginator }}active {{ end }}page-item"><a href="{{ $.Site.BaseURL}}{{ .URL }}" class="page-link hidden-md-down">{{ .PageNumber }}</a></li>
  {{ end }}

I've pushed this change up to master on my repo, so at this point I'm not quite sure how to solve for this better.

mattstratton commented 7 years ago

I think that anything that pops a "/" at the beginning will put it at the root of the FQDN, not the subdirectory, which is what's causing the issues.

I think that why mine is more complex than yours is that I'm using some more complex paginator stuff, since I am using my own pagniator stuff for the navigation at the bottom (since I use bootstrap 4)

mattstratton commented 7 years ago

I am pretty sure it's acceptable now; it's not the cleanest thing ever, but it works (I changed my baseURL to a subdirectory in the config file and generated local files with hugo -v, and saw that the URL's seem fine).

@digitalcraftsman I think we should be good now? I found a couple other random little bits too.

digitalcraftsman commented 7 years ago

There is a way to remove the double slash from the url by trimming the /s:

{{ range $paginator.Pagers }}
    {{ $url := trim (string .URL) "/" | absURL  }}
    <li class="{{ if eq . $paginator }}active {{ end }}page-item"><a href="{{ $url }}" class="page-link hidden-md-down">{{ .PageNumber }}</a></li>
{{ end }}
mattstratton commented 7 years ago

I was considering something like that, @digitalcraftsman, and bit the bullet and merged it. Can you take a look and see if this builds fine for you now? I built locally using a subdirectory and all the URL's and paths seem to work from the subdirectory location now.

digitalcraftsman commented 7 years ago

Hi Matt,

as you can already I've merged your theme. Everything looked fine. The demo is already up and running if you like to check it out on the theme site.

Last but not least thank you for your time for tweaking your theme. I'll spread the word on Twitter to put your awesome theme in the spotlight. Hopefully, you'll find new theme users :wink:

Cheers, Digitalcraftsman