patrykandpatrick / vico

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

Issue with labels not showing as expected #787

Closed findjigar closed 3 months ago

findjigar commented 3 months ago

How to reproduce

val data = MutableList(size = 31) { 8f }.apply {
  set(2, 6f)
  set(12, 4f)
  set(14, 9f)
}

val labels = MutableList(size = 31) { it.plus(1).toString() }

// Use this to re-create the issue (check the preview below)
val labelsWithEmptyString = MutableList(size = 31) { "" }.apply {
  set(0, "1")
  set(4, "5")
  set(9, "10")
  set(14, "15")
  set(19, "20")
  set(24, "25")
  set(30, "31")
}

fun getFormatter(labels: List<String>) = CartesianValueFormatter { x, _, _ ->
  labels[x.toInt() % labels.size]
}

@Composable
fun MonthlyChart(formatter: CartesianValueFormatter) {
  CartesianChartHost(
    horizontalLayout = HorizontalLayout.FullWidth(
      scalableEndPaddingDp = 0f,
      scalableStartPaddingDp = 0f,
    ),
    chart = rememberCartesianChart(
      rememberColumnCartesianLayer(),
      bottomAxis = rememberBottomAxis(
        itemPlacer = remember {
          AxisItemPlacer.Horizontal.default(addExtremeLabelPadding = true)
        },
        valueFormatter = formatter,
        label = rememberAxisLabelComponent(
          textSize = 12.sp,
          padding = Dimensions.of(0.dp, 0.dp),
          margins = Dimensions.of(0.dp, 0.dp),
        ),
      ),
    ),
    model = CartesianChartModel(ColumnCartesianLayerModel.build { series(data) }),
    scrollState = rememberVicoScrollState(scrollEnabled = false),
  )
}

@Preview(widthDp = 300, heightDp = 200)
@Composable
fun MonthlyChartAllLabels() {
  PreviewSurface {
    MonthlyChart(formatter = getFormatter(labels))
  }
}

@Preview(widthDp = 300, heightDp = 200)
@Composable
fun MonthlyChartLabelsWithEmptyString() {
  PreviewSurface {
    MonthlyChart(formatter = getFormatter(labelsWithEmptyString))
  }
}

Observed behavior

Screenshot 2024-07-08 at 8 17 58 PM

Expected behavior

Expected to see the labels correctly as 1, 5, 10, 15, 20, 25, 31

Notice below image! When using a different formatter, it shows a lot more labels. So clearly there is no issue of not having enough space, but when labelsWithEmptyString is provided to the formatter than we lose labels like 10 and chart looks broken

Screenshot 2024-07-08 at 8 18 27 PM

Vico version(s)

2.0.0-alpha.22

Android version(s)

Any

Additional information

No response

findjigar commented 3 months ago

It's related to #776 but it has been marked as non-bug, so I created this issue as it seems very much a bug to me.

Gowsky commented 3 months ago

Hi @findjigar, you shouldn't control which x values are labeled by returning empty strings from the formatter. There is a difference between an empty label and no label. This control is the responsibility of AxisItemPlacer.Horizontal. Please see here for an example of how to use it for labels that are not evenly spaced. The x axis will then behave as you expect.

So clearly there is no issue of not having enough space, but when labelsWithEmptyString is provided to the formatter than we lose labels like 10 and chart looks broken

Actually, as you can see in both cases the spacing is increased to 2 for the labels to fit. The labels with empty strings probably could fit with a spacing of 1. But some assumptions are made during calculation and they only work when the API is used the proper way.

It's related to https://github.com/patrykandpatrick/vico/issues/776 but it has been marked as non-bug, so I created this issue as it seems very much a bug to me.

That issue relates to something else, and as @patrickmichalik mentioned, the suboptimal behavior described there is fixed in Vico 2.

findjigar commented 3 months ago

ok thanks. I'll check that out

diegoup2 commented 3 months ago

@Gowsky Is there a way to actually make it multi line?

Im using this: HorizontalAxis.ItemPlacer.default(spacing = 1, shiftExtremeTicks = false, addExtremeLabelPadding = false) Screenshot 2024-07-26 at 11 44 56 AM

Gowsky commented 3 months ago

@diegoup2, yes. Please see here.

diegoup2 commented 3 months ago

@Gowsky Thanks for the answer

rememberBottomAxis(
                valueFormatter = chartValueFormatter,
                label = rememberTextComponent(
                    typeface = AppTheme.GraphAxisLabel.toGraphicsTypeFace(),
                    lineCount = 2
                ),
                itemPlacer =
                remember {
                    HorizontalAxis.ItemPlacer.default(
                        spacing = 1,
                        shiftExtremeTicks = false,
                        addExtremeLabelPadding = false
                    )
                }
            )

edit: IT WORKS with label instead of titleComponent. Thanks once again