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

Enhance srcset method #1806

Closed
wants to merge 1 commit into from
Closed

Enhance srcset method #1806

wants to merge 1 commit into from

Conversation

nilshoerrmann
Copy link
Contributor

Describe the PR

When discussing #1520 with @distantnative, I noticed that Kirby's srcset method is limited to resizing and doesn't take additional options for cropping or other image manipulations. This pull request intends to add this by switching from the resize to the thumb method for url generation.

The $sizes variable now accepts a new array structure moving the image condition to the key and the image options to the value:

$image->srcset([
    '1x' => [
        'width' => 38,
        'height' => 38,
        'crop' => 'center'
    ],
    '2x' => [
        'width' => 76,
        'height' => 76,
        'crop' => 'center'
    ],
    '3x' => [
        'width' => 152,
        'height' => 152,
        'crop' => 'center'
    ]
]);

The old condition syntax

$image->srcset([
  800  => '1x',
  1600 => '1.5x'
]);

still works for compatibility reasons but should be replaced in the docs with the more powerful proposed syntax in this pull request in my eyes.

The simple syntax of defining a plain array of widths has not been changed.

Related issues

Todos

  • Add unit tests for fixed bug/feature
  • Pass all unit tests
  • Fix code style issues with CS fixer and composer fix
  • If needeed, in-code documentation (DocBlocks etc.)

👆 I have no idea what is really needed from this list and how it's done.

@distantnative distantnative added this to the 3.2.0 milestone May 28, 2019
@distantnative distantnative added status: to review 👀 type: enhancement ✨ Suggests an enhancement; improves Kirby labels May 28, 2019
@bastianallgeier
Copy link
Member

I just merged this manually! Thanks for the great PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants