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

Page menu URLs vs permalinkable output formats #5849

Closed
akshaybabloo opened this issue Apr 13, 2019 · 10 comments

Comments

Projects
None yet
2 participants
@akshaybabloo
Copy link
Contributor

commented Apr 13, 2019

After setting section = ["HTML", "RSS", "AMP"] in config.tomal all the hyperlinks in the page links to http://domain.com/amp/.... This was working fine in 0.54.

@akshaybabloo akshaybabloo changed the title Multiple output types defaults to /amp/ links Multiple output types defaults to /amp/ links in 0.55 Apr 13, 2019

@bep

This comment has been minimized.

Copy link
Member

commented Apr 13, 2019

all the hyperlinks in the page links to

What do you mean by ALL?

Note that we added a Permalinkable config option on the Output Format -- this defaults to true for HTML and AMP. You can turn that off, but you need to add a output format config section.

@akshaybabloo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2019

Any links that have been generated by Hugo that links to other pages in the same domain, not to the external domain. Permalinkable is not the problem I guess.

Does adding AMP to sections turn all non-AMP links to AMP? as in localhost:1313/blog/welcome to localhost:1313/amp/blog/welcome in a page with URL localhost:1313/blog? This was not the case in 0.54.

@bep

This comment has been minimized.

Copy link
Member

commented Apr 14, 2019

Does adding AMP to sections turn all non-AMP links to AMP? as in localhost:1313/blog/welcome to localhost:1313/amp/blog/welcome in a page with URL localhost:1313/blog? This was not the case in 0.54.

No.

I will double check the logic to make sure that we only link from AMP => AMP, but when you now do .RelPermalink on a page when rendering the AMP variant, it will link to the AMP variant.

If this is not what you want, you can turn it off. See above.

@bep bep changed the title Multiple output types defaults to /amp/ links in 0.55 Check AMP vs Permalinkable Apr 14, 2019

@bep bep added this to the v0.55.2 milestone Apr 14, 2019

@akshaybabloo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

No.

But it's happening to me for some reason.

I am not using . RelPermalink BTW I am using .Permalink. Are both same?

@bep

This comment has been minimized.

Copy link
Member

commented Apr 16, 2019

Are both same?

They are conceptually the same, yes.

bep added a commit that referenced this issue Apr 17, 2019

@bep

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

@akshaybabloo I have added some more tests for this -- and it behaves exactly as expected.

@bep bep modified the milestones: v0.55.2, v0.56 Apr 17, 2019

@akshaybabloo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

OK, I am not sure what the problem is then:

The code is here -> https://github.com/akshaybabloo/gollahalli.com

The output options I have in my config.toml is:

[outputs]
  home = ["HTML", "RSS", "AMP", "searchindex"]
  page = ["HTML", "RSS", "AMP"]
  section = ["HTML", "RSS"]

Now, I have removed section from it but I still get amp URL in my nav bar, the code for that is:

https://github.com/akshaybabloo/gollahalli.com/blob/8d31bbc560fd7b0cd7e236e05b9d748194a68e08/themes/Spark/layouts/index.html#L20-L24

        <ul class="uk-subnav uk-subnav-divider uk-flex-center uk-visible@m gol-subnav">
            {{ range .Site.Menus.main }}
                <li><a href="{{ .URL }}">{{ .Name }}</a></li>
            {{ end }}
        </ul>

The about and projects has amp links I have no idea why.

@bep

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

OK, the page menus. I understand now. I will fix this, eventually, but you can get back the old behaviour by adding a AMP definition with permalinkable turned off:

https://gohugo.io/templates/output-formats#readout

@bep bep added Bug and removed NeedsInvestigation labels Apr 18, 2019

@bep bep changed the title Check AMP vs Permalinkable Page menu URLs vs permalinkable output formats Apr 18, 2019

@bep bep modified the milestones: v0.56, v0.55.3 Apr 18, 2019

@akshaybabloo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

you mean by adding permalinkable = false in the config.toml right?

@bep

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

Something llike this:

[outputFormats]
[outputFormats.AMP]
permalinkable = false

bep added a commit to bep/hugo that referenced this issue Apr 19, 2019

Fix menu URL when multiple permalinkable output formats
In Hugo `0.55` we introduced the `permalinkable` config attribute on Output Format, default enabled for `AMP` and `HTML`.

This meant that a Page could have different `RelPermalink` and `Permalink` depending on the rendering format.

The menu `URL` did not reflect that fact.

Fixes gohugoio#5849

@bep bep closed this in #5878 Apr 19, 2019

bep added a commit that referenced this issue Apr 19, 2019

Fix menu URL when multiple permalinkable output formats
In Hugo `0.55` we introduced the `permalinkable` config attribute on Output Format, default enabled for `AMP` and `HTML`.

This meant that a Page could have different `RelPermalink` and `Permalink` depending on the rendering format.

The menu `URL` did not reflect that fact.

Fixes #5849
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.