diogobernardino / williamchart

Android Library to rapidly develop attractive and insightful charts in android applications.
5.1k stars 800 forks source link

#253 Allow modification of x and y labels #257

Closed ubuntudroid closed 5 years ago

ubuntudroid commented 5 years ago

Fixes #253

The following screenshot shows the change in action for the line chart (note, that the y labels are actually rounded values, I've added this to the demo code as well) and the horizontal bar chart (I've added a random "h" to the end of the label, this is not part of the demo code):

device-2019-10-21-174956

ubuntudroid commented 5 years ago

@diogobernardino I just found that dev is not up-to-date with master. Intentionally?

diogobernardino commented 5 years ago

@diogobernardino I just found that dev is not up-to-date with master. Intentionally?

Now it's synced. :)

ubuntudroid commented 5 years ago

@diogobernardino RE formatting of x values: I like to see the data you add in the LinkedMap just as plain data. Formatting it for display as x and y labels should in my opinion be handled separately in the newly introduced labeling functions. Separation of concerns if you will. :)

ubuntudroid commented 5 years ago

@diogobernardino I've synced with your dev and fixed the format issues. Let me know if there is anything else that needs to be changed.

diogobernardino commented 5 years ago

Ignore my last review, I missed your comment to it.

diogobernardino commented 5 years ago

Formatting it for display as x and y labels should in my opinion be handled separately in the newly introduced labeling functions.

Your definition of xScaleLabels is val xScaleLabel: (String) -> String, formatting a String into a String. What would be the usecase to benefit from it?

ubuntudroid commented 5 years ago

Formatting it for display as x and y labels should in my opinion be handled separately in the newly introduced labeling functions.

Your definition of xScaleLabels is val xScaleLabel: (String) -> String, formatting a String into a String. What would be the usecase to benefit from it?

The benefit in my opinion would be that the data defined for the chart to display can remain unchanged while the label can be changed to one's heart's content.

So imagine your data looks like this: ("21.12.2019", 20.4f), ("10.10.2020", 14.1f), ... Then you can just pump this data into the chart, but modify the actually displayed label - e.g. by adding something to the x-value: "on 21.12.2019", "on 10.10.2020".

In the future I would like be able to input any kind of data for the x value (e.g. an actual Date object) and then have the formatter function take care of converting it to a proper string representation. This would be merely a first step on the road to such a feature.

If you feel it doesn't make much sense I can of course remove the formatter function for x from this PR.

diogobernardino commented 5 years ago

In the future I would like be able to input any kind of data for the x value (e.g. an actual Date object) and then have the formatter function take care of converting it to a proper string representation. This would be merely a first step on the road to such a feature.

If I correctly understand the usecase, you can also apply that same formatter before passing labels to the chart, you don't depend on any chart input to perform that operation (different than yScaleLabels where you need the y values). My goal is to provide the essentials to enable developers to build their charts, leaving the specifics to be done outside the library, and thus keeping the library small and easy to grasp.

ubuntudroid commented 5 years ago

@diogobernardino got your point. Will remove the changes to the x axis next week. 👍

ubuntudroid commented 5 years ago

@diogobernardino I've removed the x-label modification feature, merged with dev and added scale modification to the line chart in the demo to prove compatibility with this feature.

ubuntudroid commented 5 years ago

@diogobernardino I've fixed the conflicts.