Skip to content

feat: additional style options for Photo Gallery#1136

Merged
asanehisa merged 7 commits intomainfrom
photo-gallery
Apr 2, 2026
Merged

feat: additional style options for Photo Gallery#1136
asanehisa merged 7 commits intomainfrom
photo-gallery

Conversation

@asanehisa
Copy link
Copy Markdown
Contributor

This is to support vertical images used among horizontal images.

Screen.Recording.2026-04-01.at.1.41.45.PM.mov

https://yext.slack.com/archives/C0A1G447MPD/p1774983039261999

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2026

Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information.

@asanehisa asanehisa marked this pull request as ready for review April 1, 2026 17:58
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fe3efa7c-0d97-4235-a4e4-ec8118a1f1e6

📥 Commits

Reviewing files that changed from the base of the PR and between a31a339 and 2ca2a2e.

⛔ Files ignored due to path filters (3)
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] latest version non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 64 static headings.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (2)
  • packages/visual-editor/locales/platform/cs/visual-editor.json
  • packages/visual-editor/locales/platform/et/visual-editor.json
✅ Files skipped from review due to trivial changes (2)
  • packages/visual-editor/locales/platform/et/visual-editor.json
  • packages/visual-editor/locales/platform/cs/visual-editor.json

Walkthrough

Adds an image fill type option across PhotoGallery components. Many locale files gain fields.imageFillType plus fields.options.fill and fields.options.fit. PhotoGalleryWrapper props now include styles.imageFillType?: "fill" | "fit" (default "fill"), passed from PhotoGallerySection into Carousel and Image item components; Image items set CSS object-fit to "contain" when "fit", otherwise "cover". A new carousel test exercises imageFillType: "fit".

Sequence Diagram(s)

sequenceDiagram
    participant PhotoGallerySection
    participant PhotoGalleryWrapper
    participant Carousel
    participant ImageItem
    participant Image

    PhotoGallerySection->>PhotoGalleryWrapper: pass styles.imageFillType (default "fill")
    PhotoGalleryWrapper->>Carousel: pass imageFillType
    Carousel->>ImageItem: pass imageFillType
    ImageItem->>Image: apply style.objectFit based on imageFillType

    alt imageFillType == "fit"
        Image->>Image: objectFit = "contain"
    else imageFillType == "fill"
        Image->>Image: objectFit = "cover"
    end
Loading

Possibly related PRs

Suggested labels

create-dev-release

Suggested reviewers

  • briantstephan
  • mkilpatrick
  • benlife5
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title accurately describes the main change: adding new style options (imageFillType with 'fill' and 'fit' modes) to the Photo Gallery component.
Description check ✅ Passed The pull request description explains the motivation for the changes (supporting vertical images among horizontal images) and includes relevant supporting links.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch photo-gallery

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
packages/visual-editor/src/components/pageSections/PhotoGallerySection/PhotoGalleryWrapper.tsx (1)

61-61: Consider extracting a shared ImageFillType alias.

"fill" | "fit" is repeated across multiple signatures; a single local type alias would reduce drift.

Also applies to: 233-234, 274-275, 310-311, 397-397

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/visual-editor/src/components/pageSections/PhotoGallerySection/PhotoGalleryWrapper.tsx`
at line 61, Extract a single type alias ImageFillType = "fill" | "fit" at the
top of the file and replace every inline union occurrence (e.g., the
imageFillType?: "fill" | "fit" prop in PhotoGalleryWrapper and the other
repeated signatures) with this alias; update all component prop types and
function signatures in this file to reference ImageFillType to avoid duplication
and keep types consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/visual-editor/locales/platform/cs/visual-editor.json`:
- Line 346: The label for the "fill" key uses the infinitive "Vyplnit"; change
its value to an imperative Czech form to match the project's style (e.g.,
replace the value for the "fill" key from "Vyplnit" to an imperative like
"Naplňte" or another context-appropriate imperative phrase) so the translation
aligns with other imperative labels.

In `@packages/visual-editor/locales/platform/et/visual-editor.json`:
- Line 348: The "fit" translation key currently uses a descriptive form
("Sobivad"); change its value to the correct Estonian imperative form—replace
"Sobivad" with "sobi" for singular imperative or "sobige" for plural/formal
imperative to match other imperative labels (key: "fit"). Ensure the chosen form
is consistent with the UI context and other entries like "fill": "Täida".

---

Nitpick comments:
In
`@packages/visual-editor/src/components/pageSections/PhotoGallerySection/PhotoGalleryWrapper.tsx`:
- Line 61: Extract a single type alias ImageFillType = "fill" | "fit" at the top
of the file and replace every inline union occurrence (e.g., the imageFillType?:
"fill" | "fit" prop in PhotoGalleryWrapper and the other repeated signatures)
with this alias; update all component prop types and function signatures in this
file to reference ImageFillType to avoid duplication and keep types consistent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 06d1ea9b-64cb-4ca0-8b8f-7f1cabb18f7c

📥 Commits

Reviewing files that changed from the base of the PR and between 542a422 and a31a339.

⛔ Files ignored due to path filters (3)
  • packages/visual-editor/src/components/testing/screenshots/PhotoGallerySection/[desktop] version 71 carousel variant with imageFillType fit and vertical image.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/PhotoGallerySection/[mobile] version 71 carousel variant with imageFillType fit and vertical image.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/PhotoGallerySection/[tablet] version 71 carousel variant with imageFillType fit and vertical image.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (28)
  • packages/visual-editor/locales/platform/cs/visual-editor.json
  • packages/visual-editor/locales/platform/da/visual-editor.json
  • packages/visual-editor/locales/platform/de/visual-editor.json
  • packages/visual-editor/locales/platform/en-GB/visual-editor.json
  • packages/visual-editor/locales/platform/en/visual-editor.json
  • packages/visual-editor/locales/platform/es/visual-editor.json
  • packages/visual-editor/locales/platform/et/visual-editor.json
  • packages/visual-editor/locales/platform/fi/visual-editor.json
  • packages/visual-editor/locales/platform/fr/visual-editor.json
  • packages/visual-editor/locales/platform/hr/visual-editor.json
  • packages/visual-editor/locales/platform/hu/visual-editor.json
  • packages/visual-editor/locales/platform/it/visual-editor.json
  • packages/visual-editor/locales/platform/ja/visual-editor.json
  • packages/visual-editor/locales/platform/lt/visual-editor.json
  • packages/visual-editor/locales/platform/lv/visual-editor.json
  • packages/visual-editor/locales/platform/nb/visual-editor.json
  • packages/visual-editor/locales/platform/nl/visual-editor.json
  • packages/visual-editor/locales/platform/pl/visual-editor.json
  • packages/visual-editor/locales/platform/pt/visual-editor.json
  • packages/visual-editor/locales/platform/ro/visual-editor.json
  • packages/visual-editor/locales/platform/sk/visual-editor.json
  • packages/visual-editor/locales/platform/sv/visual-editor.json
  • packages/visual-editor/locales/platform/tr/visual-editor.json
  • packages/visual-editor/locales/platform/zh-TW/visual-editor.json
  • packages/visual-editor/locales/platform/zh/visual-editor.json
  • packages/visual-editor/src/components/pageSections/PhotoGallerySection/PhotoGallerySection.test.tsx
  • packages/visual-editor/src/components/pageSections/PhotoGallerySection/PhotoGallerySection.tsx
  • packages/visual-editor/src/components/pageSections/PhotoGallerySection/PhotoGalleryWrapper.tsx

Comment thread packages/visual-editor/locales/platform/cs/visual-editor.json Outdated
Comment thread packages/visual-editor/locales/platform/et/visual-editor.json Outdated
Copy link
Copy Markdown
Contributor

@jwartofsky-yext jwartofsky-yext left a comment

Choose a reason for hiding this comment

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

nitpicks, but otherwise lgtm!

@asanehisa
Copy link
Copy Markdown
Contributor Author

nitpicks, but otherwise lgtm!

Oh I'll add people as reviewers when it is ready for review! I just set it to ready so CodeRabbit takes a look

@asanehisa asanehisa merged commit 0ba7493 into main Apr 2, 2026
17 checks passed
@asanehisa asanehisa deleted the photo-gallery branch April 2, 2026 13:57
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.

4 participants