FormidableLabs / victory-native-xl

A charting library for React Native with a focus on performance and customization.
https://commerce.nearform.com/open-source/victory-native
753 stars 54 forks source link

formatXLabel will pass an undefined label on Samsung devices #185

Open a-arshad opened 10 months ago

a-arshad commented 10 months ago

Prerequisites

Describe Your Environment

What version of victory-native-xl are you using? (can be found by running npm list --depth 0 victory-native)

victory-native@40.0.3

What version of React and React Native are you using?

react@18.2.0 react-native@0.72.8

What version of Reanimated and React Native Skia are you using?

react-native-reanimated@3.6.1 @shopify/react-native-skia@0.1.230

Are you using Expo or React Native CLI?

React Native CLI

What platform are you on? (e.g., iOS, Android)

Android

Describe the Problem

We found that on Samsung Galaxy devices with their font size turned to the max within their accessibility settings, formatXLabel within CartesianAxis.tsx will pass an undefined value as a label.

Expected behavior: [What you expect to happen]

A valid label string will be passed into the label parameter of the formatter.

Actual behavior: [What actually happens]

An undefined label is passed into the label parameter of the formatter.

Additional Information

After our own investigations we see that the error happens around line 144 of CartesianAxis.tsx

  const xAxisNodes = xScale.ticks(xTicks).map((tick) => {
    const val = isNumericalData ? tick : ix[tick];
    const contentX = formatXLabel(val as never);
    ...
  }

Our undefined value comes from the last tick being out of bounds on ix. There seems to be an issue with the number of ticks becomes larger than the actual available x metric values. Possibly when accessibility settings are changed our ticks become out of sync with what our data actually represents.

akmere commented 9 months ago

I have the same issue on a standard Pixel 3a (API 34) Android emulator. It occurs when I'm adding some domain padding, undefined value will be passed then to formatXLabel.

masiddee commented 9 months ago

Hey @a-arshad, thanks for sharing! I noticed that you're using react-native-reanimated@3.6.1, however the current example app uses v3.3.0. And I'm not seeing this issue when testing on my Android Emulator using the example app.

I wonder if there may have been some changes introduced in between that may be causing the issues you're seeing. Could you try installing react-native-reanimated@3.3.0 and rebuilding your app to see if the issue persists?

Also, @akmere can you confirm your environment as well?

kitolog commented 8 months ago

Hey Same here, I'm running with the "react-native-reanimated": "3.7.0" "victory-native": "40.0.4"

and my X-axis provides undefined labels. The domainPadding setting doesn't affect the undefined labels

zibs commented 7 months ago

If anyone here wants to provide some sample data and the chart/code they were using here, I can take a deeper look! Thanks!

jonnyggao commented 5 months ago

I have encountered a similar issue on iOS where the last tick cannot be displayed. In my case I was attempting to only display the only the first and last ticks on the x-axis. I believe it is due to the same issue.

I have recreated the bug on the demo app in line-chart.tsx. Simply change tickCount to tickCount: { x: data.length, y: yTickCount } here

Now if you type anything in "X Label Text" then the label for the last tick would disappear. See video for reference.

https://github.com/FormidableLabs/victory-native-xl/assets/32176865/0c24f312-bfda-4ae0-a6cc-261184a9b29f

jonnyggao commented 5 months ago

@zibs bump 👊

psam44 commented 3 months ago

@jonnyggao Got a similar non appearance of the last x-label, that I resolved by giving more space with a higher value of domainPadding/right. victory185