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

Address reading non-binary static files in themes #5918

Merged
merged 3 commits into from Jun 14, 2017

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented Mar 1, 2017

This is related to #5676. I may have found a solution, but not sure if the approach is correct.
I'm pushing commits separately to trigger unique ci builds for them.

/cc @parkr

--

Fixes #5676

@ashmaroli
Copy link
Member Author

Can this be released with v3.4.1 too?

@parkr
Copy link
Member

parkr commented Mar 2, 2017

Can this be released with v3.4.1 too?

No, I don't suspect we'll do anything other than the PR I have opened. master is pointed to a v3.5 and I'd like to just keep going with that. This feels like a big enough change to be in a minor bump.

@ashmaroli
Copy link
Member Author

label: themes, fix

@ashmaroli
Copy link
Member Author

fixes #5676

@zyv
Copy link
Contributor

zyv commented Mar 27, 2017

Any news on that one? Just got hit by this bug :-( Of course, I could fork jekyll and apply the patch locally, then move our sites to use our custom fork, but that'd be a pain :-/

@ashmaroli
Copy link
Member Author

but that'd be a pain

@zyv Or you could just use this PR's branch on my fork of Jekyll, till this is merged and released.

Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -29,7 +29,7 @@ def read_theme_asset(path)
Jekyll::Page.new(site, base, dir, name)
else
append_unless_exists site.static_files,
Jekyll::StaticFile.new(site, base, dir, name)
Jekyll::StaticFile.new(site, base, "/#{dir}", name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@parkr
A Jekyll::StaticFile path in source_dir : /assets/img/logo.png

A Jekyll::StaticFile path in theme_dir without this PR : assets/images/hero.jpg
A Jekyll::StaticFile path in theme_dir with this PR : /assets/images/hero.jpg

The above minor discrepancy is what causes #5676

@parkr parkr added the pending-feedback We are waiting for more info. label Mar 31, 2017
@ashmaroli
Copy link
Member Author

StaticFile path from the source_dir has a trailing slash while the path to a StaticFile from a theme-gem doesn't. This is what caused the mentioned issue.

@jekyllbot jekyllbot removed the pending-feedback We are waiting for more info. label Mar 31, 2017
@codedbypm
Copy link

+1 would love to see this in the next jekyll release.

@ashmaroli ashmaroli mentioned this pull request May 12, 2017
@DirtyF DirtyF requested a review from pathawks May 13, 2017 00:36
Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love the interpolation of "/#{dir}" because it smells like a potential security issue to me, but I won't let a hunch block this PR.

@parkr
Copy link
Member

parkr commented Jun 14, 2017

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit fcde834 into jekyll:master Jun 14, 2017
jekyllbot added a commit that referenced this pull request Jun 14, 2017
@ashmaroli
Copy link
Member Author

I don't love the interpolation of "/#{dir}" because it smells like a potential security issue to me

@parkr Do you want me to pass the variable through an ensure_leading_slash method?

@DirtyF DirtyF added this to the 3.5 milestone Jun 18, 2017
@ashmaroli ashmaroli deleted the static-asset-reader branch August 6, 2017 05:34
mmistakes added a commit to mmistakes/minimal-mistakes that referenced this pull request Aug 6, 2017
BoWuGit pushed a commit to BoWuGit/bowugit.github.io that referenced this pull request Aug 17, 2017
amweiss pushed a commit to amweiss/amweiss.github.io that referenced this pull request Aug 22, 2017
anouri pushed a commit to IBMStreams/streamsx.jdbc that referenced this pull request Oct 19, 2017
* Bump version to 4.2.0

* Add https prefix to google-universal embed code. (#772)

* Add https prefix to google-universal embed code. (#772)

* Pagination now works for index.html files in subfolders. Links to next page consider baseurl now. (#764)

* Links to next page consider baseurl now.

* paginate_show_page_num controls whether page number gets output.

* Pagination now works for index.html files in subfolders. Links to next page consider baseurl now. (#764)

* Document moving paginated home page to an alternate location

* Update CHANGELOG and history

* Bump version to 4.2.1

* Fix bug - include section for Discourse embedded comments (#825)

* Improve the documentation for novice users (Tags+Categories and Pagination) (#826)

* Enhanced the documentation with some tips for the tags, categories and paginator

* Enhanced the documentation with some tips for the tags, categories and paginator

* Update 05-configuration.md

* Update 10-layouts.md

* Added Korean data in ui-text.yml

Added Korean translations.

* Add long `title` for testing purposes

* Update GreedyNav.js

* Replace with relative sizes

* Update GreedyNav.js

* Update GreedyNav to use flexbox instead of `display: table`
- ref #836

* Run through Autoprefixer

* Add flexbox fixes for < IE9 to make the greedy nav button visible

* Add Russian localized UI text

* Add Russian localized UI text to test and documentation site
- Close #815

* Add Korean localized UI text

* Adjust `author__urls` shadow to match navigation `box-shadow`

* Add Traditional Chinese UI text.

* Add zh-TW UI text to /docs and /test

* Added a few clarifying comments

YAML references are not always obvious, and ensuring people know to remove your Google IDs seemed a good idea to comment on.  Thanks for minimal mistakes!

* meta tag "author" added, for site author the field name is used

* Update README

* Update theme files in /docs

* Add theme meta info to _layouts/default.html

* Add theme meta info

* Update CHANGELOG and history

* Bump version to 4.2.2

* Update theme meta info

* New comment. (#863)

* Escape Liquid

* Escape Liquid

* Change default locale to `en_US` to appease OpenGraph debugger.
- Fixes #843

* 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

* Update CHANGELOG and history

* Bump version to 4.3.0

* Remove buggy conditional around related posts header (#901)

This removes a buggy conditional that checks if `related_label` is
available in translations before displaying the header for related
posts but this header already handle missing translation by using a
default string.

With this fix the header is displayed even if the translation for this
label isn't available.

Fix #900

* Update CHANGELOG

* Bump version to 4.3.1

* Bump version to 4.3.1

* Bump version to 4.3.1

* Removed extra word in comment (#911)

* Typo fix (#915)

* Bump version to 4.3.1

* Update CHANGELOG and history

* Update contributing info

* Update year

* Improve print styles (#919)

* Fix image caption border-radius

* Fix image caption border-radius

* Improve print stylesheet
- Close #909

* Update CHANGELOG and history

* Add Lithuanian language/locale (#924)

* Add Lithuanian language/locale (#924)

* Update CHANGELOG and history

* Replace modified with last_modified_at (#930)

Fixes #929

* Add ProTip about installing unreleased version on `master`

* Update CHANGELOG and history

* Fix `.masthead` and `.page__footer` overlapping full screen video elements.
- Close #933

* Add closing `}`

* Fixed link errors in docs (#946)

* Move SCSS partials to `/_sass/minimal-mistakes` for easier CSS customization

* Release 💎 4.4.0

* Update history

* fix(includes/video): use https always (#945)

closes #944

* Update ui-text.yml (#958)

* Sync `/docs` with root changes

* Update CHANGELOG and history

* New comment. (#967)

* New comment. (#968)

* Add link to comment

* Transition “hamburger” navicon on click to “X” navicon by adding `.close` to button
- Close 969

* Release 💎 4.4.1

* Bump version to 4.4.1

* Toggle close button on `mouseleave`
- Close #975

* Update 08-ui-text.md (#972)

* Update 08-ui-text.md

* Update 08-ui-text.md

* Update CHANGELOG and history to reflect #972

* Add Swedish language (#996)

Translated this for my page today https://github.com/ojn/www.netkom.se/blob/master/_data/ui-text.yml
I hope there are no spelling mistakes, since one should strive for "minimal mistakes"... :-)

* Add Swedish language

* Add new brand variable: $bitbucket-color (#1009)

* Update CHANGELOG and history

* Fix broken Kramdown TOC link

* Fix "Greek" ~> "Swedish" typo

* Removing extraneous a and li tags (#1038)

Removing duplicate <a> and <li> closing tags in paginator include.

* Remove extraneous `</a>` and `</li>` tags

* New comment. (#1064)

* Update comment-1497284119888.yml

* New comment. (#1065)

* Fixed MD link to docs in the 'Helpers' page. (#1066)

* Fix broken link to example _config.yml file (#1069)

* Fix typo

* HTTPS some external links (#1083)

HTTPS some external links

* Google+ page comments updated.

* Fix indentation and remove comments

* Fix indentation and formatting

* Update Google+ page comments for /docs

* Update CHANGELOG and history

* Fixes and updates for the Greek language (#1054)

I have translated some UI text that hadn’t been translated by @Stelios3g. And I have also changed some translated entries to look and sound more natural for general uses (eg ‘Recent posts’ was translated as ‘Latest Announcements’ before). Finally, I have added ‘RSS’ next to ‘Feed’, because 1) there is no translation (or something that sounds natural) in Greek I’m afraid, 2) the term ‘RSS Feed’ is widely used in Greek websites.

* Fixes and updates for the Greek language

* Update CHANGELOG and history

* Add Dutch language (#1081)

* Add mention of Dutch localized UI text strings to theme documentation

* add gitlab to author profile (#1050)

* add gitlab to author profile

* add gitlab to footer

* Fix alignment of :

* Update Update CHANGELOG and history

* Fixed site variables (#1063)

* Fix variable names

* Improve documentation for grid view

* Adjust nav list `.active` state to accommodate text that spans multiple lines.

* Add indonesian locale (#1101)

* Add Indonesian locale text strings

* Update CHANGELOG and history

* Add $navicon-link-color-hover 1/2 (#1089)

Since the orig color-scheme doesn't always represent the nav/site colors

*  Add $navicon-link-color-hover 2/2 (#1088)

Since the orig color-scheme doesn't always represent the nav/site colors

* Add variable for navicon link color

* Replace Gist Liquid tag with embed `script`

* Change `gems` key in config to `plugins`

* Add tzinfo.data gem for Windows to appease timezone error

* Update CHANGELOG and history

* Bump dependencies

* Release 4.4.2

* Bump version to 4.4.2

* Bump version to 4.4.2

* Add vertical scrollbar to sidebars that extend outside the viewport

Viewing overflowing sidebar content requires scrolling the entire page which is annoying and causes the reader to lose their place.

Use CSS `calc` to approximate height of the sidebar (`100vh` - height of the masthead) and apply `overflow-y: auto` to add vertical scrollbars when needed.

Fixes #706

* New comment. (#1116)

* New comment. (#1117)

* New comment. (#1119)

* New comment. (#1120)

* New comment. (#1124)

* New comment. (#1125)

* Use a plugin to read data files from gem

  - jekyll-data v1.x
  - jekyll 3.5 automatically requires a theme's runtime deps
  - jekyll v3.5.0 had bugs. Hence use v3.5.1 and beyond

* New comment. (#1133)

* Upgrade to Susy 3 and replace grid mixins with new `span` and `gutter` functions

Most of Susy's mixins have been deprecated, `@include container()`, `@include full()`, `@include span()`, `@include prefix()`, `@include suffix()`, `@include gallery()`, etc.

Fixes #1114

* Spanish text for comments (#1118)

* Add missing Spanish UI text strings

* Update CHANGELOG

* Add link to PR

* Refactor intro animations into a separate SASS variable (#1147)

* Add `$intro-transition` variable

* Fix space alignment

* Revert `plugins;` back to `gems:` until github/github-pages gem updates to newer Jekyll
- Plugins won't activate if using the new `plugins` key with Jekyll < 3.5

* Update CHANGELOG and history

* Correct name of gem to that of the theme (#1149)

* Include documentation on disabling animations (#1150)

* Update CHANGELOG and history

* Fix typo

* Change reference to "Basically Basic" back to "Minimal Mistakes"

* Update CHANGELOG and history

* Link to PR on GitHub

* Fix disappearing author sidebar links
- Close #1136

* Update CHANGELOG and history

* Header alt tags (#1138)

* Added support for an alt tag for the header image

This allows using page.header.image_description as the alt tag. It
will still default to site.title if unset.

* Added documentation

* DRY up handling of `image_description` for alternative text

* Update CHANGELOG and history

* Update documentation (#1151)

* Update Quickstart documentation

* update Installation documentation

* Update documentation for Overriding Defaults

* Release 💎 4.5.0

* support vietnamese (#1159)

* Add mention of Vietnamese to UI text docs and update CHANGELOG/history

* Remove blank YAML Front Matter from JavaScript banner
- No longer needed ref: jekyll/jekyll#5918

Ref: mmistakes/minimal-mistakes#1158

* Update 05-configuration.md (#1164)

* Update CHANGELOG and history

* Improve `page` and `archive` layouts (#1166)

* Fix collapsed white-space above pagination links

* Improve `page` and `archive` layout
- Center main content on page
- Harmonize sidebar columns to be equal widths

Fixes #1155

* Update CHANGELOG and history

* Position and align right sidebar with the top of the main content

* Offset right sidebar at `$large` viewport

* Add TOC bottom include test post

* Add right sidebar styling edits to documentation site

* Add TOC bottom include test post

* Make minor formatting changes

* Remove outside and right borders in `table`s

* Increase font-size of code blocks.

* Reduce indent of nested "table of contents" links

* Update `website_label`

* Rename gems key to `plugins`
- `gems` key in `_config.yml`  was deprecated in Jekyll 3.5 and changed to `plugins`

ref: jekyll/jekyll#5130

* Reword sentences

* Roll back `gems:` to `plugins:` change in `_config.yml`
- GitHub Pages still doesn't seem to load safe plugins when using `plugins` key

* Fix positioning of sidebar table of contents when using `layout: splash`

Close #1169

* Adjust width of `.sidebar` to match `.sidebar__right`
- Use `$right-sidebar-width-narrow` and `$right-sidebar-width` Sass variables to determine width of sidebar instead of Susy `span` function

* Fix "follow" links `z-index` order to avoid overlapping issues

Close #1167

* Replace `gems` key with `plugins: `

* Extend grid view to the right to better fill the page
- Use a negative right margin to pull `.grid__wrapper` into the dead space left for a sidebar.
- Add sample documents to "portfolio" collection to test grid view
- Update CHANGELOG and history

* Update 16-stylesheets.md (#1170)

Typo error - "want" to "what"

* Update CHANGELOG and history

* Update GEMFILE according description to use theme on github-pages

* First working version

* Remove obsolete files

* remove therubyracer gem

* gem file changed

* delete obsolete files

* synch to original

* *

* Create README.md

* Remove pictures

* remove pictures

* home.md changed excerpt w/o pictures

* Add neutral picture in home page

* Cleaning home page

* Formatting home page

* editme ... first try

* Add editme on the overview pages

* Cleaning

* Cleaning _config.yml

* set x_large variable to 1600

* Update _config.yml editme_path to full url

* Update _includes/editme to support full path from _config.yml

* new include file buttons

* Update _config.yml add github_base_url

* Update home.md add button row

* Change Main navigation from Docu to Documentation

* _config.yml: add page defaults for excerpt, cta_url

* _config.yml add missing slash in spldoc_path

* _config.yml: new defaults are properties of header

      header.cta_url , header.cta_label

* Update _config.yml

* Update home.md

* home.md replace buttons with links

* Update home.md

* Update home.md

* Add possible link in parallel to button

* home.md add link_url and link_label

* Update feature_row

* Update feature_row

* Update feature_row

* Update feature_row

* Switch off the complete Social section in footer.html

* clean gh-pages

* iopages update

* add how to connect pages
jpogorzelski pushed a commit to jpogorzelski/jpogorzelski.github.io.old that referenced this pull request Feb 11, 2019
kkunapuli pushed a commit to kkunapuli/kkunapuli.github.io that referenced this pull request May 30, 2019
@jekyll jekyll locked and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theme assets do not respect static files within the site
6 participants