Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

domainPadding does not work as expected #472

Open
ljukas opened this issue Jan 10, 2025 · 1 comment
Open

domainPadding does not work as expected #472

ljukas opened this issue Jan 10, 2025 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@ljukas
Copy link
Contributor

ljukas commented Jan 10, 2025

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@41.14.0

What version of React and React Native are you using?
"react": "18.2.0",
"react-native": "0.74.5",

What version of Reanimated and React Native Skia are you using?
"react-native-reanimated": "3.12.1",
"@shopify/react-native-skia": "1.2.3",

Are you using Expo or React Native CLI?
Expo

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

Describe the Problem

Using domainPadding does not insert the points with the set value. Which results in making predictable bar graphs very hard.

import { printJson } from '@/utils';
import { trpc } from '@/utils/trpc';
import { memo } from 'react';
import { Bar, CartesianChart } from 'victory-native';

export const MortgageInterestChart = memo(() => {
  const [interestForecast] = trpc.mortgage.interestForecast.useSuspenseQuery();

  return (
    <CartesianChart
      data={interestForecast.data}
      xKey="year"
      yKeys={['amountPerMonth']}
    >
      {({ points, chartBounds }) => {
        return <Bar points={points.amountPerMonth} chartBounds={chartBounds} />;
      }}
    </CartesianChart>
  );
});

This very simple code renders a very basic bar graph.

Data is the following:

[
  {
    "year": 2025,
    "amountPerMonth": 20084
  },
  {
    "year": 2026,
    "amountPerMonth": 19705
  },
  {
    "year": 2027,
    "amountPerMonth": 19485
  }
]

Which results in the following points by default:

[
  {
    "x": 4,
    "xValue": 2025,
    "y": 4.726153846153842,
    "yValue": 20084
  },
  {
    "x": 165.5,
    "xValue": 2026,
    "y": 116.67692307692309,
    "yValue": 19705
  },
  {
    "x": 327,
    "xValue": 2027,
    "y": 181.66153846153844,
    "yValue": 19485
  }
]

We can se here that the first bar has its midpoint in x = 4

We now want to use barWidth together with domainPadding to make sure each bar renders within the bounds and no part of each bar is outside the rendered area (as it is by default). So we set barWidth to 50, and domainPadding left and right to 25. My logic here would be that the rendered chart is moved from the left and right side 25 "pixels" and the rendered x would now be 4 + 25 for the first point and 327 - 25 for the last point. Hence moving the graph according to the domainPadding.

However, doing this as below.

import { printJson } from '@/utils';
import { trpc } from '@/utils/trpc';
import { memo } from 'react';
import { Bar, CartesianChart } from 'victory-native';

export const MortgageInterestChart = memo(() => {
  const [interestForecast] = trpc.mortgage.interestForecast.useSuspenseQuery();

  return (
    <CartesianChart
      data={interestForecast.data}
      xKey="year"
      yKeys={['amountPerMonth']}
      domainPadding={{ left: 25, right: 25 }}
    >
      {({ points, chartBounds }) => {
        return (
          <Bar
            points={points.amountPerMonth}
            barWidth={50}
            chartBounds={chartBounds}
          />
        );
      }}
    </CartesianChart>
  );
});

Leads to the following chart data:

[
  {
    "x": 25.648793565694813,
    "xValue": 2025,
    "y": 4.726153846153842,
    "yValue": 20084
  },
  {
    "x": 165.5000000000159,
    "xValue": 2026,
    "y": 116.67692307692309,
    "yValue": 19705
  },
  {
    "x": 305.351206434337,
    "xValue": 2027,
    "y": 181.66153846153844,
    "yValue": 19485
  }
]

As seen here the first bar is not inset 4 + 25, but instead to 25.64.... This leads to some of the bar not being inside the drawn area.

This can easily be seen in the rendered graph

image

Expected behavior: [What you expect to happen]
I expect domainPadding to actually move the graph the given number

Actual behavior: [What actually happens]
DomainPadding does render the graph at the expected position instead it does some calculation internally and ends up with the incorrect position for the graphed points

@carbonrobot carbonrobot added the bug Something isn't working label Jan 10, 2025
@djm158 djm158 self-assigned this Jan 14, 2025
@trujic1000
Copy link

domainPadding seems to be messing with scrolling (panning) as well. I don't have a full example, but the issue that I had is when setting domainPadding={10} and enabled panning (on the x axis) with a preset viewport, I was only able to scroll within the viewport and not through the whole graph. After commenting out the domainPadding, everything worked as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants