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

FocusFill doesn't check wether owner exists #59

Closed
lerni opened this issue Feb 7, 2019 · 6 comments
Closed

FocusFill doesn't check wether owner exists #59

lerni opened this issue Feb 7, 2019 · 6 comments

Comments

@lerni
Copy link
Contributor

lerni commented Feb 7, 2019

If an Image gets deleted but references still exists, FocusFill & FocusFillMax 'll error like: "Uncaught InvalidArgumentException: Width is required". A check like if ($this->owner->exists()) in FocusFill would prevent this. Would it be aceptable to just wrap the return in such a condition?

@jonom
Copy link
Owner

jonom commented Feb 7, 2019

Thanks @lerni. Any chance you would like to open a PR to fix this?

@lerni
Copy link
Contributor Author

lerni commented Feb 7, 2019

Yeah, with pleasure. So you think what I said is the right approach?

@jonom
Copy link
Owner

jonom commented Feb 8, 2019

I think so, but maybe you could check what the behaviour is for built-in methods. Like if you call Fill instead of FocusFill in the same situation does it fail silently or throw an error? Replicating the native behaviour for consistency is probably the best way to go.

@jonom
Copy link
Owner

jonom commented Jul 1, 2019

Fixed with #63

@jonom jonom closed this as completed Jul 1, 2019
@lerni
Copy link
Contributor Author

lerni commented Jul 2, 2019

thank you @mikenz, somehow just never made it.

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