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

[FEATURE] Add crop feature to media.image srcset #1096

Open
wants to merge 6 commits into
base: development
Choose a base branch
from

Conversation

muehle28
Copy link

Crop feature for srcset pictures. Without this patch the srcset images are not rendered when a crop variable is set.

Crop feature for srcset pictures. Without this patch the srcset images are not rendered when a crop variable is set.
Division by zero check when calculating ratio
Wrap can be ignored when $treatIdAsReference=TRUE
Height calculation can be ignored when height attribute is missing

When $treatIdAsReference = FALSE the generation of the srcset images does not work. There is a problem when processed images are getting processed again.

If $treatIdAsReference = TRUE the srcset images are getting processed from the original image. Therefore the crop and ratio information is needed.
When setting the quality attribute MathUtility was not found

add use TYPO3\CMS\Core\Utility\MathUtility;
@cedricziel cedricziel changed the title Update SourceSetViewHelperTrait.php [FEATURE] Add crop feature to media.image srcset Sep 13, 2016
}

$width = $width . $dimensions['postWidth'];
$srcsetVariant = $this->getImgResource($src, $width, $height, $format, $quality, $treatIdAsReference, $crop);
Copy link
Member

Choose a reason for hiding this comment

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

Yes, I know that's super picky, but could you transform the call to a multi-line one? The code style still fails on checking it.

$this->getImgResource(
  $src, 
  $width, 
  $height,
  $format, 
  $quality, 
  $treatIdAsReference, 
  $crop
);

@NamelessCoder
Copy link
Member

Meta note, if you enable https://github.com/blog/2247-improving-collaboration-with-forks we can fix cosmetics for you ;)

@muehle28
Copy link
Author

Thank you. I enabled it.

$treatIdAsReference = (boolean) $this->arguments['treatIdAsReference'];
if (true === $treatIdAsReference) {
$src = $this->arguments['src'];
$crop = $this->arguments['crop'];
if ($crop === null) {
$crop = $src instanceof FileReference ? $src->getProperty('crop') : null;
Copy link
Member

Choose a reason for hiding this comment

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

We better import FileReference

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d650579 on muehle28:patch-1 into * on FluidTYPO3:development*.

@NamelessCoder
Copy link
Member

getImgResource signature change is potentially breaking for other ViewHelpers using the trait - may not be exposed by unit tests either!

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling baefc77 on muehle28:patch-1 into * on FluidTYPO3:development*.

@NamelessCoder NamelessCoder force-pushed the development branch 3 times, most recently from 7b35850 to 50d769b Compare June 2, 2022 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants