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

Audits and Adds typescript props, definitions, and demo: VictoryZoomContainer #1536

Merged
merged 12 commits into from
May 4, 2020

Conversation

wsparsons
Copy link
Member

@wsparsons wsparsons commented Apr 25, 2020

  • Updates the name of CursorData to CoordinatesPropType
  • Updates the VictoryBrushContainerProps and VictoryBrushContainerDemo to use DomainTuple instead RangeTuple
  • Adds a VictoryZoomContainerDemo
  • Adds and updates missing props and their typescript definitions for VictoryZoomContainerProps interface
  • Removes props from VictoryZoomContainerProps interface that do not belong

@wsparsons wsparsons changed the title [WIP] Audits and Adds typescript props, definitions, and demo: VictoryBrushContainer [WIP] Audits and Adds typescript props, definitions, and demo: VictoryZoomContainer Apr 25, 2020
@wsparsons wsparsons force-pushed the task/audit-victory-zoom-container-typescript-props branch 2 times, most recently from 2f66b9f to 0e08423 Compare April 29, 2020 19:24
@wsparsons wsparsons marked this pull request as ready for review April 29, 2020 19:50
@wsparsons wsparsons changed the title [WIP] Audits and Adds typescript props, definitions, and demo: VictoryZoomContainer Audits and Adds typescript props, definitions, and demo: VictoryZoomContainer Apr 29, 2020
Copy link
Contributor

@boygirl boygirl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Thanks for the rangeTuple -> domainTuple change, too!

@wsparsons wsparsons force-pushed the task/audit-victory-zoom-container-typescript-props branch from a9e264c to ffb1577 Compare April 30, 2020 20:51
@@ -123,7 +118,7 @@ export default class App extends React.Component {

getZoomDomain() {
return {
y: [random(0, 0.4, 0.1), random(0.6, 1, 0.1)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ended up removing this since https://lodash.com/docs/4.17.15#random says the third argument should be a boolean. not sure if the 0.1 was intentional or if it should have a boolean value.

@@ -61,11 +61,6 @@ class CustomChart extends React.Component {
x: [data[0].x, last(data).x]
};
}
getZoomFactor() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ended up removing this uninvoked function. not sure if this should be used in the demo and somehow got missed?

@wsparsons wsparsons requested a review from boygirl April 30, 2020 20:54
@wsparsons wsparsons force-pushed the task/audit-victory-zoom-container-typescript-props branch from ffb1577 to 68a8f49 Compare May 4, 2020 22:29
@wsparsons wsparsons merged commit 08d92d2 into master May 4, 2020
@boygirl boygirl deleted the task/audit-victory-zoom-container-typescript-props branch June 23, 2020 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants