-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add text field to range annotations #1865
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
base: main
Are you sure you want to change the base?
Conversation
4d95309 to
51ef6bd
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1865 +/- ##
==========================================
- Coverage 92.51% 92.21% -0.31%
==========================================
Files 46 46
Lines 3341 3366 +25
==========================================
+ Hits 3091 3104 +13
- Misses 250 262 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi, thanks for the update. Functionality seems to work well! Good job. |
| final TextStyle style; | ||
|
|
||
| /// Text rotation in degrees | ||
| final double? rotation; |
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.
I think it's better to specifically mention that it is related to the text in it's name.
So I would go with textRotation (the same for textStyle)
| final double? rotation; | ||
|
|
||
| /// Text text padding | ||
| final double horizontalPadding; |
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.
Also, padding is not what we have here. I think we can use offset name here.
And I would define a single property final Offset textOffset for both horizontal and vertical variables (dx and dy)
| final double verticalPadding; | ||
|
|
||
| /// Lerps a [HorizontalRangeAnnotation] based on [t] value, check [Tween.lerp]. | ||
| static HorizontalRangeAnnotation lerp( |
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.
And please don't forget to add the new properties in the lerp() function as well.
It helps us to have transition animation for these added properties!
|
And my last comment: |



We can use something like this to add text for horizontal and vertical range annotations.
Refrering to: #522.
Usage: