feat(gallery): add new gallery component#31101
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
package: core
There was a problem hiding this comment.
I updated this because this rule prevented the images in my assets/ directory from being served on Vercel.
| // -------------------------------------------------- | ||
|
|
||
| :host(.gallery-layout-uniform) { | ||
| gap: var(--ion-gallery-gap, 16px); |
There was a problem hiding this comment.
Gap currently can only be changed by CSS. Should this be a property that can be updated based on breakpoint, like columns?
There was a problem hiding this comment.
Yes, I lean towards it especially seeing that Chakra does that. I do wonder if we should split it into row-gap and column-gap but we can cross that bridge if we get a request for it so gap works for me.
There was a problem hiding this comment.
Added support for gap here: d494fab
I think if we wanted to support column & row we could just make it accept two values rather than doing two separate properties. I don't know, something to consider later.
|
|
||
| await page.setContent( | ||
| ` | ||
| <ion-gallery style="--internal-gallery-columns: 2;"> |
There was a problem hiding this comment.
Overriding the internal CSS variable for columns should have no effect since the columns property takes priority.
| printIonWarning( | ||
| `[ion-gallery] - Invalid "columns" value (${JSON.stringify( | ||
| columns | ||
| )}). Expected a positive integer or breakpoint map object (e.g. { xs: 2, md: 4 }). Falling back to default responsive columns.`, |
There was a problem hiding this comment.
I wonder if it would be beneficial to display what the default responsive columns are so the dev can see it.
There was a problem hiding this comment.
It looks kind of messy but I can add it:
Lmk if you still want it.
There was a problem hiding this comment.
That looks good in my opinion. I'm getting the warning for <ion-gallery columns="{ xs: 2, sm: 3, md: 4, lg: 6, xl: 8, xxl: 12 }">. Is that expected for JS? It makes sense to me that I would get the error but just checking.
There was a problem hiding this comment.
Yes it doesn't accept the object as a string.
…herit aspect-ratio
There was a problem hiding this comment.
We need to figure out the file structure. We have some files that separate by periods (gallery.spec.ts) and other by hyphens (gallery-interface.ts). Currently, ionic-modular is converting the interface files to periods (gallery-interface.ts -> gallery.interface.ts)
There was a problem hiding this comment.
I think it's fine to separate them with periods. I was just following the interface naming.
| * Warn about an invalid gap value when it is set to a negative number | ||
| * or a breakpoint map object with invalid values. | ||
| */ | ||
| private warnInvalidGap(gap: GalleryGap) { |
There was a problem hiding this comment.
I was able to pass <ion-gallery gap="pink"> and I didn't get a warning.
2 participants
Issue number: internal
What is the current behavior?
Developers who want to use a masonry layout must rely on third-party solutions or implement their own.
What is the new behavior?
Adds a new
ion-gallerycomponent.Features
layoutproperty:uniform- evenly sized grid rowsmasonry- stacked masonry layoutorderproperty (masonryonly):sequential- preserves DOM orderbest-fit- places items in the shortest columncolumnsproperty:xs,sm,md,lg,xl,xxl)Test Coverage
columnsfallback and normalization caseslayoutwatcher, masonry scheduling, and load-event behaviororderplacement logic (sequentialandbest-fit)Does this introduce a breaking change?
Other information
Basic Preview
Layout Preview
Dev build:
8.8.6-dev.11777668103.132817bd