InnerSourceCommons / archive.innersourcecommons.org

The old version of the ISC site
Creative Commons Attribution Share Alike 4.0 International
40 stars 29 forks source link

Docker Toolbox issues #100

Closed dellagustin closed 4 years ago

dellagustin commented 4 years ago

This is a follow up of #91

Issues on Docker Toolbox

dellagustin commented 4 years ago

while looking at our code, I started thinking that our issue is not really with Docker Toolbox, but related to the fact we use absolute URLs in many places, instead of relative paths.

i.e.: https://github.com/InnerSourceCommons/innersourcecommons.org/blob/ffdf2e3ba5557c11983cb52c4c862fc2641a8e93/_includes/_head.html#L24

I will have to check what is the best practice for when referring to assets, but rendering pages with absolute links make it very difficult to add reverse proxies and use port forwarding.

dellagustin commented 4 years ago

After reading:

I believe we should be using something like:

<link rel="stylesheet" type="text/css" href={{ "/assets/css/styles_feeling_responsive.css" | relative_url }}> 

instead of

<link rel="stylesheet" type="text/css" href="{{ url }}/assets/css/styles_feeling_responsive.css"> 
dellagustin commented 4 years ago

Bad news, I think we are gonna have to get rid of urlimg, as there is no standard filter for that: https://jekyllrb.com/docs/liquid/filters/

Images src would have to be reworked. Perhaps we should create a custom filter. A possible solution would be something on this line:

To be fair, I have no idea what the urlimg is used for, as I could not find any usage of it that helped me understand if it is actually in use by the website.
I have the impression there is a lot of unused boilerplate code, but it may be just that I don't know a lot of Jekyll.

maxcapraro commented 4 years ago

I think @cewilliams created the initial version of the website. Maybe he has insights about urlimg and whether removing it will make something epxlore :)

dellagustin commented 4 years ago

I just noticed that there are a lot of concatenations of site.url with other stuff, which indicates that absolute path is used throughout the website.

A lot of refactoring will be necessary to bring the website into a state that does not use unnecessary absolute links.

I am thinking about creating a issue exclusively for this topic, and clean it up little by little. Meanwhile, I think I will have to setup my development environment on the host without docker, and have a long term goal to also support Docker Toolbox.

dellagustin commented 4 years ago

~I just found out something rather interesting. Sometimes it is good to trust your instincts... I started removing the usage of absolute path urls, but decided to first test how that looked like with jekyll build, in contrast to jekyll serve, as during my research I read somewhere that there were differences.~

~What I did, was to build it into _site, and simply serve from there using a simple node based http server (http-server .). To my surprise, when you use build, it apparently ignore site.url, and all the links (that I tested) are now relative, and so, the website works well.~

~What that means, is that possibly it would work well if I used a combination build + a webserver on docker. This would be very interesting, since it would be more in line with the script cibuild, as in github pages the content is served from static files, not with jekyll serve.~

~I'll give that a try.~

Update: the difference was not serve vs build, when I did the build for testing I did not pass any --config.

The thing is simple, _config.yml specifies url: '', which makes all links look like they are relative, but if you also use _config_dev.yml, it specifies url: 'http://localhost:4000', which then generates a lot of absolute links that will stop working on Docker Toolbox, because there you have to access the website using the docker machine ip. To allow requests from any host, we also use --host 0.0.0.0, which seems to override the domain in url, causing troubles also in Docker Toolbox.

rrrutledge commented 4 years ago

Thanks for documenting these issues, @dellagustin

dellagustin commented 4 years ago

I managed to solve this issues on #99 , see the PR text for more information. Once #99 is merged we can close this issue.

lenucksi commented 4 years ago

99 got merged so I think we can close this issue.