-
Notifications
You must be signed in to change notification settings - Fork 16
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
Support SVGs #38
Support SVGs #38
Conversation
… the creator could change the image, such as when the image fails to load and it uses the error image instead.
Merging in! 👍 |
Merging in too! |
@@ -14,7 +14,8 @@ | |||
], | |||
|
|||
"require": { | |||
"bolt/filesystem": "^2.0", | |||
"bolt/filesystem": "^2.1", | |||
"contao/imagine-svg": "^0.1.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked at the code, and I find it hard to believe they aren't going to keep 0.x a stable series, i.e using ^
is anything but a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That supports >= 0.1.2
and < 0.2.0
. Or are you saying they'll break BC on a 0.1.x release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, just that while semver says that Y
s are majors when X
is a zero, nearly nobody actually deos that, hence one of the reasons Composer switched their default constraint marker to strict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nearly nobody actually deos that
You mean people don't break BC with 0.Y
releases or they do?
^
has always had that logic. Are you saying we should use ~
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean is that in my observation, very few people are aware that 0.Y
counts as major in SemVer and behave as if it were a minor. I'm pretty sure I've seen core developers to it to.
For a require, ~
might make sense in most places, but project dependant, ad if I don't know for sure, I usually go for the loose option.
Third, we stay as-is, come back in a year and see which wat it went 😉
*/ | ||
public function __construct($limitUpscaling = true, Color $background = null) | ||
public function __construct($limitUpscaling = true, SvgImagine $svgImagine = null, Color $background = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're ignoring BC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I didn't want to go past background since it wasn't provided in service provider.
Fixes #22
Currently they are all modified the same way, which I think is
border
. SVGs can't be cropped or stretched/fit. I don't know howresize
is different thanborder
.Wait for bolt/filesystem#43 to be merged and tagged.