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

There's been a breaking change somewhere between 3.1 and 3.3.1 #1281

Closed
simonhamp opened this issue Jan 25, 2024 · 3 comments
Closed

There's been a breaking change somewhere between 3.1 and 3.3.1 #1281

simonhamp opened this issue Jan 25, 2024 · 3 comments

Comments

@simonhamp
Copy link

Describe the bug
My test suite is failing when running against 3.3.1, but passes when running against 3.1.

You can see the test suite results here

It seems to be occurring because I'm manually instantiating the driver-specific modifier.

I'm doing that so that I can use a custom version of the modifier which explicitly exposes the Imagick TextModifier's boxSize method's visibility as public.

I'm doing this so that I can re-use that mechanism of calculating the size of the text, which I'm using as an approximation to help decide where lines should be broken when the text is constrained to a specific width.

(Perhaps there is already a better way to do this?)

Code Example
The files I've linked above may be the best code example.

Expected behavior
Given that the tests pass when running v3.1 of intervention/image, I'd have expected them to still pass on v3.3.1 as it's not clear that there are breaking changes

Environment (please complete the following information):

  • PHP Version: 8.2.14
  • OS: Ubuntu 22.04.3
  • Intervention Image Version: 3.3.1
  • GD or Imagick: Imagick
@olivervogel
Copy link
Member

olivervogel commented Jan 25, 2024

This could well be, since this version jump it is no longer possible to instantiate the "driver specific" modifiers directly, you always have to go through the "generic" class.

This is also consistent with your error message.

If I understand you correctly, you are adding the ability to output text in a specific width with text wrapping. This is a long requested feature. See #143 Can you report on your success of the implementation? Perhaps you would like to integrate the wrapping feature directly into this library.

If I understand you correctly, one of the following things would provide a rather quick help.

  • Change the visibility of Intervention\Image\Drivers\Imagick\Modifiers\TextModifier::boxSize() to public
  • Allow direct instantiation of "driver-specific" modifiers

simonhamp added a commit to simonhamp/the-og that referenced this issue Jan 25, 2024
A breaking change was introduced somewhere after 3.1, so this is a hotfix for that.

See Intervention/image#1281
@simonhamp
Copy link
Author

Thanks for your rapid reply @olivervogel and confirming the issue 🙏

I don't think anything provides quick help unfortunately... it looks like I'm going to need to refactor my code one way or another to instantiate the modifier your way.

Short-term, I've locked to v3.1 as that's the version that works.

But ultimately I think the right solution here will be making boxSize public so that I don't have to create my very basic custom TextModifier. Then I could completely rely on your instantiation logic. I'm happy to PR this change.

I also agree that text-wrapping would be good as a core feature - that would also allow me to move that out of my package too. My approach is quite opinionated tho and is even still a bit buggy.

It would be great if we could collab on that piece at some point.

@olivervogel
Copy link
Member

I hope I could help you with the MR. Feel free to contact me if you have any further questions or if you are interested in implementing your text wrapping solution directly into Intervention Image.

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