BarLabel Decorator cosniders TextDirection #22
Conversation
Computing the labelX by taking into consideration the TextDirection
There was a problem hiding this comment.
@AITechNassim, Thanks so much for porting this to here!
One other suggestion, I always like to add tests if possible. It looks there there are some tests in charts_common_cf/test/chart/bar/bar_label_decorator_test.dart. I see at least two RTL tests:
test('RTL TextAnchor.start starts on the right', () {
test('RTL TextAnchor.end starts on the left', () {
Please try to enhance those tests so that the code paths you changed are tested. Ideally the tests will fail before applying these changes and succeed afterwards.
| @@ -311,12 +311,19 @@ class BarLabelDecorator<D> extends BarRendererDecorator<D> { | |||
|
|
|||
| switch (anchor) { | |||
| case BarLabelAnchor.middle: | |||
| labelX = (bounds.left + | |||
| labelElement.textDirection = rtl ? TextDirection.rtl : TextDirection.ltr; | |||
There was a problem hiding this comment.
Looking at this it looks like we can refactor line 314 to be outside of the if (calculateLabePosition == BarLabelPosition.inside) { above line 308 and then remove it below from lines 337, 340 and 346.
|
@AITechNassim I've released a new version of charts_cf, 0.10.3+cf. In this release I fixed the two bugs I ran into and created a release process. I'd love to get feed back on its current state as well as having the next release contian this PR and @dreamnice's PR #9. If either of you have feed back for me charts_cf or need help on your PR's don't hesitate to ask. |
|
@winksaville Thanks for the update. Unfortunatly, I cannot do any test before the next two weeks. I have a lot of work :( . |
|
Ok, txs for letting me know.
|
No description provided.