Closed danielfdickinson closed 4 years ago
Hello Daniel,
thank you for making another submission attempt with a revised version of your theme. Let's start straight with the review:
stable-{version}
branch. Pulling releases from a single, pre-defined branch is much easier in the long run.https://themes.gohugo.io/theme/{some-theme}/
. In search.min.js
you hard-coded the url to the search index as /index.json
. This way you refer to https://themes.gohugo.io/index.json
instead of https://themes.gohugo.io/theme/{some-theme}/index.json
. A possible work-around is to define the url to the search index as variable in combination with a template function such as absURL
:var url = {{ "index.json" | absURL }};
// include search.min.js afterwards and use the url instead of a hard-coded path
@digitalcraftsman Thanks again for all you hard work on hugo themes repository and especially for the help you have given me in the past (and now).
I've been thinking about the branch thing and decided the best way forward is to make master the release (stable) branch and make the development branch something like devel-
On at least some projects I may fork off and oldstable-x.x.x branch when stable changes, but I think user master for the current production version rather than development better suits most expectations and norms.
Anyway, on the search form: Thanks for catching that -- it wasn't even on my radar. The workaround seems quite satisfactory for my needs.
That has, however, reminded me of another things that's not going to work on the theme site, and that is my contact form -- it is designed to contact the same server on a different port and doesn't go out some other site (and I don't want to change that).
Is there a way to not render certain pages only when for the hugo themes site? (Assuming exampleSite is being used and not the hugoBasicExample. If it's just a standard hugoBasicExample, it's non-issue).
I realized for the conditional render I can check baseURL...so I'll try that out this week.
This issue has been automatically marked as stale because it has not had recent activity. Feel free to ask questions. We're glad to help. This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.
Been busy. Will open a new issue if necessary, once ready.
@digitalcraftsman I believe I've fixed the issues with the search (no longer hard coded, and works using the test instructions in this repo's README). Also I've made master the stable branch (so the URL for the repo is https://github.com/cshoredaniel/new-oldnew-mashup.git . Development will be done on other branches and only merged into stable for releases. I also fixed a few other nits I noticed, unrelated to the theme site. If you could kind review again it'd be much appreciated!
Hello @cshoredaniel
When I try to test your theme I get the following ERROR
Error: module "github.com/cshoredaniel/necolas-normalize.css" not found; either add it as a Hugo Module or store it in "/home/alex/work/src/hugoThemes".: module does not exist
It seems that you need to configure that module again.
Hmmm... I wonder if it's related the version of Hugo. My CI tests start from scratch on Travis and succeed, but I've been using an older version because there was a version that was segfaulting in the Travis Bionic environment (I believe I filed a bug about that). I guess it time to try the latest version and do necessary updates.
@cshoredaniel The themes repo always uses the latest Basic version of Hugo, so it's advisable to check your theme with the latest.
I am not sure whether the ERROR is caused due to the way that module is named i.e. necolas-normalize.css
. The .css
extension may be causing the issue, but I'm not sure, that's just a hunch, but if it's true, then probably a bug report may need to be filed at https://github.com/gohugoio/hugo/issues
Ok, I've updated the theme (it now only uses Hugo modules, no more git submodules). I didn't need to quite do do that (the real issue was some difference in the generated hash ca. 0.58.0 or .59.0 compared to earlier versions; no bug to report; also the Travis CI tests now work, so the reason I was sticking with 0.57.2 has been resolved).
I did notice that the generateTheme.sh results in the use of my exampleSite instead of basicTheme -- either will work, but I wanted to make sure that was intentional and not because of some issue with generateTheme.sh script.
I hope you like the new theme (and it's another theme that is using Hugo modules that folks can look at, if you think it worthwhile.
Oh, and I caved and added back at least rudimentary IE11 support -- I was trying to drop it before, but my stats show too many people trying to access my main website with IE11 for that to work. Pity.
I did notice that the generateTheme.sh results in the use of my exampleSite instead of basicTheme -- either will work, but I wanted to make sure that was intentional and not because of some issue with generateTheme.sh script.
That's because you have mounted the exampleSite's content using Hugo Modules. Currently the Build Script does not enforce the hugoBasicExample through Hugo modules and as a result the override does not work in your case.
However your theme's previous version was whitelisted so that the demo could render fine and I am fine with having your provided exampleSite used in the demo.
If the Build Script gets adapted to take into account Hugo Modules we will add your theme in the whitelist again.
I have added the theme in the repo in 3f14ab9
@digitalcraftsman
The new version of the throwback Old New Mashup theme has now been added to the Themes Showcase.
Thank you for all your help, and adding my theme.
@cshoredaniel I've promote your theme on Hugo's official Twitter account.
@onedrawingperday thanks for your support in this issue.
OldNew Mashup theme submission
This basically a complete rewrite of the old OldNew Mashup theme I asked you to remove. It's much, much better, validates against W3C org's tools, and looks better.
I hope you like if you get a chance to check it out!
NB The License file I used is LICENSE not LICENSE.md, however I have an appropriate licenselink in theme.toml and testing using the theme site generateThemeSite and reveals that the link works as expected (goes to GitHub repo's copy of LICENSE).
Also, the theme works as expected with the hugoBasicTheme, but the content under exampleSite shows off more features of the theme. I have a demo site (linked as the theme homepage) for the theme so it's not essential that exampleSite be used.
Link to my theme repository: https://github.com/cshoredaniel/new-oldnew-mashup/tree/stable-0.9
I made sure that...
README.md
describing my themeLICENSE
theme.toml
images/
folder with the required dimensionshttps://example.com
is set as base url inexampleSite/config.{toml, yaml, json}
to avoid the abuse of unused domainstoCSS
andPostCSS
that I have committed the/resources
directory with all generated assets, for my theme to work in the basic version of HugoI'll do my best to keep the theme maintained, or let you know if it no longer is alive.