Skip to content

Commit

Permalink
Fix victory-native/victory-zoom-container clipping bug (#3026)
Browse files Browse the repository at this point in the history
Issue summary:

Previously, victory-native/victory-zoom-container spread the result of
`useVictoryZoomContainer` into `VictoryContainer`

```ts
const props = useVictoryZoomContainer(...)
return <VictoryContainer {...props} />;
```

Where the return type of `useVictoryZoomContainer` is

```
{
  props: VictoryZoomContainerMutatedProps;
  children: React.ReactElement[];
}
```

Because `VictoryContainer` received the entire `props` object as a prop, instead of spreading its
properties, values such as `width` and `height` were `undefined` - leading to the viewbox for the svg
element to render as `<svg viewBox="0 0 undefined undefined" ...>` thus clipping the rendered chart.

Solution:

Destructure the props/children result from `useVictoryContainer` and pass correctly to
`VictoryContainer`
  • Loading branch information
djm158 authored Jan 14, 2025
1 parent cd0f963 commit 1c8217d
Show file tree
Hide file tree
Showing 6 changed files with 312 additions and 209 deletions.
5 changes: 5 additions & 0 deletions .changeset/fifty-pears-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"victory-native": patch
---

Fixes clipping/loss of interactivity bug in victory-zoom-container
6 changes: 4 additions & 2 deletions demo/rn/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"eject": "expo eject"
},
"dependencies": {
"@expo/metro-runtime": "~3.1.3",
"@react-navigation/native": "^6.1.17",
"@react-navigation/native-stack": "^6.9.26",
"expo": "~50.0.17",
Expand All @@ -23,6 +24,7 @@
"react-native-safe-area-context": "4.10.1",
"react-native-screens": "~3.31.1",
"react-native-svg": "14.1.0",
"react-native-web": "~0.19.6",
"victory": "workspace:*",
"victory-area": "workspace:*",
"victory-axis": "workspace:*",
Expand All @@ -40,6 +42,7 @@
"victory-histogram": "workspace:*",
"victory-legend": "workspace:*",
"victory-line": "workspace:*",
"victory-native": "workspace:*",
"victory-pie": "workspace:*",
"victory-polar-axis": "workspace:*",
"victory-scatter": "workspace:*",
Expand All @@ -49,8 +52,7 @@
"victory-tooltip": "workspace:*",
"victory-voronoi": "workspace:*",
"victory-voronoi-container": "workspace:*",
"victory-zoom-container": "workspace:*",
"victory-native": "workspace:*"
"victory-zoom-container": "workspace:*"
},
"devDependencies": {
"@babel/core": "^7.20.0",
Expand Down
15 changes: 14 additions & 1 deletion demo/rn/src/screens/chart-screen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import {
VictoryArea,
VictoryStack,
VictoryTooltip,
VictoryZoomContainer,
} from "victory-native";
import viewStyles from "../styles/view-styles";
import { getTransitionData } from "../data";
import { getTransitionData, generateRandomData } from "../data";

export const ChartScreen: React.FC = () => {
const [transitionData, setTransitionData] =
Expand Down Expand Up @@ -186,6 +187,18 @@ export const ChartScreen: React.FC = () => {
>
<VictoryBar />
</VictoryChart>
<VictoryChart
containerComponent={
<VictoryZoomContainer
zoomDimension="x"
zoomDomain={{
x: [0, 5],
}}
/>
}
>
<VictoryBar barRatio={1.2} data={generateRandomData(10)} />
</VictoryChart>
</ScrollView>
);
};
11 changes: 6 additions & 5 deletions packages/victory-native/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,12 @@ To open the demo app, just fire up the expo app.
# Install victory and its dependencies
$ git clone https://github.com/FormidableLabs/victory
$ cd victory
$ yarn install
# Open up the React Native demo app
$ cd demo/rn
$ yarn install
$ yarn start
$ pnpm install
$ pnpm run build --watch
# In a new terminal, open up the React Native demo app
$ cd victory/demo/rn
$ pnpm install
$ pnpm start
```

Once Expo has fired up, it should open a web browser window where you can find instructions to open the demo application (either on a simulator or a physical device using the Expo Go app).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ export interface VictoryZoomContainerNativeProps
export const VictoryZoomContainer = (
initialProps: VictoryZoomContainerNativeProps,
) => {
const props = useVictoryZoomContainer({
const { props, children } = useVictoryZoomContainer({
...initialProps,
clipContainerComponent: initialProps.clipContainerComponent ?? (
<VictoryClipContainer />
),
});
return <VictoryContainer {...props} />;

return <VictoryContainer {...props}>{children}</VictoryContainer>;
};

VictoryZoomContainer.role = "container";
Expand Down
Loading

0 comments on commit 1c8217d

Please sign in to comment.