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

Fix responsive url generation #2852

Merged
merged 2 commits into from
Mar 22, 2022
Merged

Fix responsive url generation #2852

merged 2 commits into from
Mar 22, 2022

Conversation

mabdullahsari
Copy link
Contributor

This is a duplicate of #2837 that fixes the issue for v10.

Closes #2778.

@mabdullahsari
Copy link
Contributor Author

mabdullahsari commented Mar 21, 2022

@freekmurze Hey 👋🏼

I think the tests need updating, am I right? I tested it locally on an app (L9 - ML10) that relies heavily on media-library, and everything seems to be working fine with the S3 disk.

The localhost portion is probably omitted everywhere because it is the local disk, and thus does not really require an absolute URL.

How do I proceed?

@freekmurze
Copy link
Member

The tests indeed need updating. If you want, go ahead and already do that. If not I'll update them later this week.

@mabdullahsari
Copy link
Contributor Author

The tests indeed need updating. If you want, go ahead and already do that. If not I'll update them later this week.

I'll update them no problems. Just wanted to make sure. Thanks!

@mabdullahsari
Copy link
Contributor Author

I guess all of the code analysis failures are unrelated to my changes.

@freekmurze
Copy link
Member

Yeah, I'll fix those myself, no worries!

@mabdullahsari
Copy link
Contributor Author

The tests are OK!

@freekmurze freekmurze merged commit 2799377 into spatie:main Mar 22, 2022
@freekmurze
Copy link
Member

Thank you!

@mabdullahsari
Copy link
Contributor Author

Thank you!

Tyvm for merging 🙏

Could you release this as a patch please? 😇

@ricardo-lobo
Copy link

Can this be merged into v10 too?

@freekmurze
Copy link
Member

freekmurze commented Oct 11, 2022 via email

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

Successfully merging this pull request may close these issues.

Responsive images of a conversion from a collection changed in v10?
3 participants