search

patrykandpatrick / vico

A light and extensible chart library for Android.
https://patrykandpatrick.com/vico/wiki
Apache License 2.0
2.08k stars 125 forks source link

Marker label position #540

Closed MessiasLima closed 7 months ago

MessiasLima commented 8 months ago

Description

This pull request allows the users to choose where the marker label should be rendered inside the Chart.

Why?

Sometimes it's a requirement from the design team to position the label of the marked in a specific place inside the chart. Mine was to show the labels close to the marker indicator.

How?

First, I created a sealed interface with two implementations to represent where we want to render the label. The Top implementation represents the previous behaviour of showing the labels always on top. The AboveIndicator, as the name suggests, represents the behaviour of rendering the label above the marker's indicator. I've added a spacingDp argument to add a bit more space between the indicator and the label. That would be useful when the user has a custom label layout or/and custom indicators and wants to fine-adjust the spacing between them.

Then, I updated the MarkerComponent to receive the label position. I extracted the current implementation of the Y position of the label and put it alongside the new position calculation separated by a when structure. This way, if someone decides to implement another position, they only need to implement a new branch.

And finally, I updated the sample application by using the new label position on the Chart3

What does that look like?

https://github.com/patrykandpatrick/vico/assets/10220064/05765188-ea90-4a78-8190-d2792b928f00

MessiasLima commented 7 months ago

@patrickmichalik Is there any other thing here that needs some change?

patrickmichalik commented 7 months ago

This is included in Vico 2.0.0 Alpha 7. I wanted to follow up and briefly explain the reason for one of the changes we made, as it may not be immediately obvious. You’ll notice that AboveIndicator#spacingDp has been removed. This is because TextComponent supports margins and padding, making the parameter in question redundant. Setting spacingDp to 4 would be equivalent to giving the label a bottom margin of 4 dp. We overlooked this while reviewing the PR. Cheers for contributing!