feat: custom srcset #55
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sherwinski
reviewed
May 4, 2020
src/Imgix/UrlBuilder.php
Outdated
| @@ -65,65 +74,96 @@ public function createURL($path, $params=array()) { | |||
| return $uh->getURL(); | |||
| } | |||
|
|
|||
| public function createSrcSet($path, $params=array()) { | |||
| public function createSrcSet($path, $params=array(), $start=100, $stop=8192, $tol=8, $disable_variable_quality=false) { | |||
There was a problem hiding this comment.
Should we have disable_variable_quality written in camelCase? [1]
There was a problem hiding this comment.
Good catch! Yes! It should be disableVariableQuality, thank you! I will make this change right now.
There was a problem hiding this comment.
Okay, I have updated and camel-cased the flag: disableVariableQuality
sherwinski
approved these changes
May 4, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces custom srcsets. Target widths functionality has been
extended to support
$start,$stop, and$tol. These values definewhere the array of target widths start, stop, and how much variation
is tolerated between each subsequent width.
This functionality is accessed via the
createSrcSetfunction whichhas also been extended to support the above functionality.
Now when source set pairs are constructed they are constructed either
with default target widths or by a custom target widths array generated
if
$start,$stop, and/or$toldeviates from the defaults.This PR aims to test the new functionality in a simple manner. By
leveraging php's string literal conventions, integration tests are
performed on the entire output
srcsetstring. I chose this routeto keep the tests easy to reason about (i.e. tried to avoid complicated
setup and string/array manipulation).
Food for thought, we discussed whether we should impl this feature
with default arguments or by passing an
options=array(). I've chosento move ahead with default args, but am happy to impl another solution.
I could also add some constants here, namely for
8192,100, butI figured this could be done if/when
$INCREMENT_PERCENTAGEisnormalized to
$SRCSET_WIDTH_TOLERANCE.