Skip to content
This repository has been archived by the owner. It is now read-only.

New theme: Amperage #782

Closed
8 of 10 tasks
asurcodes opened this issue Dec 11, 2019 · 12 comments
Closed
8 of 10 tasks

New theme: Amperage #782

asurcodes opened this issue Dec 11, 2019 · 12 comments

Comments

@asurcodes
Copy link

@asurcodes asurcodes commented Dec 11, 2019

Theme submission

Please make sure that you have read the theme submission guidelines before submitting a theme. The guidelines provide all relevant information and requirements that have to be fulfilled before the submission. Please tick the relevant boxes for your theme in the checklist below:

Link to my theme repository: https://github.com/asurbernardo/amperage

I made sure that...

  • the repository contains a good README.md describing my theme
  • an open source license has been added to LICENSE.md
  • all metadata have been added to theme.toml
  • screenshots have been added in the images/ folder with the required dimensions
  • in case I'm using a customized demo via the exampleSite folder that
  • I tested my theme against the gohugoio/HugoBasicExample
    • I've checked the developer tools' console in my browser for error messages
  • in case my theme is using Hugo Pipes features like toCSS and PostCSS that I have committed the /resources directory with all generated assets, for my theme to work in the basic version of Hugo

N.B. By submitting a theme to the Hugo Themes Showcase you understand that you need to maintain your theme. You will need to keep an eye on whether your theme's demo generates on the Hugo website, whenever there is a new release. We are a very small team and we just cannot keep up opening issues in the various theme repositories to notify authors. Whenever a theme demo breaks and remains broken then at some point it will be removed from the list without prior warning. If you no longer wish to maintain a theme please let us know.

Question:

I have noticed while building hugoThemes site to test the theme $url | absURL links work fine but $url | absLangURL links refer to the root domain instead of the theme subfolder. Does this have anything to do with my project? 🤔

@asurcodes asurcodes changed the title New theme: Amperage New theme: Amperage Dec 11, 2019
@onedrawingperday
Copy link
Contributor

@onedrawingperday onedrawingperday commented Dec 12, 2019

I have noticed while building hugoThemes site to test the theme $url | absURL links work fine but $url | absLangURL links refer to the root domain instead of the theme subfolder. Does this have anything to do with my project?

We already mention in the README that using a forward slash at the beginning of a URL will point to the host root.

The same applies when one uses just / like you do over here: https://github.com/asurbernardo/amperage/blob/ed73659cd459a845625fb40d4c3d1e3a667b8a2a/layouts/partials/header/base.html#L2

@asurcodes
Copy link
Author

@asurcodes asurcodes commented Dec 12, 2019

We already mention in the README that using a forward slash at the beginning of a URL will point to the host root.

Oh, I misread that part, I thought it was a url by it self what caused problems and if they were piped it was fine.

One more thing, how should I approach a pagination demo? The paginator .URL starts with a slash (/page/2/), should I just trim it?

https://github.com/asurbernardo/amperage/blob/master/layouts/partials/page/pagination.html#L4

@digitalcraftsman
Copy link
Member

@digitalcraftsman digitalcraftsman commented Dec 12, 2019

Hello @asurbernardo,

thank you for sharing this theme with the Hugo community 👍

While looking at your theme I noticed that thumbnail (tn.png) in the images folder has the right dimensions. You can find it in the README.

@asurcodes
Copy link
Author

@asurcodes asurcodes commented Dec 12, 2019

While looking at your theme I noticed that thumbnail (tn.png) in the images folder has the right dimensions. You can find it in the README.

Fixed! 👍

@asurcodes
Copy link
Author

@asurcodes asurcodes commented Dec 16, 2019

@onedrawingperday Urls are now fixed for the demo. 👍

I ended up passing an empty string and triming the default .URL variable:

{{ "" | absLangURL }}

and

<a href="{{ trim .Paginator.Prev.URL "/" | absLangURL }}" data-rel="prefetch"><< {{ i18n "previous" }} </a>

It's not the most elegant solution IMO, so if you know a better way please let me know.

@onedrawingperday
Copy link
Contributor

@onedrawingperday onedrawingperday commented Dec 18, 2019

@asurbernardo

No. You do not need to use trim. Have a look at the internal pagination template

@asurcodes
Copy link
Author

@asurcodes asurcodes commented Dec 18, 2019

@onedrawingperday The thing is due to AMP requirements I need to generate absolute URLs on every link, so the internal pagination template doesn't help me because the .URL is relative.

That is why I do a trim + absURL pipe to make it work on the demo.

@onedrawingperday
Copy link
Contributor

@onedrawingperday onedrawingperday commented Dec 22, 2019

@asurbernardo

Ok. Since this is due to AMP requirements trim seems like the way to go.

However I found the time to have a proper look at your theme and you need to do the following:

  • Your theme's thumbnail currently features the theme name and its logo and that looks out of proportion compared to the other listed themes on the Hugo Showcase. Instead the thumbnail needs to feature the layout of the theme demo homepage like you do in the screenshot.

  • Change line 3 of layouts/partials/header/menu.html to:
    {{ $url := trim .URL "/" | absURL }} as you already did for other links so that internal menu links resolve properly (currently they don't)

  • Remove line 4 of layouts/partials/header/menu.html the logic in this check may work when you test your theme locally but it does not work with the Hugo Themes Build Script.

  • To check whether a menu entry points to an external or internal URL change lines 7-8 of layouts/partials/header/menu.html to:

{{ with .Identifier }} target="_blank" rel="nofollow noopener noreferrer"
                {{else}} data-rel="prefetch" {{ end }}>

Currently as you have set up the menu in your theme demo:

  • The menu entries specified in the exampleSite/config have identifiers.
  • The menu entries that point to internal pages are specified in the front matter of the respective content files and as such they do not have identifiers.

You need to make sure to document this feature of your theme in the README so that users know about the detection of internal links for menu entries.

Once you do the above please let me know.

@asurcodes
Copy link
Author

@asurcodes asurcodes commented Jan 2, 2020

@onedrawingperday

I've done the following:

  • Update the thumbnail to feature the actual layout of the theme
  • Update menu links logic based on identifier presence
  • Document menu links creation on README

Thank you so much for taking the time and for the thorough review! 😃

@onedrawingperday
Copy link
Contributor

@onedrawingperday onedrawingperday commented Jan 2, 2020

@asurbernardo

I added your theme to the repo in 6efd432


@digitalcraftsman

This is a blog theme with AMP support.

@digitalcraftsman
Copy link
Member

@digitalcraftsman digitalcraftsman commented Jan 8, 2020

I've promoted your theme on Hugo's official Twitter account.

@asurcodes
Copy link
Author

@asurcodes asurcodes commented Jan 8, 2020

@digitalcraftsman Thank you for the shout-out! I can't wait to get feature requests and new ideas to improve the theme. 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants