patrykandpatrick / vico

A light and extensible chart library for Android.
https://patrykandpatrick.com/vico/guide
Apache License 2.0
2.18k stars 131 forks source link

Zoom has no function for the LineChart x/y datapoints Offsets. #342

Closed Josuhu closed 1 year ago

Josuhu commented 1 year ago

BTW, super cool Chart library!

It seems the "isZoomEnabled = true" does not actually zoom the LineChart x/y datapoints Offsets but rather stretches the Canvas X scroll length causing empty scroll space. There is no scaling done for the line chart height or length itself and therefore it seems useless unless I have missed important setups from below...

Is this the actual desired zoom function implementation how it is supposed to work? The documentation is also great but contains no clues if the zoom requires more than true/false value as below.

val marker = rememberMarker(theme) val legend = rememberLegend(theme, dataPointMap) Chart( chart = lineChart( lines = dataSetLineSpecs, persistentMarkers = persistentMarkers.value ), chartModelProducer = modelProducer, startAxis = startAxis( title = "Weekly charts", maxLabelCount = 10, tickLength = 0.dp ), bottomAxis = bottomAxis( title = "Days", tickLength = 0.dp ), marker = marker, markerVisibilityChangeListener = object : MarkerVisibilityChangeListener { // Once clicked the marker is visible at all times override fun onMarkerMoved( marker: Marker, markerEntryModels: List ) { persistentMarkers.value = mapOf(markerEntryModels.first().entry.x to marker) }

    override fun onMarkerShown(
        marker: Marker,
        markerEntryModels: List<Marker.EntryModel>
    ) {
        val markerXValue = markerEntryModels.first().entry.x
        persistentMarkers.value = if (persistentMarkers.value.containsKey(markerXValue)) {
            emptyMap()
        } else { mapOf(markerXValue to marker) }
    }
},
legend = legend,
isZoomEnabled = true

)

Josuhu commented 1 year ago

Capture

patrickmichalik commented 1 year ago

Hello! This certainly isn’t the expected behavior—the line should be scaled. I’ll run your code and try to reproduce the issue as soon as I can. In the meantime, could you install the sample app and check if the issue occurs there? The APK can be found here. Also, just to confirm, what version of Vico are you using?

Josuhu commented 1 year ago

Thanks for the info, it´s good to know the real expected behaviour. Below are the dependencies in use, in the mean time I will study and try using the sample app.

// Vico charts // Includes the core logic for charts and other elements. implementation "com.patrykandpatrick.vico:core:1.8.1" // For Jetpack Compose. implementation "com.patrykandpatrick.vico:compose:1.8.1" // For compose. Creates a ChartStyle based on an M2 Material Theme. implementation "com.patrykandpatrick.vico:compose-m2:1.8.1" // For compose. Creates a ChartStyle based on an M3 Material Theme. implementation "com.patrykandpatrick.vico:compose-m3:1.8.1"

Josuhu commented 1 year ago

The demo app found in your link here works as expected while zooming and scales the Offsets. I need to review the demo source code more and compare in to my setup...

patrickmichalik commented 1 year ago

It’s worth noting that this version of the sample app uses Vico 1.9.0. I don’t believe we’ve changed anything related to zooming recently, but it might be worth checking if updating from version 1.8.1 to version 1.9.0 (or perhaps 1.9.1, which we released a moment ago) changes anything.

Josuhu commented 1 year ago

Tried with latest version 1.9.1 and no change. There is something preventing the line scaling at this point which I cannot figure out for now.

Thanks for the help so far.

patrickmichalik commented 1 year ago

Would you be able to provide a recording of the incorrect behavior, @Josuhu?

Josuhu commented 1 year ago

https://github.com/patrykandpatrick/vico/assets/55630343/e66dc63a-694e-4fad-bbd9-4a29eb76237e

Here is the zooming event recorded.

patrickmichalik commented 1 year ago

Thank you! I think I know what’s going on. We’ll look into it.

patrickmichalik commented 1 year ago

Vico 1.11.0 fixes this. Cheers!