-
Notifications
You must be signed in to change notification settings - Fork 37
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
Replace circle chart by hoop chart #234
Replace circle chart by hoop chart #234
Conversation
Commit that adds Maui compatibility is omitted from this PR as |
Please add a link to that commit from here, for the record. |
|
|
||
|
||
type private HoopChartState = | ||
| Uninitialized |
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.
@webwarrior-ws remind us why do you need this DU member? <- @aarani pls review this bit
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.
IIRC this is because control is created before data sources are set for it.
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.
why does the circleChart not need this then?
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.
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.
Because property for default image may not be yet set
Let's set it earlier then?
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.
so why CircleChartView doesn't need Uninitialized thing?
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.
Probably because HoopChart does layout itself, and must know its size, while old CircleChartView just replaced its Content
property with either chart grid or image.
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.
IIUC A view is rendered multiple times before the parameters get fully propagated so Uninitialized state might be necessary.
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.
Cannot we have an invisible image for the logo that only turns into visible if balances are zero?
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.
Cannot we have an invisible image for the logo that only turns into visible if balances are zero?
I don't know, tbh.
Workaround for Android when certain small arcs wouldn't render. If arc lengthis very small, draw a dot instead.
Change `this` identifier to `self` to follow coding guidelines.
LGTM! 🚀 However, I encountered a problem when attempting to install it on the Android emulator using JetBrains Rider (windows):
The issue was resolved by deleting certain lines from the geewallet.sln file:
|
Fix wrong total balance label layout on GTK.
b841302
to
ccb1641
Compare
Superseded by #294 |
Addresses #172 (Better design for cheese chart (so that text is inside it)).
Replaces #209.