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

Theme asset files are not overwritten properly (trying to update FontAwesome) #722

Closed
BenjaminJurke opened this Issue Dec 19, 2016 · 15 comments

Comments

Projects
None yet
2 participants
@BenjaminJurke

BenjaminJurke commented Dec 19, 2016

  • This is a question about using the theme.
  • I believe this to be a bug with the theme --- not Jekyll, GitHub Pages or one of the bundled plugins.
  • This is a feature request.
  • I have updated all gems with bundle update.
  • I have tested locally with bundle exec jekyll build.

Environment informations

  • Minimal Mistakes version: 4.1.0
  • github-pages or jekyll gem version: 3.3.1
  • Operating system: Ubuntu 16.10 x64

Expected behavior

I tried to update the FontAwesome 4.6.3 that is bundled within the Minimal Mistakes theme to the most recent version 4.7.0. The local _sass/vendor/font-awesome/ folder is easily updated by replacing the files and a minimal renaming of the main _font-awesome.scss file (adding the leading underscore). However, updating the actual font files in the local assets/fonts/ folder does NOT update the files in the destination output.

Naturally, I would expect my local source files to replace the default theme files in the destination folder.

I also tried adding the assets/fonts/ folder to the keep_files in the _config.yml in order to preserve a manual update of the relevant font files after the Jekyll run, but even then the theme files are used and overwrite my replacement,

Steps to reproduce the behavior

See above. However, I am not sure if this a theme error, a general Jekyll error or entirely my own stupidity...

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Dec 19, 2016

Owner

Smells like a Jekyll error to me. Bundled assets folder support is fairly new so it's possible there are bugs and the local versions are not overriding the bundled ones.

If I get time this week I'll test it and see if I notice the same issue you're having.

In the meantime can you verify its not a browser caching issue. Can you diff the updated files inassets/fonts against _site/assets/fonts after a build just to double check. If they're the same then that tells me your browser is caching the old assets. If the'yre different then it's likely a bug with Jekyll as the theme has nothing to do with processing files.

Owner

mmistakes commented Dec 19, 2016

Smells like a Jekyll error to me. Bundled assets folder support is fairly new so it's possible there are bugs and the local versions are not overriding the bundled ones.

If I get time this week I'll test it and see if I notice the same issue you're having.

In the meantime can you verify its not a browser caching issue. Can you diff the updated files inassets/fonts against _site/assets/fonts after a build just to double check. If they're the same then that tells me your browser is caching the old assets. If the'yre different then it's likely a bug with Jekyll as the theme has nothing to do with processing files.

@BenjaminJurke

This comment has been minimized.

Show comment
Hide comment
@BenjaminJurke

BenjaminJurke Dec 20, 2016

This is definitely not a browser caching issue. I cleared my browser cache multiple times and cleared the jekyll caching (jekyll clean). Ultimately, I noticed that the files in the _site/assets/fonts folder are of different (smaller = older) size compared to my assets/fonts folder with the new ones. This is clearly visible at the local file level, with no browsers involved at all. There is no difference between jekyll build or jekyll serve.

It is noteworthy that additional files (i.e. files that are not overwriting asset files from the theme) are processed correctly, i.e. they are properly added to the _site/assets/... folder as expected. The issue only seems to hit static asset files that are supposed to overwrite files from the theme gem. As mentioned, the keep_files directive does not work as one would expect as well.

For the moment I would be happy if you could simply update the Font Awesome package in the next gem theme version, but this is obviously just a workaround and not the core of the issue.

In any case: congratulation and thanks for this overall awesome Jekyll theme! :-)

BenjaminJurke commented Dec 20, 2016

This is definitely not a browser caching issue. I cleared my browser cache multiple times and cleared the jekyll caching (jekyll clean). Ultimately, I noticed that the files in the _site/assets/fonts folder are of different (smaller = older) size compared to my assets/fonts folder with the new ones. This is clearly visible at the local file level, with no browsers involved at all. There is no difference between jekyll build or jekyll serve.

It is noteworthy that additional files (i.e. files that are not overwriting asset files from the theme) are processed correctly, i.e. they are properly added to the _site/assets/... folder as expected. The issue only seems to hit static asset files that are supposed to overwrite files from the theme gem. As mentioned, the keep_files directive does not work as one would expect as well.

For the moment I would be happy if you could simply update the Font Awesome package in the next gem theme version, but this is obviously just a workaround and not the core of the issue.

In any case: congratulation and thanks for this overall awesome Jekyll theme! :-)

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Dec 20, 2016

Owner

Ok. I'll take a look and see if I can reproduce. Will have to file an issue upstream since it seems to be Jekyll related.

I don't think setting keep_files will make any difference. Pretty sure there is a bug in how the assets folder is handled with respect to theme gems. As I alluded to above the feature is relatively new and likely not widely used so I wouldn't be surprised that it has a few kinks left to iron out.

Owner

mmistakes commented Dec 20, 2016

Ok. I'll take a look and see if I can reproduce. Will have to file an issue upstream since it seems to be Jekyll related.

I don't think setting keep_files will make any difference. Pretty sure there is a bug in how the assets folder is handled with respect to theme gems. As I alluded to above the feature is relatively new and likely not widely used so I wouldn't be surprised that it has a few kinks left to iron out.

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Dec 20, 2016

Owner

PS I opened an issue to update Font Awesome #723. Not sure when I'll get to it but if you'd like to submit a pull request it should be a relatively easy one to knock out.

Owner

mmistakes commented Dec 20, 2016

PS I opened an issue to update Font Awesome #723. Not sure when I'll get to it but if you'd like to submit a pull request it should be a relatively easy one to knock out.

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Dec 22, 2016

Owner

Just got around to testing this and I was able to reproduce it. Looks like there might be a bug with Jekyll and how it handles files in /assets/ which is out of my hands to correct.

Best I can tell files with YAML Front Matter override fine ie: assets/css/main.scss, but when I try to override any of the files in assets/fonts or assets/js they spit out the theme gem versions.

When I get a chance I'll file an issue on jekyll/jekyll and see where that leads.

Owner

mmistakes commented Dec 22, 2016

Just got around to testing this and I was able to reproduce it. Looks like there might be a bug with Jekyll and how it handles files in /assets/ which is out of my hands to correct.

Best I can tell files with YAML Front Matter override fine ie: assets/css/main.scss, but when I try to override any of the files in assets/fonts or assets/js they spit out the theme gem versions.

When I get a chance I'll file an issue on jekyll/jekyll and see where that leads.

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes
Owner

mmistakes commented Dec 22, 2016

Bug filed jekyll/jekyll#5676

@BenjaminJurke

This comment has been minimized.

Show comment
Hide comment
@BenjaminJurke

BenjaminJurke Dec 22, 2016

Great, thanks for your effort.

Like you mentioned, the override seems to work for files, that are actually being processed by Jekyll. The font files, as far as I can tell, are essentially just being copied over to the output folder. I guess, there may be very small bug somewhere in Jekyll for this particular case (overriding static unprocessed files from the theme). Additional files in the asset folder are handled correctly.

BenjaminJurke commented Dec 22, 2016

Great, thanks for your effort.

Like you mentioned, the override seems to work for files, that are actually being processed by Jekyll. The font files, as far as I can tell, are essentially just being copied over to the output folder. I guess, there may be very small bug somewhere in Jekyll for this particular case (overriding static unprocessed files from the theme). Additional files in the asset folder are handled correctly.

@BenjaminJurke

This comment has been minimized.

Show comment
Hide comment
@BenjaminJurke

BenjaminJurke Dec 22, 2016

Just saw the theme update to 4.1.1 - thanks for the super-fast update! Thumbs up!

(Yes, I know, this does not fix the underlying issue, but at the moment solves my specific problem :) )

BenjaminJurke commented Dec 22, 2016

Just saw the theme update to 4.1.1 - thanks for the super-fast update! Thumbs up!

(Yes, I know, this does not fix the underlying issue, but at the moment solves my specific problem :) )

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes
Owner

mmistakes commented Dec 23, 2016

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Jan 4, 2017

Owner

additional reference issue: jekyll/jekyll#5704

Owner

mmistakes commented Jan 4, 2017

additional reference issue: jekyll/jekyll#5704

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Feb 14, 2017

Owner

Closing as this is an upstream issue.

Owner

mmistakes commented Feb 14, 2017

Closing as this is an upstream issue.

@mmistakes mmistakes closed this Feb 14, 2017

@mmistakes mmistakes referenced this issue Feb 26, 2017

Closed

Javascript Changes Not being applied #874

0 of 2 tasks complete

mmistakes added a commit that referenced this issue Feb 28, 2017

Add banner text to main.min.js
- Add `---` YAML Front Matter as a workaround to allow the theme gem's version to be overridden locally.
- Adjust page scope in Front Matter default to avoid adding a layout to `main.min.js`
- Fixes #874, Fixes #722

mmistakes added a commit that referenced this issue Feb 28, 2017

Add banner text to main.min.js (#880)
- Add `---` YAML Front Matter as a workaround to allow the theme gem's version to be overridden locally.
- Adjust page scope in Front Matter default to avoid adding a layout to `main.min.js`
- Fixes #874, Fixes #722
@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Feb 28, 2017

Owner

@BenjaminJurke FYI I found a workaround until this is fixed upstream. If I add YAML Front Matter to main.min.js it can be overridden. In the next release of MM I'll push it out. All you'll have to do is add:

---
---

to the top of your modified /assets/js/main.min.js file and it should override the theme gem's version.

Owner

mmistakes commented Feb 28, 2017

@BenjaminJurke FYI I found a workaround until this is fixed upstream. If I add YAML Front Matter to main.min.js it can be overridden. In the next release of MM I'll push it out. All you'll have to do is add:

---
---

to the top of your modified /assets/js/main.min.js file and it should override the theme gem's version.

@BenjaminJurke

This comment has been minimized.

Show comment
Hide comment
@BenjaminJurke

BenjaminJurke Feb 28, 2017

Well, since you updated FontAwesome in the gem my issue has been resolved.

However, I recently stumbled over something that may be related: I recently used your theme for a German language website, i.e. I am using localizations for the first time. But it appears that the translations in the ui-text.yml file (which is part of the theme and should be in the gem) are only used if the file is locally present. It is not a big issue, since I can simply copy the file from the repository into the _data directory and everything works fine. However, one would naively expect this to work without the file present, i.e. defaulting to the file in the gem.

This appears to be (at least somewhat) related, because once again processing of local and gem-default files is handled differently (unexpected), in this particular case the local variants appear to have preference. Jekyll and theme are at the most recent version - I basically run bundle update whenever I do some work on my site.

Like I said, not a real issue, but maybe something worth looking into it.

BenjaminJurke commented Feb 28, 2017

Well, since you updated FontAwesome in the gem my issue has been resolved.

However, I recently stumbled over something that may be related: I recently used your theme for a German language website, i.e. I am using localizations for the first time. But it appears that the translations in the ui-text.yml file (which is part of the theme and should be in the gem) are only used if the file is locally present. It is not a big issue, since I can simply copy the file from the repository into the _data directory and everything works fine. However, one would naively expect this to work without the file present, i.e. defaulting to the file in the gem.

This appears to be (at least somewhat) related, because once again processing of local and gem-default files is handled differently (unexpected), in this particular case the local variants appear to have preference. Jekyll and theme are at the most recent version - I basically run bundle update whenever I do some work on my site.

Like I said, not a real issue, but maybe something worth looking into it.

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Feb 28, 2017

Owner

@BenjaminJurke That's another issue. Jekyll only reads _layouts, _includes, _sass and /assets that are included in a theme gem. _config.yml and _data files whether they're included in the gem or not don't get pulled in by Jekyll.

There's been talk about bringing them in (which I'm in favor of) since it reduces the friction of starting with a theme gem. Right now you have to physically copy over the files from the MM repo.

Ref: jekyll/jekyll#5470

Owner

mmistakes commented Feb 28, 2017

@BenjaminJurke That's another issue. Jekyll only reads _layouts, _includes, _sass and /assets that are included in a theme gem. _config.yml and _data files whether they're included in the gem or not don't get pulled in by Jekyll.

There's been talk about bringing them in (which I'm in favor of) since it reduces the friction of starting with a theme gem. Right now you have to physically copy over the files from the MM repo.

Ref: jekyll/jekyll#5470

@BenjaminJurke

This comment has been minimized.

Show comment
Hide comment
@BenjaminJurke

BenjaminJurke Feb 28, 2017

OK, I see - was not aware of this.

BenjaminJurke commented Feb 28, 2017

OK, I see - was not aware of this.

antonizoon pushed a commit to antonizoon/antonizoon.github.io that referenced this issue Jul 27, 2018

Add banner text to main.min.js (mmistakes#880)
- Add `---` YAML Front Matter as a workaround to allow the theme gem's version to be overridden locally.
- Adjust page scope in Front Matter default to avoid adding a layout to `main.min.js`
- Fixes mmistakes#874, Fixes mmistakes#722
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment