Skip to content

optionally render stacked bar chart values with text color that match…#3809

Open
mtotschnig wants to merge 8 commits intoPhilJay:masterfrom
mtotschnig:StackedBarCharValueTextColor
Open

optionally render stacked bar chart values with text color that match…#3809
mtotschnig wants to merge 8 commits intoPhilJay:masterfrom
mtotschnig:StackedBarCharValueTextColor

Conversation

@mtotschnig
Copy link
Copy Markdown

…es data color

fixes #2460

@devkai666
Copy link
Copy Markdown

Please review this commit and merge to fix the value text colors as soon as possible, we need this.

almic
almic previously requested changes Apr 29, 2018
Copy link
Copy Markdown
Contributor

@almic almic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable names should align with the current code style, starting with the letter 'm'.

You may also consider changing the name to something shorter and easier to type, like setValuesUseBarColor().

Also, there is no need to create the getStackTextColors(), that exact method already exists in BaseDataSet: dataSet.getColors() And the boolean variable inside the new method can just be called enabled, not the name of the function.

Last thing, the partner function isValuesUseBarColor() must exist, and return the boolean. That is how you should check if the option is enabled. As well, the option needs to be false by default, I suggest explicitly writing that into the member declaration.

@almic almic dismissed their stale review May 1, 2018 15:07

updating review

@almic almic self-assigned this May 1, 2018
@almic almic added the enhancement improves upon existing functionality label May 1, 2018
…names, reuse already defined getColors method, add getter
@mtotschnig
Copy link
Copy Markdown
Author

@almic I have taken into account all your suggestions, with the exception of "And the boolean variable inside the new method can just be called enabled, not the name of the function.", which I do not understand.

…ial NPEs we pull the decision up into the interface
# Conflicts:
#	MPChartLib/src/main/java/com/github/mikephil/charting/data/BarDataSet.java
#	MPChartLib/src/main/java/com/github/mikephil/charting/renderer/BarChartRenderer.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement improves upon existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

How to set text color of bar value as bar color for Stacked BarChart in MPChart

3 participants