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(unsplash): merge query params with src #712

Merged
merged 3 commits into from
Jan 30, 2023
Merged

Conversation

danielpetrica
Copy link
Contributor

Edits done: Edited the provider for Unsplash, added a new case to the playground for Unsplash

Hi everyone, I found out that when using URLs which already contains query parameters, the provider will wrongly attach a '?' instead of using a & before the applied operations and Unsplash would ignore the first passed operation.

In these cases, placing a & would be the correct option.

I think another workaround would be to always place a '&' after '?' Like this: ?& but I don't think is elegant.

Here's an example of one generate url wrong:
https://images.unsplash.com/photo-1532236204992-f5e85c024202?crop=entropy&cs=tinysrgb&fm=jpg&ixid=MnwxOTgwMDZ8MHwxfHNlYXJjaHwyOHx8dG9reW98ZW58MHx8fHwxNjczNzk3MDIz&ixlib=rb-4.0.3&q=80?w=300&q=80
The wrong part: &q=80?w=300&q=80

Here's how it should appear correctly generated: https://images.unsplash.com/photo-1532236204992-f5e85c024202?crop=entropy&cs=tinysrgb&fm=jpg&ixid=MnwxOTgwMDZ8MHwxfHNlYXJjaHwyOHx8dG9reW98ZW58MHx8fHwxNjczNzk3MDIz&ixlib=rb-4.0.3&q=80&w=300&q=80
The corrected part: &q=80&w=300&q=80

And this is the source url: https://images.unsplash.com/photo-1532236204992-f5e85c024202?crop=entropy&cs=tinysrgb&fm=jpg&ixid=MnwxOTgwMDZ8MHwxfHNlYXJjaHwyOHx8dG9reW98ZW58MHx8fHwxNjczNzk3MDIz&ixlib=rb-4.0.3&q=80

I tried it in the playground and it works.
I'm open to edit it if there's a better way of doing it.

… by correctly appending the format query params.

I found out that when using URLs which already contains query parameters, the provider will wrongly attach a '?' Before the applied operations and Unsplash would ignore the first passed operation.
In these cases, placing a & would be a better option.
I think another workaround would be to always place a & after ? like this: ?&
Here's an example of one such link.
@netlify
Copy link

netlify bot commented Jan 16, 2023

👷 Deploy request for nuxt-image-v1 pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 8d409b6

@pi0 pi0 changed the title fix: Edit unsplash provider to support image urls returned by the api by correctly appending the format query params fix(unsplash): merge query params with src Jan 27, 2023
@pi0 pi0 merged commit a55b8b2 into nuxt:main Jan 30, 2023
@danielpetrica danielpetrica deleted the patch-1 branch January 30, 2023 12:05
procrates pushed a commit to procrates/nuxt-image that referenced this pull request Feb 21, 2023
@pi0 pi0 mentioned this pull request Mar 31, 2023
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.

2 participants