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

Generate HTML code and Cloudinary URLs only for "production" environment? #20

Closed
nhoizey opened this Issue Sep 5, 2016 · 9 comments

Comments

5 participants
@nhoizey
Owner

nhoizey commented Sep 5, 2016

This is somehow a poll to know if people would find it nice to let the plugin generate responsive HTML with Cloudinary URL only for the production environment, and use a plain old <img> with local source for the development environment.

If you're not aware of this feature, read Environments in Jekyll (aka JEKYLL_ENV)

This would help develop with Jekyll's serve command while offline, and only generate production-ready HTML code with the build command.

It could be off by default (always generating the production code, as currently) and activated with a setting in _config.yml.

Any thought?

@eeeps

This comment has been minimized.

Show comment
Hide comment
@eeeps

eeeps Sep 14, 2016

This sounds like a good option to have for doing offline work.

One big caveat - if the appearance of the image relies on Cloudinary’s transformations (which is possible with resizing depending on the page’s CSS, and unavoidable if you ever integrate smart cropping), jekyll build will produce something that looks different than jekyll serve, which seems bad.

eeeps commented Sep 14, 2016

This sounds like a good option to have for doing offline work.

One big caveat - if the appearance of the image relies on Cloudinary’s transformations (which is possible with resizing depending on the page’s CSS, and unavoidable if you ever integrate smart cropping), jekyll build will produce something that looks different than jekyll serve, which seems bad.

@nhoizey

This comment has been minimized.

Show comment
Hide comment
@nhoizey

nhoizey Mar 15, 2017

Owner

@eeeps sorry, I didn't see your answer back in september, and didn't work at all on the plugin for months.

You're right we might miss some features and have different results, but I know I need this feature because I'm often offline.

Using a setting in _config.yml to activate this feature means there's an opportunity to explain the differences in the documentation, at least.

Owner

nhoizey commented Mar 15, 2017

@eeeps sorry, I didn't see your answer back in september, and didn't work at all on the plugin for months.

You're right we might miss some features and have different results, but I know I need this feature because I'm often offline.

Using a setting in _config.yml to activate this feature means there's an opportunity to explain the differences in the documentation, at least.

@alexandrediasdasilva

This comment has been minimized.

Show comment
Hide comment
@alexandrediasdasilva

alexandrediasdasilva Oct 15, 2017

That would be great indeed!

After struggling and failing to get jekyll-assets to work for image optimization on my Jekyll website, I've just stumbled on your plugin and it's working smoothly on my production environment - thanks for the work you put in. Now the thing is, I'd like to be able to preview the images on my staging environment before I deploy changes to production.

Currently, my Liquid tags are replaced with the following when building my website on staging:

<img src="https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_1200/http://0.0.0.0:8080/path/to/img.jpg" srcset="https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_320/http://0.0.0.0:8080/path/to/img.jpg 320w, https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_540/http://0.0.0.0:8080/path/to/img.jpg 540w, https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_760/http://0.0.0.0:8080/path/to/img.jpg 760w, https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_980/http://0.0.0.0:8080/path/to/img.jpg 980w, https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_1200/http://0.0.0.0:8080/path/to/img.jpg 1200w" sizes="100vw" alt="up7" width="1500" height="716">

Obviously, there's no way for Cloudinary to access my local project through http://0.0.0.0:8080, so using a plain old <img> with local source for the development environment would make sense in that case.

However I was wondering, would there be a workaround to change the site.url that is used by default by jekyll-cloudinary? I'm not even sure that it's the site.url variable you're using to build the dynamic Cloudinary URL, since I tried to override it in my config_dev.yml file, but to no avail.

That would allow me to preview the images on staging by first pushing the new images to my production server (no one would be able to see them yet, unless they figure out the direct path), and then using the cloudinary tag just like in production.

That might feel a bit hackish, I'd be curious to know if there's any other way to do that. Would greatly appreciate your insight on the topic!

alexandrediasdasilva commented Oct 15, 2017

That would be great indeed!

After struggling and failing to get jekyll-assets to work for image optimization on my Jekyll website, I've just stumbled on your plugin and it's working smoothly on my production environment - thanks for the work you put in. Now the thing is, I'd like to be able to preview the images on my staging environment before I deploy changes to production.

Currently, my Liquid tags are replaced with the following when building my website on staging:

<img src="https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_1200/http://0.0.0.0:8080/path/to/img.jpg" srcset="https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_320/http://0.0.0.0:8080/path/to/img.jpg 320w, https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_540/http://0.0.0.0:8080/path/to/img.jpg 540w, https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_760/http://0.0.0.0:8080/path/to/img.jpg 760w, https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_980/http://0.0.0.0:8080/path/to/img.jpg 980w, https://res.cloudinary.com/cloudname/image/fetch/c_limit,f_auto,q_auto,w_1200/http://0.0.0.0:8080/path/to/img.jpg 1200w" sizes="100vw" alt="up7" width="1500" height="716">

Obviously, there's no way for Cloudinary to access my local project through http://0.0.0.0:8080, so using a plain old <img> with local source for the development environment would make sense in that case.

However I was wondering, would there be a workaround to change the site.url that is used by default by jekyll-cloudinary? I'm not even sure that it's the site.url variable you're using to build the dynamic Cloudinary URL, since I tried to override it in my config_dev.yml file, but to no avail.

That would allow me to preview the images on staging by first pushing the new images to my production server (no one would be able to see them yet, unless they figure out the direct path), and then using the cloudinary tag just like in production.

That might feel a bit hackish, I'd be curious to know if there's any other way to do that. Would greatly appreciate your insight on the topic!

@aarongustafson

This comment has been minimized.

Show comment
Hide comment
@aarongustafson

aarongustafson Oct 19, 2017

Collaborator

@alexandrediasdasilva Are you building to a staging server or are you thinking in the context of jekyll serve? The first is more difficult and would likely require a separate jekyll command. The second could be easily handled by checking a configuration variable in the render method:

def render(context)
  serving = context.registers[:site].config['serving']
  if serving
    # use local URLs  
  else
    # use cloudinary URLs
  end
  #
end
Collaborator

aarongustafson commented Oct 19, 2017

@alexandrediasdasilva Are you building to a staging server or are you thinking in the context of jekyll serve? The first is more difficult and would likely require a separate jekyll command. The second could be easily handled by checking a configuration variable in the render method:

def render(context)
  serving = context.registers[:site].config['serving']
  if serving
    # use local URLs  
  else
    # use cloudinary URLs
  end
  #
end
@pascalwhoop

This comment has been minimized.

Show comment
Hide comment
@pascalwhoop

pascalwhoop Nov 11, 2017

Contributor

I was thinking about changing the code a bit for my personal usage. Instead of having it generate

<img
  src="http://res.cloudinary.com/<cloud_name>/image/fetch/c_limit,w_800,q_auto,f_auto/https://<your-domain>/assets/img.jpg"
  srcset="
    http://res.cloudinary.com/<cloud_name>/image/fetch/c_limit,w_320,q_auto,f_auto/https://<your-domain>/assets/img.jpg 320w,
    ..."
/>

I would it have do

<img
  src="/assets/img.jpg"
  srcset="
    http://res.cloudinary.com/<cloud_name>/image/fetch/c_limit,w_320,q_auto,f_auto/https://<your-domain>/assets/img.jpg 320w,
    ..."
/>

So basically I would "break" your setup and take the performance hit on those browsers that don't support srcset (massive hit too, since my masters are 5MB+).
But caniuse is giving me good support.

Do you see any downside to this?

The reason is simple: It saves the whole "development mode" discussion. It doesn't find the srcset so it falls back to src right?

Contributor

pascalwhoop commented Nov 11, 2017

I was thinking about changing the code a bit for my personal usage. Instead of having it generate

<img
  src="http://res.cloudinary.com/<cloud_name>/image/fetch/c_limit,w_800,q_auto,f_auto/https://<your-domain>/assets/img.jpg"
  srcset="
    http://res.cloudinary.com/<cloud_name>/image/fetch/c_limit,w_320,q_auto,f_auto/https://<your-domain>/assets/img.jpg 320w,
    ..."
/>

I would it have do

<img
  src="/assets/img.jpg"
  srcset="
    http://res.cloudinary.com/<cloud_name>/image/fetch/c_limit,w_320,q_auto,f_auto/https://<your-domain>/assets/img.jpg 320w,
    ..."
/>

So basically I would "break" your setup and take the performance hit on those browsers that don't support srcset (massive hit too, since my masters are 5MB+).
But caniuse is giving me good support.

Do you see any downside to this?

The reason is simple: It saves the whole "development mode" discussion. It doesn't find the srcset so it falls back to src right?

@pascalwhoop

This comment has been minimized.

Show comment
Hide comment
@pascalwhoop

pascalwhoop Nov 11, 2017

Contributor

ah I just tested it. The browser doesn't fall back to the src="" version if srcset version is not available. That is unfortunate.

Contributor

pascalwhoop commented Nov 11, 2017

ah I just tested it. The browser doesn't fall back to the src="" version if srcset version is not available. That is unfortunate.

@pascalwhoop

This comment has been minimized.

Show comment
Hide comment
@pascalwhoop

pascalwhoop Nov 11, 2017

Contributor

this one worked for me in line 292

if ENV["JEKYLL_ENV"] != "production"
            return "<img src=\"#{image_dest_url}\" #{attr_string} #{img_attr} #{width_height}/>"
end

sure it's dirty as we should check this at the beginning somewhere and not AFTER we built all the strings and probably there is a better "right way" to access that variable through the jekyll context, but this was the quickest way for me :-)

also it keeps the width and height attributes that you add so we don't get random surprises all of the sudden with our layout being blown apart by these once we push

Contributor

pascalwhoop commented Nov 11, 2017

this one worked for me in line 292

if ENV["JEKYLL_ENV"] != "production"
            return "<img src=\"#{image_dest_url}\" #{attr_string} #{img_attr} #{width_height}/>"
end

sure it's dirty as we should check this at the beginning somewhere and not AFTER we built all the strings and probably there is a better "right way" to access that variable through the jekyll context, but this was the quickest way for me :-)

also it keeps the width and height attributes that you add so we don't get random surprises all of the sudden with our layout being blown apart by these once we push

@nhoizey

This comment has been minimized.

Show comment
Hide comment
@nhoizey

nhoizey Dec 15, 2017

Owner

There is already a PR that might help, but I didn't check it yet:
#29

Owner

nhoizey commented Dec 15, 2017

There is already a PR that might help, but I didn't check it yet:
#29

@nhoizey

This comment has been minimized.

Show comment
Hide comment
@nhoizey

nhoizey May 21, 2018

Owner

@pascalwhoop's PR is merged

Owner

nhoizey commented May 21, 2018

@pascalwhoop's PR is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment