Skip to content

feat: derive image field ID from focal point field ID for dynamic fie…[] #9899

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

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

Conversation

iDVB
Copy link

@iDVB iDVB commented Jun 6, 2025

Dynamic Image Field Association for Focal Point App

Purpose

The Image Focal Point app was previously hardcoded to only work with a sibling field named "image". This limitation made it difficult to use the app with multiple image fields in the same content type. This change enables more flexible field configurations by dynamically deriving the target image field ID from the focal point field's own ID.

Approach

We implemented a new deriveImageFieldId utility function that follows a consistent naming pattern:

  • focalPointimage (maintains backward compatibility)
  • focalPointLargeimageLarge
  • focalPointSomeThingimageSomeThing

All references to the hardcoded IMAGE_FIELD_ID constant were updated to use this dynamic approach, while ensuring backward compatibility for existing implementations.

Testing steps

  1. Add a field of type JSON with ID "focalPoint" and configure the focal point app - verify it works with an "image" field
  2. Add a field of type JSON with ID "focalPointLarge" and configure the focal point app - verify it works with an "imageLarge" field
  3. Add a field of type JSON with ID "focalPointCustom" and configure the focal point app - verify it works with an "imageCustom" field
  4. Check that error handling shows the correct field ID in error messages when target image fields are not found

Breaking Changes

None. This change is fully backward compatible:

  • Existing fields named exactly "focalPoint" will continue to associate with "image" fields
  • The default fallback remains "image" for any unexpected field naming patterns
  • No configuration changes are required for existing implementations

Dependencies and/or References

N/A

Deployment

Standard deployment process applies; no special deployment steps required.

@iDVB iDVB requested a review from a team as a code owner June 6, 2025 14:22
Copy link

netlify bot commented Jun 6, 2025

👷 Deploy request for ecommerce-app-base-components pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit f9f5634

Copy link
Contributor

@sarahlessner sarahlessner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iDVB Thank you for taking the time to enhance the functionality of our image focal point app. I tested the change locally and it seems like the new functionality works as expected. I do see some test failures though, are these on your radar?

@sarahlessner sarahlessner changed the title feat: derive image field ID from focal point field ID for dynamic fie… feat: derive image field ID from focal point field ID for dynamic fie…[] Jun 6, 2025
@iDVB
Copy link
Author

iDVB commented Jun 6, 2025

@iDVB Thank you for taking the time to enhance the functionality of our image focal point app. I tested the change locally and it seems like the new functionality works as expected. I do see some test failures though, are these on your radar?

No problem @sarahlessner !
I took a stab at looking at your CI details and could not deduce what exactly is the error(s).
I saw that something wasn't liked about the PR name itself?
Also don't seem to be able to get into circle ci build step to see what went wrong with the build.

Not sure what the requirement for the PR title is?

@sarahlessner sarahlessner self-requested a review June 6, 2025 23:31
@sarahlessner
Copy link
Contributor

sarahlessner commented Jun 6, 2025

Hey @iDVB I was having some issues inspecting the failures in CircleCI as well, but when I run npm run test locally there are some tests failing related to mocks that need to be updated related to the changes you made.

Don't worry about the PR title failure check, it's more for us internally related to associating tickets with our PRS, and we need to refine that a bit not to run on forked branches.

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.

Can't have more than one focalPoint per content entry
2 participants