Changed comment that referenced getFormattedValue() method in IValueFormatter #3518
Conversation
IValueFormatter had a comment suggesting that you can override the getFormattedValue(…) method but that method doesn’t exist. The comment has been removed.
Adding back the comment but revised it to refer to the stringForValue() method in the IValueFormatter protocol. I know, I know, it should have been obvious but @philfi had to point it out to me.
Codecov Report
@@ Coverage Diff @@
## master #3518 +/- ##
=======================================
Coverage 29.26% 29.26%
=======================================
Files 117 117
Lines 13277 13277
=======================================
Hits 3886 3886
Misses 9391 9391Continue to review full report at Codecov.
|
| @@ -24,12 +24,10 @@ public protocol IValueFormatter: class | |||
| /// | |||
| /// For performance reasons, avoid excessive calculations and memory allocations inside this method. | |||
| /// | |||
| /// - returns: The formatted label ready for being drawn | |||
| /// - returns: The formatted label ready to be drawn | |||
| /// | ||
| /// - parameter value: The value to be formatted | ||
| /// | ||
| /// - parameter axis: The entry the value belongs to - in e.g. BarChart, this is of class BarEntry |
There was a problem hiding this comment.
please add correct parameter here
There was a problem hiding this comment.
I believe 'axis' was a parameter for getFormattedValue but it's not a parameter for stringForValue. This is why I removed it.
There was a problem hiding this comment.
But I will add something for the 'entry' parameter.
| /// | ||
| /// Then override the getFormattedValue(...) method and return whatever you want. | ||
| /// Simply create your own formatting class and let it implement ValueFormatter. Then override the stringForValue() | ||
| /// method and return whatever you want. |
There was a problem hiding this comment.
I'm not sure if this is github display issue or your code. /// method and return whatever you want. in a new line is strange to me
|
@liuxuan30 - Sorry but I'm new to this. Do I push the changes to the existing PR or create a new PR? |
|
@JCMcLovin sorry to be late. You just push to your branch |
|
@liuxuan30 this good to merge? |
|
@petester42 I need to push a very minor change now that I know I should just push to my branch (my first PR). I will do this as soon as I’m back at my computer. |
|
I've pushed the final change. Good to merge. |
|
You sure you pushed to the correct branch? Last commit is from June. |
|
Yeah, it’s the latest. It took a long time to get an answer about if I just push the new commit or create a new PR and then I dragged my feet for a while after I got the answer. |
|
Thanks 👍 |
Updated the documentation markup in the IValueFormatter protocol by changing the reference to getFormattedValue() to the newer stringForValue() method.