-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Updating gallery layout to use Robs distribution logic from v2 #2289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2709627
updating gallery layout to use Robs distribution logic from v2
LFDanLu 1bb748a
Merge branch 'cardview_followup' of github.com:adobe/react-spectrum i…
LFDanLu a8aa4f6
force gallery images to match minItem width even if space alotted to …
LFDanLu 565e203
partial fix for min item height
LFDanLu 648aa81
Merge branch 'cardview_followup' of github.com:adobe/react-spectrum i…
LFDanLu 77f0c3f
Merge branch 'cardview_followup' of github.com:adobe/react-spectrum i…
LFDanLu 7ff62e2
Merge branch 'cardview_followup' of github.com:adobe/react-spectrum i…
LFDanLu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ export interface GalleryLayoutOptions extends BaseLayoutOptions { | |
| // */ | ||
| // cardSize?: 'S' | 'M' | 'L', | ||
| /** | ||
| * The the default row height. | ||
| * The the default row height. Note this must be larger than the min item height. | ||
| * @default 208 | ||
| */ | ||
| idealRowHeight?: number, | ||
|
|
@@ -33,7 +33,18 @@ export interface GalleryLayoutOptions extends BaseLayoutOptions { | |
| * The vertical padding for an item. | ||
| * @default 114 | ||
| */ | ||
| itemPadding?: Size, | ||
| itemPadding?: number, | ||
| /** | ||
| * Minimum size for a item in the grid. | ||
| * @default 136 x 136 | ||
| */ | ||
| minItemSize?: Size, | ||
| /** | ||
| * Target for adding extra weight to elements during linear partitioning. Anything with an aspect ratio smaler than this value | ||
| * will be targeted. | ||
| * @type {number} | ||
| */ | ||
| threshold?: number, | ||
| /** | ||
| * The margin around the grid view between the edges and the items. | ||
| * @default 24 | ||
|
|
@@ -69,6 +80,8 @@ export class GalleryLayout<T> extends BaseLayout<T> { | |
| protected margin: number; | ||
| protected itemSpacing: Size; | ||
| protected itemPadding: number; | ||
| protected minItemSize: Size; | ||
| protected threshold: number; | ||
|
|
||
| constructor(options: GalleryLayoutOptions = {}) { | ||
| super(options); | ||
|
|
@@ -77,13 +90,57 @@ export class GalleryLayout<T> extends BaseLayout<T> { | |
| this.idealRowHeight = options.idealRowHeight || DEFAULT_OPTIONS[cardSize].idealRowHeight; | ||
| this.itemSpacing = options.itemSpacing || DEFAULT_OPTIONS[cardSize].itemSpacing; | ||
| this.itemPadding = options.itemPadding != null ? options.itemPadding : DEFAULT_OPTIONS[cardSize].itemPadding; | ||
| this.minItemSize = options.minItemSize || DEFAULT_OPTIONS[cardSize].minItemSize; | ||
| this.threshold = options.threshold || 1; | ||
| this.margin = options.margin != null ? options.margin : DEFAULT_OPTIONS[cardSize].margin; | ||
| } | ||
|
|
||
| get layoutType() { | ||
| return 'gallery'; | ||
| } | ||
|
|
||
| /** | ||
| * Takes a row of widths and if there are any widths smaller than the min-width, leech width starting from | ||
| * the widest in the row until it can't give anymore, then move to the second widest and so forth. | ||
| * Do this until all assets meet the min-width. | ||
| * */ | ||
| _distributeWidths(widths) { | ||
| // create a copy of the widths array and sort it largest to smallest | ||
| let sortedWidths = widths.concat().sort((a, b) => a[1] > b[1] ? -1 : 1); | ||
| for (let width of widths) { | ||
| // for each width, if it's smaller than the min width | ||
| if (width[1] < this.minItemSize.width) { | ||
| // then figure out how much smaller | ||
| let delta = this.minItemSize.width - width[1]; | ||
| for (let item of sortedWidths) { | ||
| // go from the largest width in the row to the smallest | ||
| // if the width is greater than the min width | ||
| if (widths[item[0]][1] > this.minItemSize.width) { | ||
| // subtract the delta from the width, if it's still greater than the min width | ||
| // then we have finished, subtract the delta permanently from that width | ||
| if (widths[item[0]][1] - delta > this.minItemSize.width) { | ||
| widths[item[0]][1] -= delta; | ||
| delta = 0; | ||
| break; | ||
| } else { | ||
| // otherwise, we take as much as we can from the current width and then move on to | ||
| // the next largest and take some width from it | ||
| let maxChange = widths[item[0]][1] - this.minItemSize.width; | ||
| delta -= maxChange; | ||
| widths[item[0]][1] -= maxChange; | ||
| } | ||
| } | ||
| } | ||
| if (delta > 0) { | ||
| return false; | ||
| } | ||
| // force the width to be the min width that we just rebalanced for | ||
| width[1] = this.minItemSize.width; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| validate() { | ||
| this.collection = this.virtualizer.collection as GridCollection<T>; | ||
| this.buildCollection(); | ||
|
|
@@ -117,16 +174,35 @@ export class GalleryLayout<T> extends BaseLayout<T> { | |
| // Compute aspect ratios for all of the items, and the total width if all items were on in a single row. | ||
| let ratios = []; | ||
| let totalWidth = 0; | ||
| let minRatio = this.minItemSize.width / this.minItemSize.height; | ||
| let maxRatio = availableWidth / this.minItemSize.height; | ||
|
|
||
| for (let node of this.collection) { | ||
| let ratio = node.props.width / node.props.height; | ||
| if (ratio < minRatio) { | ||
| ratio = minRatio; | ||
| } else if (ratio > maxRatio && ratio !== minRatio) { | ||
| ratio = maxRatio; | ||
| } | ||
|
|
||
| let itemWidth = ratio * this.minItemSize.height; | ||
| ratios.push(ratio); | ||
| totalWidth += ratio * this.idealRowHeight; | ||
| totalWidth += itemWidth; | ||
| } | ||
|
|
||
| totalWidth += this.itemSpacing.width * (this.collection.size - 1); | ||
|
|
||
| // Determine how many rows we'll need, and partition the items into rows | ||
| // using the aspect ratios as weights. | ||
| let rows = Math.max(1, Math.round(totalWidth / availableWidth)); | ||
| let partition = linearPartition(ratios, rows); | ||
| let rows = Math.max(1, Math.ceil(totalWidth / availableWidth)); | ||
| // if the available width can't hold two items, then every item will get its own row | ||
| // this leads to a faster run through linear partition and more dependable output for small row widths | ||
| if (availableWidth <= (this.minItemSize.width * 2) + (this.itemPadding * 2)) { | ||
| rows = this.collection.size; | ||
| } | ||
|
|
||
| let weightedRatios = ratios.map(ratio => ratio < this.threshold ? ratio + (0.5 * (1 / ratio)) : ratio); | ||
| let partition = linearPartition(weightedRatios, rows); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This partitioning doesn't quite respect the minItemSize.height provided, will need to dig into how the linearPartition works. May require a refactor of these calculations but that can be done after this alpha |
||
|
|
||
| let index = 0; | ||
| for (let row of partition) { | ||
|
|
@@ -137,18 +213,29 @@ export class GalleryLayout<T> extends BaseLayout<T> { | |
| } | ||
|
|
||
| // Determine the row height based on the total available width and weight of this row. | ||
| let rowHeight = (availableWidth - (row.length - 1) * this.itemSpacing.width) / totalWeight; | ||
| if (row === partition[partition.length - 1] && rowHeight > this.idealRowHeight * 2) { | ||
| rowHeight = this.idealRowHeight; | ||
| } | ||
| let bestRowHeight = (availableWidth - (row.length - 1) * this.itemSpacing.width) / totalWeight; | ||
|
|
||
| let itemHeight = Math.round(rowHeight) + this.itemPadding; | ||
| // if this is the last row and the row height is >2x the ideal row height, then cap to the ideal height | ||
| // probably doing this because if the last row has one extremely tall image, then the row becomes huge | ||
| // though that can happen anywhere if a row has lots of tall images... so i'm not sure why this one matters | ||
| if (row === partition[partition.length - 1] && bestRowHeight > this.idealRowHeight * 2) { | ||
| bestRowHeight = this.idealRowHeight; | ||
| } | ||
| let itemHeight = Math.round(bestRowHeight) + this.itemPadding; | ||
| let x = this.margin; | ||
|
|
||
| // if any items are going to end up too small, add a bit of width to them and subtract it from wider objects | ||
| let widths = []; | ||
| for (let j = index; j < index + row.length; j++) { | ||
| let width = Math.round(bestRowHeight * ratios[j]); | ||
| widths.push([j - index, width]); | ||
| } | ||
| this._distributeWidths(widths); | ||
|
|
||
| // Create items for this row. | ||
| for (let j = index; j < index + row.length; j++) { | ||
| let node = this.collection.rows[j]; | ||
| let itemWidth = Math.round(rowHeight * ratios[j]); | ||
| let itemWidth = Math.max(widths[j - index][1], this.minItemSize.width); | ||
| let rect = new Rect(x, y, itemWidth, itemHeight); | ||
| let layoutInfo = new LayoutInfo(node.type, node.key, rect); | ||
| this.layoutInfos.set(node.key, layoutInfo); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this still has a bit of an issue where it is possible for the item's height for extremely tall images to be a bit taller than its row peers:

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well that doesn't seem good, lets sit down and go through the math together sometime