Skip to content
This repository has been archived by the owner on Feb 19, 2022. It is now read-only.

Fix date handling cross axis #318

Merged
merged 9 commits into from
Jul 27, 2016
Merged

Fix date handling cross axis #318

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
77d4da3
bar demo
ebrillhart Jul 1, 2016
1bc5bcb
commit to checkout
ebrillhart Jul 5, 2016
a1cdf13
demo change
ebrillhart Jul 15, 2016
488dcdf
commit changes to checkout
ebrillhart Jul 22, 2016
fb55909
reviewing data
ebrillhart Jul 25, 2016
1f7bfdf
fix date handling for cross axes
ebrillhart Jul 26, 2016
608cc34
fix merge errors and linting issues
ebrillhart Jul 26, 2016
66b61ed
update how helper methods are used for min and max values
ebrillhart Jul 26, 2016
593bd8a
update to newest version of core
ebrillhart Jul 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions demo/components/victory-axis-demo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ export default class App extends React.Component {
label={"Decades"}
tickLabelComponent={<VictoryLabel y={25}/>}
tickValues={[
new Date(1960, 1, 1),
new Date(1970, 1, 1),
new Date(1980, 1, 1),
new Date(1990, 1, 1),
new Date(2000, 1, 1),
new Date(2010, 1, 1),
new Date(2020, 1, 1)]}
new Date(2000, 1, 1)]}
tickFormat={(x) => x.getFullYear()}
/>

Expand Down Expand Up @@ -245,7 +245,6 @@ export default class App extends React.Component {
/>
</div>


</div>
);
}
Expand Down
83 changes: 72 additions & 11 deletions demo/components/victory-line-demo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,17 @@ export default class App extends React.Component {
<div className="demo">
<h1>VictoryLine</h1>

<VictoryLine
style={{parent: parentStyle, data: this.state.style}}
data={this.state.transitionData}
animate={{duration: 800}}
containerComponent={
<VictoryContainer
title="Line Chart"
desc="This is a line chart for displaying data."
/>
}
/>
<VictoryLine
style={{parent: parentStyle, data: this.state.style}}
data={this.state.transitionData}
animate={{duration: 800}}
containerComponent={
<VictoryContainer
title="Line Chart"
desc="This is a line chart for displaying data."
/>
}
/>

<VictoryLine
style={{parent: parentStyle, data: this.state.style}}
Expand Down Expand Up @@ -215,6 +215,67 @@ export default class App extends React.Component {
]}
/>

<VictoryLine
style={{parent: parentStyle}}
scale={{x: "linear", y: "log"}}
/>
<VictoryLine
style={{parent: parentStyle}}
data={this.state.arrayData}
label="Hello"
x={0}
y={1}
theme={VictoryTheme.grayscale}
/>

<VictoryChart
theme={VictoryTheme.grayscale}
>
<VictoryLine
style={{parent: parentStyle}}
data={this.state.arrayData}
label="Hello"
x={0}
y={1}
/>
</VictoryChart>

<VictoryChart
height={450}
scale={{
x: "time"
}}
>
<VictoryLine
data={[
{x: new Date(1960, 1, 1), y: 125},
{x: new Date(1987, 1, 1), y: 257},
{x: new Date(1993, 1, 1), y: 345},
{x: new Date(1997, 1, 1), y: 515},
{x: new Date(2001, 1, 1), y: 132},
{x: new Date(2005, 1, 1), y: 305},
{x: new Date(2011, 1, 1), y: 270},
{x: new Date(2015, 1, 1), y: 470}
]}
/>
</VictoryChart>

<VictoryLine
style={{parent: parentStyle}}
data={[
{x: 1, y: 1},
{x: 2, y: 3},
{x: 3, y: 5},
{x: 4, y: 2},
{x: 5, y: null},
{x: 6, y: null},
{x: 7, y: 6},
{x: 8, y: 7},
{x: 9, y: 8},
{x: 10, y: 12}
]}
/>

<VictoryLine
style={{parent: parentStyle}}
scale={{x: "linear", y: "log"}}
Expand Down
6 changes: 4 additions & 2 deletions src/components/victory-chart/helper-methods.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ export default {
const {axisComponents, domain, scale} = calculatedProps;
// make the axes line up, and cross when appropriate
const origin = {
x: Math.max(Math.min(...domain.x), 0),
y: Math.max(Math.min(...domain.y), 0)
x: Collection.containsDates(domain.x) ? Math.min(...domain.x)
: Math.max(Math.min(...domain.x), 0),
y: Collection.containsDates(domain.y) ? Math.min(...domain.y)
: Math.max(Math.min(...domain.y), 0)
};
const axisOrientations = {
x: Axis.getOrientation(axisComponents.x, "x"),
Expand Down
25 changes: 19 additions & 6 deletions src/helpers/domain.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ export default {
const { horizontal } = props;
const ensureZero = (domain) => {
const isDependent = (axis === "y" && !horizontal) || (axis === "x" && horizontal);
const zeroDomain = isDependent ? [Math.min(...domain, 0), Math.max(... domain, 0)]
: domain;
const min = Collection.containsDates(domain) ?
Helpers.retainDate(Math.min(...domain, 0)) :
Copy link
Contributor

Choose a reason for hiding this comment

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

if all retainDate does is new Date(val) why not just do that here? When we were chatting about extracting helper methods I was imagining getMaxValue(arr) which would do all this date checking and return the correct value type. If you end up going that route, the new methods should be added to util/collection since they operate on arrays. I thought this would be happening in may more places though, so if you just want to swap out retainDate(val) for new Date(val) that's fine too

Math.min(...domain, 0);
const max = Collection.containsDates(domain) ?
Helpers.retainDate(Math.max(...domain, 0)) :
Math.max(...domain, 0);
const zeroDomain = isDependent ? [min, max] : domain;
return this.padDomain(zeroDomain, props, axis);
};
const categoryDomain = this.getDomainFromCategories(props, axis);
Expand All @@ -50,8 +55,12 @@ export default {
getDomainFromData(props, axis, dataset) {
const currentAxis = Axis.getCurrentAxis(axis, props.horizontal);
const allData = flatten(dataset).map((datum) => datum[currentAxis]);
const min = Math.min(...allData);
const max = Math.max(...allData);
const min = Collection.containsDates(allData) ?
Helpers.retainDate(Math.min(...allData)) :
Math.min(...allData);
const max = Collection.containsDates(allData) ?
Helpers.retainDate(Math.max(...allData)) :
Math.max(...allData);
// TODO: is this the correct behavior, or should we just error. How do we
// handle charts with just one data point?
if (min === max) {
Expand Down Expand Up @@ -180,8 +189,12 @@ export default {
return domain;
}

const domainMin = Math.min(...domain);
const domainMax = Math.max(...domain);
const domainMin = Collection.containsDates(domain) ?
Helpers.retainDate(Math.min(...domain)) :
Math.min(...domain);
const domainMax = Collection.containsDates(domain) ?
Helpers.retainDate(Math.max(...domain)) :
Math.max(...domain);
const range = Helpers.getRange(props, axis);
const rangeExtent = Math.abs(Math.max(...range) - Math.min(...range));

Expand Down
8 changes: 7 additions & 1 deletion src/helpers/wrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,14 @@ export default {
};

const childDomains = getChildDomains(childComponents);
const min = Collection.containsDates(childDomains) ?
Helpers.retainDate(Math.min(...childDomains)) :
Math.min(...childDomains);
const max = Collection.containsDates(childDomains) ?
Helpers.retainDate(Math.max(...childDomains)) :
Math.max(...childDomains);
return childDomains.length === 0 ?
[0, 1] : [Math.min(...childDomains), Math.max(...childDomains)];
[0, 1] : [min, max];
},

getDataFromChildren(props, childComponents) {
Expand Down