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

Improve Image Guide #1925

Merged
merged 11 commits into from
Mar 7, 2019
Merged

Improve Image Guide #1925

merged 11 commits into from
Mar 7, 2019

Conversation

gchtr
Copy link
Member

@gchtr gchtr commented Feb 18, 2019

Ticket: #429, #404 and #1672

Issue

There have been requests to better document the limitations with Timber’s image functions.

Solution

Let’s try to list the limitations in the image guide.

Impact

None.

Usage Changes

None.

Considerations

Improving Timber’s overall image experience will eventually make these updates useless. Hopefully.

Testing

None.

@codecov
Copy link

codecov bot commented Feb 18, 2019

Codecov Report

Merging #1925 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1925   +/-   ##
=========================================
  Coverage     95.04%   95.04%           
  Complexity     1556     1556           
=========================================
  Files            48       48           
  Lines          3672     3672           
=========================================
  Hits           3490     3490           
  Misses          182      182

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 587cff2...ebb67e2. Read the comment docs.

@jarednova
Copy link
Member

@gchtr thanks for those adds! I cleaned-up a few details and also tried to elaborate on the CDN limitations. In what I've used before (WPEngine, Pantheon) those filters have all worked 👌. My guess is that plugins/methods that rely on WP "knowing" about the generated images will fail; while ones that are merely watching for new/changed files should work. Any hosts/setups you've found that work for those?

@coveralls
Copy link

coveralls commented Feb 22, 2019

Coverage Status

Coverage remained the same at 94.324% when pulling ebb67e2 on docs/resize-limitations into 587cff2 on master.

@gchtr
Copy link
Member Author

gchtr commented Feb 22, 2019

@jarednova Well, I’ve never worked with CDNs and Timber images before, so I really can’t tell 🤷‍♂️. I added some changes to update the text so that it basically says: «It works with some CDN plugins of a certain type, but not with all.»

@gchtr gchtr requested a review from jarednova February 22, 2019 23:48
Copy link
Member

@jarednova jarednova left a comment

Choose a reason for hiding this comment

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

Your breakdown of the CDNs is much clearer than mine. I just fixed one other small thing. Good to go!

@gchtr gchtr merged commit dd57f2b into master Mar 7, 2019
@palmiak
Copy link
Collaborator

palmiak commented Mar 7, 2019

@gchtr as I made the same mistake with review as in #1795 (sorry) I want you to consider one more thing:

I would add here a word or two about Pull Zone CDNs. Timber should work flawlessly with all of the them.

There is a nice definition on cdn.net:

When the end-user sends the request for the web content it “pulls” it down from the nearest edge server (cdn location). All the content is cached in one place and the CDN does the work to pull it down into the end-users browser.
The catch: It needs to be said, that the first person to send a request to a new CDN location will find it hasn’t yet pulled that information and cached it ready for viewing. Making their experience seem no different to a site without CDN enabled. But once that first request has been made, the content is cached, and there it will stay until you tell it otherwise.

What do you think about this?

@gchtr gchtr deleted the docs/resize-limitations branch March 7, 2019 17:38
@gchtr
Copy link
Member Author

gchtr commented Mar 7, 2019

@palmiak Nevermind about the review misunderstanding :). Thanks, that’s a good hint! Now I too understand the difference between Push and Pull CDN 👍. I updated the text in #1944.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDN Content Delivery Networks docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants