Skip to content

feat(gallery): add new required gallery-item component for child items#31195

Merged
brandyscarney merged 29 commits into
nextfrom
FW-7301
Jun 12, 2026
Merged

feat(gallery): add new required gallery-item component for child items#31195
brandyscarney merged 29 commits into
nextfrom
FW-7301

Conversation

@brandyscarney

@brandyscarney brandyscarney commented Jun 5, 2026

Copy link
Copy Markdown
Member

Issue number: internal


What is the current behavior?

The Gallery component queries all direct children and uses them to build both the uniform and masonry layouts. This causes issues when a wrapper element exists between the gallery and the gallery items. For example:

<ion-gallery>
  <div class="some-wrapper">
    <img src="/src/components/gallery/test/assets/01.png" alt="One" />
    <img src="/src/components/gallery/test/assets/02.png" alt="Two" />
    <img src="/src/components/gallery/test/assets/03.png" alt="Three" />
    <img src="/src/components/gallery/test/assets/04.png" alt="Four" />
  </div>
</ion-gallery>

In this scenario, the gallery treats the wrapper as a single item and does not correctly lay out the images contained within it.

What is the new behavior?

Several approaches were considered:

  1. Add a class or attribute to wrapper elements that the Gallery could use to locate its items.
  2. Query for items using a specific class or attribute when present, and fall back to querying direct children otherwise.
  3. Introduce a new ion-gallery-item component that encapsulates all item-related styling and can contain arbitrary content (img, div, etc.).

We ultimately chose option 3 and introduced a new ion-gallery-item component. The ion-gallery component now queries for ion-gallery-item elements instead of relying on direct children.

As such, this PR does the following:

  • Adds a new ion-gallery-item component.
  • Updates ion-gallery to query for ion-gallery-item elements.
  • Updates existing E2E and spec tests to use the new structure.
  • Adds new E2E and spec tests for ion-gallery-item.
  • Adds an E2E test verifying that wrapper elements around gallery items are effectively transparent to gallery layout calculations.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 8.8.9-dev.11781201980.1b6e8398

Previews:

@vercel

vercel Bot commented Jun 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ionic-framework Ready Ready Preview, Comment Jun 12, 2026 10:15pm

Request Review

@github-actions github-actions Bot added package: core @ionic/core package package: angular @ionic/angular package package: vue @ionic/vue package package: react @ionic/react package labels Jun 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change is actually more accurate. In both images the blue rectangle I added is 16px tall. You can see on the left there is a half pixel above and below it.

Image

div {
ion-gallery-item {
color: #fff;
height: 150px;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved this to be on the items themselves because I can't apply them to all items or the ones containing img will also get an explicit height.

@@ -58,7 +58,7 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ config, screenshot, t
await expect(gallery).toHaveScreenshot(screenshot(`gallery-${layout}${orderSuffix}-divs-same-height`));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I didn't change the name of this to replace divs with items so you could see there are not diffs on these but I can update them after it's been reviewed if desired.

@brandyscarney brandyscarney marked this pull request as ready for review June 8, 2026 21:13
@brandyscarney brandyscarney requested a review from a team as a code owner June 8, 2026 21:13

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is looking really cool! I just had some change requests and some nits, but I'm loving the way it's looking!

Comment thread core/src/components/gallery/gallery.tsx Outdated
Comment thread core/src/components/gallery/gallery.tsx Outdated
Comment thread core/src/components/gallery-item/gallery-item.tsx Outdated
Comment thread core/src/components/gallery-item/gallery-item.tsx
Comment thread core/src/components/gallery/gallery.tsx
Comment thread core/src/components/gallery-item/gallery-item.tsx
Comment thread core/src/components/gallery-item/gallery-item.tsx
Comment thread core/src/components/gallery/gallery.spec.ts Outdated

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome work! Looks good to me 🚀

@thetaPC thetaPC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, left suggestions but not blockers

Comment thread core/src/components/gallery-item/gallery-item.scss Outdated
Comment thread core/src/components/gallery-item/gallery-item.scss Outdated
Comment thread core/src/components/gallery-item/gallery-item.tsx Outdated

import { sharedGalleryStyles } from '../utils';

const LAYOUT_OPTIONS = ['uniform', 'masonry'];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We might want to create const like we've been doing in the Modular Ionic so we can pass them to tests and the component itself. Doesn't have to happen now, but it'll have to during the migration.

Suggested change
const LAYOUT_OPTIONS = ['uniform', 'masonry'];
export const ION_GALLERY_LAYOUT_OPTIONS = ['uniform', 'masonry']as const;
export type IonGalleryLayout = (typeof ION_GALLERY_LAYOUT_OPTIONS)[number];

@brandyscarney brandyscarney Jun 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd rather just do that during the migration if that's okay since we will need to create the interfaces file and everything.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works for me!

@brandyscarney brandyscarney merged commit 9fb3990 into next Jun 12, 2026
49 checks passed
@brandyscarney brandyscarney deleted the FW-7301 branch June 12, 2026 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: angular @ionic/angular package package: core @ionic/core package package: react @ionic/react package package: vue @ionic/vue package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants