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

Unable to wrap victory components into customizable components #2342

Open
Rahiniji opened this issue Jun 30, 2022 · 7 comments
Open

Unable to wrap victory components into customizable components #2342

Rahiniji opened this issue Jun 30, 2022 · 7 comments
Labels
Issue: Accepted The submitted issue has been confirmed by the Victory core team Type: Enhancement ✏️ An enhancement or feature proposal that will be addressed after the next release

Comments

@Rahiniji
Copy link

Is your feature request related to a problem? Please describe.
When I try to wrap Victory components like VictoryLine or VictoryBar, the graph is not plotted for reasons unknown.

Describe the solution you'd like
Need to get Victory component wrapped to my custom component thereby plotting chart smoothly as expected.

Describe alternatives you've considered
I tried mentioning the roles like below, but no luck.
MyCustomLine.role = 'line';

Additional context
Below is the Codesandbox link demonstrating the sample application I am trying. It contains the examples of working one wherein Victory component is directly used and not working one, wherein Victory component is wrapped up
https://codesandbox.io/s/victory-wrapper-p6ffzy?file=/src/App.tsx

Please let me know if I am missing something to get my chart plotted using my wrapped component.

@dsanders1234
Copy link

I've also tried this, even passing all the props from parent to child and it does not work. I believe that this is related to:

#2076

I noticed that the above issue is closed. Is this fix going towards a specific version?

@becca-bailey
Copy link
Contributor

becca-bailey commented Jul 8, 2022

Unless there is some workaround I'm not yet aware of, Victory doesn't currently support custom components as top-level components inside a VictoryChart component. This isn't ideal, and is definitely a reflection of some not-so-great architectural patterns we have going on right now, but basically the VictoryChart component makes decisions about how to render its children based on the top-level props and static properties of each child, and there is some really specific logic for doing this in each Victory data component like VictoryBar or VictoryLine.

For now, I would encourage you to use the Victory component at the top-level when rendering inside a VictoryChart, VictoryStack, or VictoryGroup component, as you have done in two of your examples. There may be another workaround if I spend a little more time on this, but we are planning to overhaul this architecture in the v37 release.

@anstapol
Copy link

anstapol commented May 3, 2023

Hi guys, any solution to this problem? Can't create custom components? I've tried merging props which my custom component receives still have some weird behaviour.

@ThaJay
Copy link

ThaJay commented Oct 23, 2023

@becca-bailey Do you mean that custom components using Victory components should work if I wrap the custom components in VictoryGroup? Because that does not seem to help. Perhaps I misunderstand your wording.

This really does feel like an important issue to me, I'm not looking for a repeat of the very long and hard to read file that the people at Chart.js made me write and this looked like a very promising library.

@robcostello1
Copy link

@becca-bailey I see v37 was released a few days ago but I don't believe this included the architecture changes you mentioned above. Are these still on the roadmap?

@carbonrobot
Copy link
Contributor

@robcostello1 The v37 release was a necessity for a different reason; changing the babel output target. Although v36.x made a lot of changes in the architecture, we still do not have a direct way to allow custom components. The original code of Victory was designed prior to hooks, context, and concepts like slots (9yrs ago now) so the architecture does not have a simple way to accommodate a custom component because it looks at the Type of the first child and runs logic based on that Type.

For now, this feature remains on the backlog with no roadmap plan. Our hope is to move completely to functional components and use context in the future which solves this, but we have several hurdles to jump before its possible (particularly #2778).

@carbonrobot carbonrobot reopened this Mar 21, 2024
@carbonrobot carbonrobot added Type: Enhancement ✏️ An enhancement or feature proposal that will be addressed after the next release Issue: Accepted The submitted issue has been confirmed by the Victory core team labels Mar 21, 2024
@ThaJay
Copy link

ThaJay commented Mar 27, 2024

@carbonrobot That sould like an oversight in the design phase, is it not possbile for this library to use normal React code so the Victory components don't break on a React API update?

It would make sense to me if Victory components were just regular old React components with some sugar on top.

Is there any graphing library out there that does conform to React's standards and actually is composable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Accepted The submitted issue has been confirmed by the Victory core team Type: Enhancement ✏️ An enhancement or feature proposal that will be addressed after the next release
Projects
None yet
Development

No branches or pull requests

7 participants