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 Incorrect Mapping of 'fit' Attribute Values in imgix Configuration #1104

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

drzastwakamil
Copy link

@drzastwakamil drzastwakamil commented Nov 10, 2023

Context:
This PR addresses a specific issue in the nuxt-image library's imgix.ts file, which is responsible for configuring the imgix service integration. The imgix service provides functionality to manipulate images, including the use of the 'fit' attribute to control how images are adjusted to fit within specified dimensions.

Problem:
The value 'fill' for the 'fit' attribute is incorrectly mapped to 'scale'. This misalignment results in unexpected behavior, as the 'fill' and 'scale' values dictate different fitting strategies for images:
Here is how Scale attribute behaves
Here is how Fill attribute behaves

Issue:
Due to this incorrect mapping, when users specify 'fill' for the 'fit' attribute in their imgix configuration, the system defaults to the 'scale' behavior. This issue persists even when 'fill' is explicitly passed through modifiers, leading to a significant deviation from expected functionality.

Solution:
This PR proposes a modification to the imgix.ts configuration file. The goal is to correctly map the 'fit' property so that specifying 'fill' will accurately invoke the 'fill' behavior, rather than defaulting to 'scale'. It also adds the scale value to the values dictionary. This change will ensure that users leveraging the imgix service through nuxt-image will experience the expected behavior when they specify the 'fit' attribute for their images.

Impact:
This change will provide a more reliable and intuitive experience for developers using imgix with nuxt-image, ensuring that image fitting options perform as documented.

Updating valueMap for fit property.
@drzastwakamil drzastwakamil changed the title Update imgix.ts - fixing fit properties value map Fix Incorrect Mapping of 'fit' Attribute Values in imgix Configuration Nov 10, 2023
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9427c1c) 89.94% compared to head (a2b1231) 89.94%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1104   +/-   ##
=======================================
  Coverage   89.94%   89.94%           
=======================================
  Files          46       46           
  Lines        3093     3094    +1     
  Branches      347      347           
=======================================
+ Hits         2782     2783    +1     
  Misses        310      310           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielroe
Copy link
Member

The reason for this behaviour is that Nuxt Image documents fill=fill as how imgix uses scale:

nuxt image docs: fill: Ignore the aspect ratio of the input and stretch to both provided dimensions.
https://image.nuxt.com/usage/nuxt-img#fit

imgix docs: scale: Scales the image to fit the constraining dimensions exactly. The resulting image will fill the dimensions, and will not maintain the aspect ratio of the input image.

We likely should document this rather than changing the behaviour here...

@drzastwakamil
Copy link
Author

The reason for this behaviour is that Nuxt Image documents fill=fill as how imgix uses scale:

nuxt image docs: fill: Ignore the aspect ratio of the input and stretch to both provided dimensions.
https://image.nuxt.com/usage/nuxt-img#fit

imgix docs: scale: Scales the image to fit the constraining dimensions exactly. The resulting image will fill the dimensions, and will not maintain the aspect ratio of the input image.

We likely should document this rather than changing the behaviour here...

Maybe I'am misunderstanding something but according to nuxt image imgix provider documentation users should be able to use imgix modifiers via the modifiers attribute. With current configuration I'am unable to recreate such imgix url:
ar=9:3&fit=fill&fill=solid&fill-color=orange
because the fill argument is mapped to scale. The expected outcome is different than what imgix produces. You can see the example at imgix aspect ratio docs. Please let me know if I understand it correctly. If I misunderstood I will close the PR.

Thanks

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.

None yet

3 participants