docs(gallery): add documentation for new gallery component#4484
docs(gallery): add documentation for new gallery component#4484brandyscarney wants to merge 19 commits intomajor-9.0from
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
I chose to add this under "Grids" because otherwise we would need to redo the component page to either add a new image for a top-level Gallery section or to add something like "Layout" which would reorder a few of them. I can change this if we want to categorize this differently.
| } | ||
|
|
||
| let npmTag = 'latest'; | ||
| let npmTag = '8.8.6-dev.11777569278.1613db2e'; |
There was a problem hiding this comment.
We need to actually merge this with a dev build otherwise the docs build will fail since Gallery only exists on the dev build.
There was a problem hiding this comment.
I don't love this location of images used by the demos but other demos use this as well so I followed the existing structure.
There was a problem hiding this comment.
Do have a preferred folder structure that you would like to use?
There was a problem hiding this comment.
I would prefer to keep the images under the component's folder like this:
static
└── usage
└── v9
└── gallery
└── assets
|── 01.png
|── 02.png
|── 03.png
|── ...
|── 10.png
|── 11.png
└── 12.png
But I am not going to try to refactor how this is currently working.
| </ion-header> | ||
| <ion-content class="ion-padding"> | ||
| <ion-gallery> | ||
| <img src="https://ionicframework.com/docs/img/demos/gallery/01.png" alt="Image 1" /> |
There was a problem hiding this comment.
I am following how other demos include images because the only other way to pass them to StackBlitz is using base64 encoding.
ShaneK
left a comment
There was a problem hiding this comment.
This looks so good, Brandy! Awesome work!!
thetaPC
left a comment
There was a problem hiding this comment.
LGTM, with minor requested changes
| <div>6</div> | ||
| </ion-gallery> | ||
|
|
||
| <style> |
There was a problem hiding this comment.
Thoughts on including a comment stating that the styles are for demo only and not needed to get gallery working? The same for the other frameworks.
| In masonry layouts, top-level `img` elements are given default styles to ensure consistent rendering. These styles make images fill their container while preserving their aspect ratio and keeping them centered. | ||
|
|
||
| :::note | ||
| Images wrapped in other elements (for example, inside a `figure`) do not inherit these defaults. Apply the same styles to the nested `img` if you want matching behavior. |
There was a problem hiding this comment.
Could we point them to a permalink of the styles needed on the Ionic Framework repo?
There was a problem hiding this comment.
I don't think this is a good idea because if we update those styles in the future then they will be out of sync here. I included a code block to make it simpler to copy/paste: c2a85bb
| ``` | ||
|
|
||
| ## Properties | ||
| <Props /> |
There was a problem hiding this comment.
The default value for columns did not render as expected. It just says DEFAULT_COLUMNS. Not sure if that's something we can fix easily or if it should be handle through another PR.
There was a problem hiding this comment.
Do have a preferred folder structure that you would like to use?
| label: 'Grids', | ||
| collapsed: false, | ||
| items: ['api/grid', 'api/col', 'api/row'], | ||
| items: ['api/grid', 'api/col', 'api/row', 'api/gallery'], |
There was a problem hiding this comment.
Is it intentional to not have it alphabetical?
There was a problem hiding this comment.
Yes because the main documentation (including playgrounds) for grid is in ion-grid and we always want to highlight the main documentation first. Grid is a bit of a weird one because it doesn't prefix all of its components with ion-grid-* like other components (card, fab, item, etc) do:
If it did this would look less weird. But when you navigate from the components page it would look even weirder to land in the middle here:
|
@brandyscarney I did notice that Firefox doesn't load the images even after changing the URL. I assume that it's something out of our scope but just wanted to let you know as a heads up.
|
|
@thetaPC I confirmed that the same issue happens on the Cards example on Firefox so this seems to be a sitewide issue. |

Adds documentation & playgrounds for the new
ion-gallerycomponent.Preview
Note: when opening the basic usage or masonry images examples in StackBlitz the img URL must be changed from
https://ionicframework.com/tohttps://ionic-docs-git-fw-7287-ionic1.vercel.appto load the images.