[ABW-2213] Manually Calculate Item Limit #729
Merged
Conversation
This file contains 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
maciek-rdx
reviewed
Sep 7, 2023
|
|
||
| FungibleResourcesSection(fungibles: viewStore.fungibleResourceIcons, itemLimit: limit) | ||
|
|
||
| // FIXME: ViewThatFits is better, but it causes issues with @MainActor, which probably shouldn't be used in the first place |
There was a problem hiding this comment.
Which @MainActor are you referring to @kugel3 ?
There was a problem hiding this comment.
all of our MainActors all over the place
| // FIXME: Workaround to avoid ViewThatFits | ||
| let limit = viewStore.state.itemLimit( | ||
| iconSize: Constants.iconSize.rawValue, | ||
| width: proxy.size.width |
There was a problem hiding this comment.
geometry is a more common name, but fine for now
Comment on lines
+260
to
+296
| func itemLimit(trying limits: [Int?], iconSize: CGFloat, width: CGFloat) -> Int? { | ||
| for limit in limits { | ||
| if usedWidth(itemLimit: limit, iconSize: iconSize) < width { | ||
| return limit | ||
| } | ||
| } | ||
|
|
||
| return 3 | ||
| } | ||
|
|
||
| func usedWidth(itemLimit: Int?, iconSize: CGFloat) -> CGFloat { | ||
| let items = min(fungibleResourceIcons.count, itemLimit ?? .max) | ||
| let showFungibleLabel = fungibleResourceIcons.count > items | ||
|
|
||
| let hasItems = items > 0 | ||
| let hasPoolUnits = poolUnitsCount > 0 | ||
| let hasNFTs = nonFungibleResourcesCount > 0 | ||
| let sections = (hasItems ? 1 : 0) + (hasPoolUnits ? 1 : 0) + (hasNFTs ? 1 : 0) | ||
|
|
||
| var width: CGFloat = 0 | ||
|
|
||
| if hasItems { | ||
| let extraWidth = 2 * iconSize / 3 | ||
| width += iconSize + CGFloat(items - 1) * extraWidth | ||
| } | ||
| if showFungibleLabel { | ||
| width += iconSize | ||
| } | ||
| if hasPoolUnits { | ||
| width += iconSize + .medium1 | ||
| } | ||
| if hasNFTs { | ||
| width += iconSize + .medium1 | ||
| } | ||
|
|
||
| return width + max(CGFloat(sections - 1) * .small1, 0) | ||
| } |
There was a problem hiding this comment.
Can be private or even nested in the itemLimit method
| @@ -241,6 +251,51 @@ extension AccountList.Row.View { | |||
| } | |||
| } | |||
|
|
|||
| // FIXME: Workaround to avoid ViewThatFits | |||
| extension AccountList.Row.ViewState { | |||
| func itemLimit(iconSize: CGFloat, width: CGFloat) -> Int? { | |||
There was a problem hiding this comment.
I'd be a bit more explicit with naming here - it's about fingible resources, right?
maciek-rdx
added
the
DO NOT MERGE
CyonAlexRDX
approved these changes
Sep 7, 2023
There was a problem hiding this comment.
I think the calculations looks right, let @umair-rdx help with testing
|
Yep, it's 1.0.0#13-Pre-Alpha @umair-rdx |
maciek-rdx
approved these changes
Sep 7, 2023
|
There is a subtle issue when we have more resources, but fine for now |
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Jira ticket: ABW-2213
Description
Calculates the number of fungible icons to display manually, instead of using ViewThatFits, thereby avoiding data race warnings (and presumably potential issues)
Screenshot