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

Add support for Variant Presets #12

Closed
kdambekalns opened this issue Aug 6, 2019 · 2 comments
Closed

Add support for Variant Presets #12

kdambekalns opened this issue Aug 6, 2019 · 2 comments

Comments

@kdambekalns
Copy link
Contributor

I'd have a go at adding support for "variant presets" to Kaleidoscope. But…

First question would be: What to do about the name clash with the existing (thumbnail) preset option on ImageSource?

Should there be a new option variant? And should that be given as <PresetIdentifier>::<VariantIdentifier> or rather as two new options variantPreset and variant?

We could also "reuse" the preset option and take it as variant preset when given with :: in the identifier… too magic?

Any opinions here, @mficzel, @robertlemke, @kitsunet?

@mficzel mficzel self-assigned this Aug 7, 2019
@mficzel
Copy link
Member

mficzel commented Aug 7, 2019

@kdambekalns i would go with the name VariantPreset as the existing presets are working differently and this is what it is called in the configuration. (maybe we should rename the key "preset" to "thumbnailPreset" alongside).

Primarily the ImageSourceHelperInterface has to be extended for that (not really sure wether to use a combined identifier or two strings here):

interface ImageSourceHelperInterface {
    public function applyVariantPreset(string $group, string $variant) : 
ImageSourceHelperInterface;
}

While the function has to be implemented in all ImageSourceHelpers it can only have an effect in the scalable ones (Asset and Dummy).

In the assetImageSource the asset variant will obviously be switched. In addition the target dimensions have to be adjusted for Asset- and DummyImageSources from the variant configuration which is the part i consider most complicated.

The idea is that it should be possible to decide on the actually used variant on the presentation side which will be especially helpful for picture-tags.

On top of this it obviously makes sense add a variantPreset option to the Sitegeist.Kaleidoscope:AssetImageSource and Sitegeist.Kaleidoscope:AssetImageSource that internally trigger a call of applyVariantPreset(). That will make accessing variants on the integration side as easy as on the presentation side.

@mficzel
Copy link
Member

mficzel commented Oct 29, 2019

@kdambekalns this is released as kaleidoscope 5.1.0. I even managed to make this 4.3 and 5.0 compatible by checking if (FLOW_VERSION_BRANCH == '5.3') before sending cache headers

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

No branches or pull requests

2 participants