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

Update masthead.html #2332

Merged
merged 2 commits into from
Dec 19, 2019
Merged

Update masthead.html #2332

merged 2 commits into from
Dec 19, 2019

Conversation

dmitrypv
Copy link
Contributor

fix image path in masthead for relative url

This is a bug fix.

Summary

Absolute/relative path check {% if site.logo contains "://" %} for logo image must prepend the baseurl in case of relative path.

Context

If you specify 'logo: "/assets/images/logoexample.png' in _config.yml, logoexample.png will not be shown in masthead unless absolute path is used.

fix image path in masthead for relative url
_includes/masthead.html Outdated Show resolved Hide resolved
Co-Authored-By: iBug ♦ <git@ibugone.com>
@mmistakes mmistakes merged commit abf2943 into mmistakes:master Dec 19, 2019
@Geekdude
Copy link

This change breaks the site-logo for jekyll 4.0 with a baseurl.

With this change I get ERROR '/~ayoung48/~ayoung48/assets/images/logo88x88.png' not found. If I undo this change, so that the line is {% capture logo_path %}{{ site.logo }}{% endcapture %} this issue goes away.

@iBug
Copy link
Collaborator

iBug commented Jan 29, 2020

@Geekdude Hi, can you please share the exact value for your site.logo and site.baseurl? I guess you've manually prepended your base URL to the logo URL, which shouldn't have been done in the first place. I would suggest that you adopt your settings because IMO this PR is the correct way to do it.

@Geekdude
Copy link

Geekdude commented Jan 29, 2020

@iBug Hi, here are the logo and baseurl settings from _config.yml.

baseurl : "/~ayoung48"
logo : "/assets/images/logo88x88.png"

The issue could be related to changes made in jekyll 4.0.
From the update notes:

The link and post_url tags no longer need site.baseurl prepended every time they’re used. Those tags now use our relative_url filter to take care of this for you. Existing uses of the prepending pattern will break though! Sorry! 😅

iBug added a commit to iBug/minimal-mistakes that referenced this pull request Jan 29, 2020
There's already a `relative_url` in place, shouldn't stack up another
@mmistakes
Copy link
Owner

mmistakes commented Jan 29, 2020

@Geekdude Don't think it's a Jekyll 4.0 issue. That quote above is in reference to when you use Jekyll's {% link %} and {% post_url %} tags to link to pages... we're not doing that with the logo in the masthead.

I think you may be misusing site.baseurl. Could be the ~ to. Can't say that I've seen that in a path before. Can you confirm what a full path to a page on your site should be?

For example. If I had a domain of https://foo.com and wanted all my Jekyll posts/pages to live in a /blog subfolder I'd set the following in my _config.yml

url: "https://foo.com"
baseurl: "/blog"

Which would give me URLs like https://foo.com/blog/my-awesome-post/

With your settings you'd get https://whateveryourdomainis.com/~ayoung48/my-awesome-post/ which looks incorrect to me.

@Geekdude
Copy link

@mmistakes I host my website on my university's servers. They use ~username for the student pages. My website is hosted at https://web.eecs.utk.edu/~ayoung48/.

@mmistakes
Copy link
Owner

@Geekdude OK. That makes more sense then.

@iBug
Copy link
Collaborator

iBug commented Jan 29, 2020

@Geekdude Thank you very much for your report. I've reproduced the issue locally and can confirm changes in this PR caused your issue. I've fired up #2385 and hopefully it'll fix this.

@Geekdude
Copy link

@iBug Cool, thank you for finding and fixing the issue.

mmistakes pushed a commit that referenced this pull request Jan 29, 2020
* Regression for #2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
kamil-adam pushed a commit to twocolumn/minimal-mistakes that referenced this pull request Apr 1, 2020
* Update masthead.html

fix image path in masthead for relative url

* Update _includes/masthead.html

Co-Authored-By: iBug ♦ <git@ibugone.com>
jesuswasrasta pushed a commit to jesuswasrasta/jesuswasrasta.github.io that referenced this pull request Jul 8, 2020
* Update masthead.html

fix image path in masthead for relative url

* Update _includes/masthead.html

Co-Authored-By: iBug ♦ <git@ibugone.com>
jesuswasrasta pushed a commit to jesuswasrasta/jesuswasrasta.github.io that referenced this pull request Jul 8, 2020
* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
mzaffran pushed a commit to mzaffran/mzaffran.github.io that referenced this pull request Jan 4, 2021
* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
kamil-adam pushed a commit to twocolumn/minimal-mistakes that referenced this pull request Mar 15, 2021
* Update masthead.html

fix image path in masthead for relative url

* Update _includes/masthead.html

Co-Authored-By: iBug ♦ <git@ibugone.com>
kaitokikuchi pushed a commit to kaitokikuchi/kaitokikuchi.github.io that referenced this pull request Sep 4, 2023
* Update masthead.html

fix image path in masthead for relative url

* Update _includes/masthead.html

Co-Authored-By: iBug ♦ <git@ibugone.com>
kaitokikuchi pushed a commit to kaitokikuchi/kaitokikuchi.github.io that referenced this pull request Sep 4, 2023
* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
chukycheese pushed a commit to chukycheese/chukycheese.github.io that referenced this pull request Sep 18, 2023
* Update masthead.html

fix image path in masthead for relative url

* Update _includes/masthead.html

Co-Authored-By: iBug ♦ <git@ibugone.com>
chukycheese pushed a commit to chukycheese/chukycheese.github.io that referenced this pull request Sep 18, 2023
* Regression for mmistakes#2332

There's already a `relative_url` in place, shouldn't stack up another

* Update CHANGELOG and history
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants