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

[RTM] Provide a configuration option to define the predefined image sizes #537

Merged
merged 23 commits into from Jul 2, 2019

Conversation

@qzminski
Copy link
Member

commented Jun 27, 2019

This will allow to configure the image sizes in config.yml file as follows:

contao:
    image:
        sizes:
            foobar:
                width: 100
                height: 200
                resizeMode: 'box'
                zoom: 100
                cssClass: 'foobar-image'
                densities: '1x, 2x'
                sizes: '100vw'
                items:
                    -
                        width: 50
                        height: 50
                        resizeMode: 'box'
                        zoom: 100
                        cssClass: 'foobar-image'
                        densities: '0.5x, 2x'
                        sizes: '50vw'
                        media: '(max-width: 900px)'
                    -
                        width: 25
                        height: 25
                        resizeMode: 'box'
                        zoom: 100
                        densities: '0.5x, 2x'
                        sizes: '25vw'
                        media: '(max-width: 450px)'
            foobaz:
                width: 300
                height: 400
                #

The labels would be defined in a Symfony translation file:

# /translations/image_sizes.en.yml
foobar: Foobar image size

Idea came up together with @Toflar.

@Toflar
Copy link
Member

left a comment

Awesome! This will allow us to configure image sizes that are versioned and can be deployed to multiple setups 🕺

core-bundle/src/Image/ImageSizes.php Outdated Show resolved Hide resolved
@fritzmg

This comment has been minimized.

Copy link
Collaborator

commented Jun 27, 2019

How do you reference these in the code (e.g. with Controller::addImageToTemplate)?

Just like this?

[null, null, 'foobar']
@qzminski

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

@fritzmg yes, exactly like that.

@fritzmg

This comment has been minimized.

Copy link
Collaborator

commented Jun 27, 2019

Nice :). However, what happens if you define something like this?

contao:
    image:
        sizes:
            box:
                width: 100
                height: 200
                resizeMode: 'box'
@qzminski

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

@fritzmg that is a valid point, also relates to the database entries which consists only of the ID. We can either make something prevail or add validation upon container compiling. What do you think guys?

@Toflar

This comment has been minimized.

Copy link
Member

commented Jun 27, 2019

Validate👌🏻

@qzminski

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

Validation done and you can guys review everything now.

@qzminski qzminski marked this pull request as ready for review Jun 27, 2019

@qzminski qzminski changed the title Provide a configuration option to define the predefined image sizes [POC] Provide a configuration option to define the predefined image sizes Jun 27, 2019

@Toflar Toflar requested a review from ausi Jun 27, 2019

@Toflar Toflar added the feature label Jun 27, 2019

@Toflar Toflar changed the title [POC] Provide a configuration option to define the predefined image sizes Provide a configuration option to define the predefined image sizes Jun 28, 2019

@leofeyer leofeyer added this to the 4.8 milestone Jun 28, 2019

leofeyer added a commit that referenced this pull request Jul 1, 2019
Group image sizes by theme (see #539)
Description
-----------

This PR implements #326. It groups the predefined image sizes by theme instead of listing them all in the "Predefined dimensions" section.

@ausi @qzminski @Toflar Before we merge this, we should check how the changes affect #537. Do we want to list image sizes defined in a `config.yml` file under "Predefined images" or do we want to be able to define custom section names?

Commits
-------

c4cdb54 Group image sizes by theme
bf21f7d Restore the image_sizes array and label
444d5d3 Use array_merge_recursive() to merge the options
qzminski added 10 commits Jun 27, 2019

@qzminski qzminski force-pushed the qzminski:feature/predefined-image-sizes branch from 6c81ecc to 9c5e842 Jul 1, 2019

@qzminski

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2019

I rebased my changes on #539 👍

qzminski added 2 commits Jul 1, 2019
qzminski added 5 commits Jul 1, 2019
core-bundle/src/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved
core-bundle/src/Image/ImageFactory.php Outdated Show resolved Hide resolved
core-bundle/src/Image/ImageFactory.php Outdated Show resolved Hide resolved
core-bundle/src/Image/ImageFactory.php Outdated Show resolved Hide resolved
core-bundle/src/Image/ImageSizes.php Outdated Show resolved Hide resolved
core-bundle/src/Image/ImageSizes.php Outdated Show resolved Hide resolved
core-bundle/src/Image/PictureFactory.php Outdated Show resolved Hide resolved
core-bundle/src/Image/PictureFactory.php Outdated Show resolved Hide resolved
qzminski added 3 commits Jul 2, 2019
CS
@ausi
ausi approved these changes Jul 2, 2019
ResizeConfigurationInterface::MODE_BOX,
ResizeConfigurationInterface::MODE_PROPORTIONAL,
ResizeConfigurationInterface::MODE_CROP,
'left_top',

This comment has been minimized.

Copy link
@leofeyer

leofeyer Jul 2, 2019

Member

@ausi I wonder if we should add a ResizeConfigurationInterface::CROP_MODES constant so we do not have to duplicate this code?

@leofeyer leofeyer merged commit 9c95faa into contao:master Jul 2, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.3%) to 87.103%
Details
@leofeyer

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

Thank you @qzminski.

@qzminski qzminski deleted the qzminski:feature/predefined-image-sizes branch Jul 2, 2019

leofeyer added a commit that referenced this pull request Jul 3, 2019
Autorotate images based on their EXIF metadata (see #529)
Description
-----------

Depends on ~~contao/image#52 and ~~#537~~

As discussed in contao/image#44 (comment) we want to automatically rotate images if they have an EXIF IFD0-Orientation tag.

Because lighbox images now run through the resizer too I added a ~~`contao.image.lightbox_size` configuration~~ `lightboxSize` layout setting where you can define the (maximum) size of your lightbox images. This can be overwritten by providing an `$arrItem['lightboxSize']` to the `addImageToTemplate()` method.

I also added a ~~`contao.image.force_re_encoding` configuration~~ `skipIfDimensionsMatch` flag to the image size settings, this might be useful to get the benefits of resizing (like to-RGB-conversion, interlacing, metadata-stripping, sampling factors and specific JPEG-quality) even if no resize is required. (Related: contao/core-bundle#636)

~~Any opinions on the naming of `lightbox_size` and `force_re_encoding`?~~

Commits
-------

51c1fd6 Autorotate images based on EXIF metadata
3798184 Add contao.image.lightbox_size configuration
39b3cef Add image.force_re_encoding configuration
4fb9ea2 Use the autorotation branch of contao/image
6fc4dba Fix tests
a6d92a2 Move force_re_encoding flag to size configuration
689fd3e Move lightbox_size to layout configuration
3e0fb0f Add translation messages
bae425f Improve skipIfDimensionsMatch description
cb6395e Fix phpstan issues
c3b981b Add $arrItem['lightboxSize'] parameter
32cffec Fix composer.json
f247946 Fix typo
aa83d03 Rename forceReEncoding to inverted skipIfDimensionsMatch
ee41741 Use 1.0.0-beta2 version of contao/image
14433cf Add skipIfDimensionsMatch to predefined sizes
5ec5af1 Remove leftover lightbox_size configuration
610264d Fix issues from rebase
aea51f4 Fix the coding style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.