List view
Victory has several issues with it's current approach to unit testing and integration testing. It would be in the best interests of the project to reconfigure and modernize testing from the ground up. Problems: - Current solution (Karma) is browser only. SSR/Node.js testing is not included and can potentially have issues not surfaced by solely browser-based testing. - All tests are confined within their own directory outside of the packages where the actual code being tested lives - Storybook/Chromatic screenshot testing works well, but forces use of Storybook into a very specific use case that doesn't allow for using it in a practical user testing sense, such adjusting knobs (props) for manually testing various render states. - Testing doesn't happen within CI/CD process Potential Solutions: - Replace Karma altogether with single unit/integration testing library that allows for both browser and server based testing - Alternatively integrate a second server-based testing framework alongside Karma. - Integrate spec files within packages so they're more tightly scoped to their source, while being aware of leaving these out of the build process. - Integrate unit-testing (not just Chromatic testing) within the CI/CD pipeline - Come up with a way to allow Storybook to work for both Chromatic as well as manually user testing. Open Questions: - Do we like enzyme over react-testing-library or another solution? - How efficient and thorough are the tests? How long do they take to run? - How are children within components handled for testing? Are we only concerned with shallow rendering or do we need to cover more?
No due date•13/13 issues closedI have noticed a few bugs coming up recently around transitions, and this code is currently pretty difficult to update. Some specifics I have identified: - It's not currently possible to configure the `move` transition - The wrong transition is getting called in some components. The default prop is being used as the `onLoad` transition duration rather than the `onLoad` transition duration from the props or configuration. As a part of fixing some of these transition bugs, it would be good to re-think some of this logic and make it easier to understand.
No due dateOn a basic level, rendering data in Victory is a three step process. 1. Victory iterates through each child component to get its data. For container components like `VictoryGroup` or `VictoryContainer`, the parent calls the `getChildren` static property to iterate through nested children. 2. Victory uses each child component's data to calculate values like domain, range, and scale. 3. Victory iterates through each child component again, clones it, and passes in calculated props like events and scaled data based on the component's role. This pattern works well for custom data components, as it makes it easy for the consumer to access any state it needs to render data through the props passed in by Victory. However, continually iterating through child components has some performance tradeoffs, particularly for larger charts. (See this issue: [https://github.com/FormidableLabs/victory/issues/1996](https://github.com/FormidableLabs/victory/issues/1996)) It can also be difficult to debug, as it's not always clear where a particular prop is coming from inside Victory. Props can seemingly disappear, and right now the static "role" property can seem like a magic word that is needed in order to get the correct props passed down. Overall, I think we should use modern tooling to work towards making Victory less magical. Rather than continually iterating through children and injecting props into children with specific roles, it would be better to have some shared state that children can read from and update. ```jsx // in Victory data component (like VictoryLine) const { setData, getScaledData } = useVictoryData(); React.useEffect(() => { if (data) { setData(key, data); } }, [data]) return React.cloneElement(dataComponent, { data: getScaledData(key), ...etc }); ``` With this shared state, values like `domain`, `range`, and `scale` could be accessed by any component rather than re-calculated in multiple places. Rather than the parent deciding which props a given child needs based on the role property, the child component should be responsible for getting the state that it needs from shared context. This would make it easier to track where data is coming from, and the top-level components wouldn't need to be continually iterating through children looking for data and prop changes, which may allow for some significant performance improvements.
No due date•4/5 issues closedThis milestone will be focused on labels, legends and tooltips
Due by August 2, 2019•11/11 issues closed- onLoad Animations - v1 VictoryTooltip - v1 victory-cli - Export data primitives _i.e._ Line, Bar, Point etc. - enhancements for VictoryGroup - v1 VictoryGauge
Due by August 31, 2016•3/3 issues closed