[lit-virtualizer] Fixes #4505 - Adds documentation for lit-virtualizer/grid #4543
Conversation
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
📊 Tachometer Benchmark ResultsSummaryA summary of the benchmark results will show here once they finish. ResultsThe full results of your benchmarks will show here once they finish. |
There was a problem hiding this comment.
Thanks @JamesIves!!
A couple of small comments from me, but otherwise looks good. @graynorton can you take a quick pass?
packages/labs/virtualizer/README.md
Outdated
|
|
||
| The `gap` property determines spacing between items and can be either a single pixel value such as `8px` (the default), or by applying both a vertical and horizontal pair such as `8px 0`. | ||
|
|
||
| Additionally `auto` can be used to split available space evenly between items, however this may only make sense for inline axis. |
There was a problem hiding this comment.
Regarding "however this may only make sense for inline axis" does auto even do anything for the block axis? If not, I'd say "however this only works for the inline axis". If it does do something for the block axis it'd be good to say what that is.
There was a problem hiding this comment.
Tried to check on this, but I am running into some issues. I based this particular example on the playground, but it seems like the auto property may be bugged, or I'm just misunderstanding how it works.
render() {
return html`
<lit-virtualizer
.layout=${grid({
itemSize: {width: '100px', height: '100px'},
gap: '0 auto',
justify: 'space-between'
})}
.items=${this.photos}
.renderItem=${photos => html`<img src=${photo.url}>`}
></lit-virtualizer>
`;
}The following produces this error: Error: grid layout: column-gap cannot be set to 'auto' when direction is set to vertical, but from what I can tell direction doesn't actually work for a grid layout, as changing it at all causes the virtualizer to disappear.
Perhaps I should omit documenting the auto property for now? Regardless I've added some additional context surrounding the justify requirement which was missing before.
packages/labs/virtualizer/README.md
Outdated
|
|
||
| #### padding | ||
|
|
||
| The `padding` property determines the spaces around the edges and can either be a single value pair such as `8px 16px`, or four values to specify top, right, bottom, and left independently, for example `8px 12px 16px 24px`. |
There was a problem hiding this comment.
"around the edges" around the edges of items? If so I'd say that, and if possible clarify the difference between padding and gap. Maybe "around each item" would be enough to imply the difference and most people should be familiar with the concepts from CSS?
|
Bumpity bump @graynorton |
Adds documentation for
lit-virtualizergridlayout.Documentation was produced using examples from the Playground / code. Please let me know if you feel I need to go into more detail in any specific area.
Closes #4505