[3.4-dev] Include blank option for image size selects

[ausi]

If you create a new image content element in Contao 3.3 the size is set to ['', '', 'proportional'] which doesn't resize the image because no dimensions are set.

If you do the same in Contao 3.4 size is set to something like ['', '', '1'] which does resize the image because '1' is the ID of an image size.

We should set 'includeBlankOption' => true for all image size fields.

[aschempp]

That would be a BC break with all existing extensions.

[ausi]

Why?

"Old" extensions wouldn't use System::getImageSizes() so they don't need to set 'includeBlankOption' => true

[leofeyer]

Hm, this is a problem indeed. But includeBlankOption does not seem a good solution, does it? How does Contao behave, if an empty value is passed?

[aschempp]

Well if width & height are empty, the third flag should be completely ignored. That should also be the case in the future, no matter if it's 1 or proportional.

(I have not checked any code yet).

[leofeyer]

Well if width & height are empty, the third flag should be completely ignored.

That's not possible, because when you select a predefined image set, width and height are always empty.

[ausi]

If the resize mode is an image size ID, width and height are ignored.

If it's not, and width and height are empty, the resize mode gets ignored, see system/modules/core/library/Contao/Image.php:408-413

So ['', '', 'proportional'] and ['', '', ''] produce the same result.

[leofeyer]

Why is it that in Contao 3.4 the size is set to ['', '', '1'] instead of ['', '', 'proportional'] by default?

[ausi]

Because the image sizes are listed at the top of the select list (which is IMO correct) and because there is no blank option the first item gets selected by the browser.

[leofeyer]

So if there are no custom images, the default selection is still ['', '', 'proportional']?

[ausi]

Yes.

[ausi]

We could also move the image sizes to the bottom of the list to fix this issue, but i don't like that.

[leofeyer]

I think we can leave it as is. Actually, I think it is good that Contao uses a predefined dimension as default if there is one. And if there are none, it behaves exactly like before, so everything is fine to me.

[ausi]

But if I don't want to resize an image I have to select "proportional" which is confusing IMO.

[leofeyer]

Oh, now I see what you mean. Then why don't we add the includeBlankOption flag? :D

Sorry, sometimes its hard to understand things from reading tickets only.

[aschempp]

The blank option is confusing too, because then I can enter width & height and nothing will happen because no resize mode is selected…

[ausi]

We could update the script to disable the width and height fields if the blank option is selected.

[leofeyer]

Good idea.

[leofeyer]

We could also – for the sake of backwards compatibility – preselect the "proportional" option if no selection has been made.

[leofeyer]

This would solve the problem:

            'load_callback' => array
            (
                function($val)
                {
                    if (empty($val))
                    {
                        $val = serialize(array('', '', 'proportional'));
                    }

                    return $val;
                }
            ),

@ausi @aschempp What do you think?

[leofeyer]

And this would solve the issue in the ImageSize class:

    public function generate()
    {
        if (!is_array($this->varValue))
        {
            $this->varValue = array($this->varValue);
        }

        // Set the default value (see #7436)
        if (!isset($this->varValue[2]))
        {
            $this->varValue[2] = 'proportional';
        }

        // Backwards compatibility (see #3911)
        elseif ($this->varValue[2] == 'crop')
        {
            $this->varValue[2] = 'center_center';
        }

        ...

Probably even better, because it does not require to adjust the DCA files.

[ausi]

I still don't like having to select "proportional" to disable resizing, even if it is the default. What about adding a none option to System::getImageSizes()?

static::$arrImageSizes = array_merge(array('none', 'image_sizes' => $sizes), $GLOBALS['TL_CROP']);

[aschempp]

Would not solve my previous comment:

The blank option is confusing too, because then I can enter width & height and nothing will happen because no resize mode is selected…

[ausi]

Updating the script to disable the width and height fields if none is selected would solve it, right?

assets/contao/js/core-uncompressed.js:2026:

if (select.get('value') === 'none' || select.get('value').toInt().toString() === select.get('value')) {

[leofeyer]

I still don't like having to select "proportional" to disable resizing, even if it is the default.

Me neither. But I'd prefer to keep the behavior of the previous Contao versions for the sake of consistency. Still I wouldn't mind having a blank/none option with the input fields disabled. You two just have to manage to agree to something now :)

[aschempp]

Can you point me to some code about the new image size options? I must admit I don't know anything about them yet. Maybe @Toflar can help here too?

[ausi]

DCA image size: system/modules/core/dca/tl_content.php:246-255

System::getImageSizes(): system/modules/core/library/Contao/System.php:480-510

JS which disables/enables width and height fields: assets/contao/js/core-uncompressed.js:2017-2048

[leofeyer]

If we really cannot agree on a solution, it will be up to @ausi to make the decision, because it kind of is "his" feature ;)

[ausi]

I would go with 'includeBlankOption' => true and update assets/contao/js/core-uncompressed.js:2026 to:

if (select.get('value') === '' || ...

[Toflar]

I agree with @ausi. I'd do the same.

[aschempp]

Fine with me :)

[leofeyer]

Changed in 7ec91ee.