fix datachart bug for single chart data #7020
Conversation
Sulaymon333
changed the title
There was a problem hiding this comment.
The DataChart works with one value when direction="vertical", but when we change direction to "horizontal that is where we run into issues. This makes me think that there is something in the YAxis code that is not handling the 1 data point case well. The XAxis code seems to be able to handle 1 data point. Comparing the XAxis code with the YAxis code might be a good place to look into more.
| @@ -547,7 +547,8 @@ const DataChart = forwardRef( | |||
| if (!property || (!horizontal && y) || (horizontal && !y)) { | |||
| if (render) return render(value); | |||
| } else { | |||
| const datum = data[axisValue]; | |||
| // const datum = data[axisValue]; | |||
| const datum = data[0]; | |||
There was a problem hiding this comment.
This change causes an issue with the axis labels. If we have the following code we end up with two xaxis labels that are the same value (Jun 23) coming from data[0].
const data = [
{ a: 1, b: 'one', c: 111111, d: '2020-06-24' },
{ a: 2, b: 'two', c: 222222, d: '2020-06-23' },
];
export const Simple = () => (
<Box align="center" justify="start" pad="large">
<DataChart
data={data}
series={['d', 'a']}
axis={{
x: { property: 'd' },
y: { property: 'a' },
}}
/>
</Box>
);
| @@ -527,7 +527,7 @@ const DataChart = forwardRef( | |||
| series.forEach(({ property, render }) => { | |||
| if ( | |||
| !render && | |||
| data.length > 1 && | |||
| data.length >= 1 && | |||
There was a problem hiding this comment.
| data.length >= 1 && | |
| data.length > 1 && |
I think we should revert this change back. If we only have 1 data point we don't need the createDateFormat code
| if (delta === 0) | ||
| // equal 0 - single data point | ||
| options = { | ||
| month: full ? 'short' : 'numeric', | ||
| day: 'numeric', | ||
| }; | ||
| else if (delta < 60000) |
There was a problem hiding this comment.
| if (delta === 0) | |
| // equal 0 - single data point | |
| options = { | |
| month: full ? 'short' : 'numeric', | |
| day: 'numeric', | |
| }; | |
| else if (delta < 60000) | |
| if (delta < 60000) |
Lets revert this change back, I don't think we need to adjust this
| @@ -96,7 +102,7 @@ export const createDateFormat = (firstValue, lastValue, full) => { | |||
| else if (delta < 86400000) | |||
| // less than 1 day | |||
| options = { hour: 'numeric' }; | |||
| else if (delta < 2592000000) | |||
| else if (delta < 2592000000 || delta === 0) | |||
There was a problem hiding this comment.
| else if (delta < 2592000000 || delta === 0) | |
| else if (delta < 2592000000) |
Let's revert this change as well
What does this PR do?
This pull request is meant to fix datachart bug with single data item
Where should the reviewer start?
What testing has been done on this PR?
How should this be manually tested?
Do Jest tests follow these best practices?
screenis used for querying.userEventis used in place offireEvent.asFragment()is used for snapshot testing.Any background context you want to provide?
What are the relevant issues?
closes #6991
Screenshots (if appropriate)
Do the grommet docs need to be updated?
No
Should this PR be mentioned in the release notes?
Yes
Is this change backwards compatible or is it a breaking change?
Backward compatible