Merged
Conversation
samholmes
requested changes
Feb 13, 2025
samholmes
reviewed
Feb 13, 2025
|
|
||
| // A delayed fadein for the max/min labels, to ensure the labels don't get | ||
| // rendered before the price line. Also hidden when gesture is active | ||
| const minPriceLabelY = Platform.OS === 'ios' ? chartHeight - theme.rem(2.5) : chartHeight - theme.rem(2.75) |
Contributor
There was a problem hiding this comment.
This logic needs comment.
samholmes
approved these changes
Feb 13, 2025
swansontec
force-pushed
the
william/wallet-search-performance
branch
2 times, most recently
from
0b6528d to
61ad9bc
Compare
samholmes
approved these changes
Feb 13, 2025
The react-native-slide-charts library defaults to the phone width. Since we place the chart into our layout hierarchy, we need to override this default.
Since we only show the first 5 transactions, we don't need to ask the core for more when we reach the end of the scroll. The core's initial batch of 10 transactions will be perfectly fine.
We never actually render any content in this list, so we could just use a normal scroll view. Also, fix the bottom area to have a minimum height, so viewing & searching transactions is less glitchy.
swansontec
force-pushed
the
william/wallet-search-performance
branch
from
61ad9bc to
89e8021
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The first commit fixes the hang - something is wrong with the swipe chart, but we can work around that by providing an explicit width.
The next two commits are necessary cleanups.
The final commit fixes some of the UX issues I was experiencing when using this scene.
We really only need the first commit, but the next 3 are "nice to have".
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: