Draws Labels inside the circles #8
Open
Conversation
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
| Thank you for your contribution. I will check and update you on the PR by the end of the week. …On Tue, Nov 12, 2019 at 11:22 PM Giovanni De Francesco < ***@***.***> wrote: This PR enables to write the labels inside the circles instead of below. This is especially useful when a user needs to draw different numbers, such as the example below: Before: [image: Screenshot 2019-11-12 at 11 19 57] <https://user-images.githubusercontent.com/2427135/68663095-6216e400-053e-11ea-8ed1-e7a44401945c.png> After: [image: Screenshot 2019-11-12 at 11 13 54] <https://user-images.githubusercontent.com/2427135/68662946-17956780-053e-11ea-9e91-d3c3cc14c6a4.png> Note that this can be used to write inside the circles labels like "A", "B" or "C", thus giving a bit more flexibility to the library. Let me know if you don't like something and I'll fix it or if something isn't clear. Thanks a lot. ------------------------------ You can view, comment on, or merge this pull request online at: #8 Commit Summary - made possible to have labels only inside - clean up File Changes - *M* app/src/main/java/params/com/statusView/StatusView.kt <https://github.com/params-ing/StepViewAndroid/pull/8/files#diff-0> (49) - *M* app/src/main/res/layout/activity_main.xml <https://github.com/params-ing/StepViewAndroid/pull/8/files#diff-1> (5) - *M* app/src/main/res/values/attrs.xml <https://github.com/params-ing/StepViewAndroid/pull/8/files#diff-2> (3) Patch Links: - https://github.com/params-ing/StepViewAndroid/pull/8.patch - https://github.com/params-ing/StepViewAndroid/pull/8.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#8?email_source=notifications&email_token=AG4QI4H5IOBM7W5PMIBYPATQTJ7XVA5CNFSM4JMBLCB2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HYU2RZQ>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG4QI4HX5DXPWOVXIHEZLH3QTJ7XVANCNFSM4JMBLCBQ> . |
params-ing reviewed Nov 17, 2019
| * A boolean which decides if labels should be written inside | ||
| */ | ||
| | ||
| var drawLabels: Boolean by OnValidateProp(false){ |
There was a problem hiding this comment.
Actually I see you are reusing the old variable drawLabels for new functionality.
This might make sense, but on a newer version people already using the library would get unexpected views & might be confused.
Can you instead create a new variable drawStatusInsideCircle & keep the old one as it is
params-ing reviewed Nov 17, 2019
| textPaintLabel.getTextBounds(text, 0, text.length, measuringRect) | ||
| statusItemText = LabelItemText(text, textPaintLabel, circleItem.center.x, circleItem.center.y - measuringRect.exactCenterY()) | ||
| drawLabels -> { | ||
| val text: String = statusData[pos].text |
There was a problem hiding this comment.
statusData[pos].text, this might crash, can you please place a safety check
params-ing reviewed Nov 17, 2019
| app:currentCount="4" | ||
| app:drawLabels="true" | ||
| app:drawLabels="false" | ||
| app:drawCount="true" |
There was a problem hiding this comment.
Also can you check if status data is longer like
"ABCD" does it exceed the circle or looks okay.
Actually it does, so we might have to handle it
| Thanks for the review, I'll try to do make these changes next weekend. |
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.
This PR enables to write the labels inside the circles instead of below.
This is especially useful when a user needs to draw different numbers, such as the example below:
Before:
After:
Note that this can be used to write inside the circles labels like "A", "B" or "C", thus giving a bit more flexibility to the library.
Let me know if you don't like something and I'll fix it or if something isn't clear.
Thanks a lot.