Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

added prop rotateAngle to avoid the overlapping x-axis tick values #322

Merged
merged 10 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions packages/carbon-graphs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# ChangeLog

## Unreleased
* Fixed
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Fixed
* Added

* Fixed tick values overlapping on X-axis by rotating 15,30,45.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Fixed
* Fixed tick values overlapping on X-axis by rotating 15,30,45.
* Added
* Added prop `rotateAngle` to allow rotation of x-axis tick labels to prevent overlap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved in 3a9d13e


## 2.23.3 - (September 25, 2023)

Expand Down
21 changes: 19 additions & 2 deletions packages/carbon-graphs/src/js/helpers/axis.js
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,8 @@ const getY2AxisYPosition = (config) => calculateVerticalPadding(config);
const createAxes = (axis, scale, config, canvasSVG) => {
getAxesScale(axis, scale, config);
prepareHAxis(scale, axis, config, prepareHorizontalAxis);
// eslint-disable-next-line no-use-before-define
const rotateAngle = config.rotateAngle ? calculateRotateAngleBasedOnWidth(config) : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is vague. Since this is an x-axis prop, we can change this toconfig.axis.x.ticks.rotationAngle.

https://engineering.cerner.com/terra-ui/graphs/cerner-terra-graphs-docs/core-configuration/documentation/axes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved in 3a9d13e

canvasSVG
.append('g')
.classed(styles.axis, true)
Expand All @@ -893,7 +895,11 @@ const createAxes = (axis, scale, config, canvasSVG) => {
)})`,
)
.call(axis.x)
.call(resetD3FontSize);
.call(resetD3FontSize)
.attr('id', 'x')
.selectAll('text')
.style('text-anchor', config.rotateAngle ? 'end' : 'middle')
.attr('transform', () => `rotate(${rotateAngle})`);
canvasSVG
.append('g')
.classed(styles.axis, true)
Expand Down Expand Up @@ -922,7 +928,18 @@ const createAxes = (axis, scale, config, canvasSVG) => {
.call(resetD3FontSize);
}
};

const calculateRotateAngleBasedOnWidth = (config) => {
const containerWidth = config.canvasWidth;
// Define thresholds for rotation based on container width
const thresholdSmall = 400;
const thresholdMedium = 800;
if (containerWidth < thresholdSmall) {
return -45; // rotate by -45 degrees for small screens
} if (containerWidth < thresholdMedium) {
return -30; // rotate by -30 degrees for medium screens
}
return -15; // rotate by -15 degrees for larger screens
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the solution we agreed upon was to make the prop an integer and let consumers choose the angle as per their own requirements. What if a consumer wants to rotate it by 60º? Or what if they would like to rotate it in the other direction like positive 45º? By hardcoding the values and specifying them per screen size, we risk interpreting their data in ways they do not want it to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved in 3a9d13e

Copy link

Choose a reason for hiding this comment

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

@sdadn we had meetings after that meeting where we decided that we should restrict the available values to the default horizontal or -45º

cc: @mjpalazzo

/**
* X Axis's position vertically relative to the canvas
*
Expand Down
3 changes: 3 additions & 0 deletions packages/terra-graphs-docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# ChangeLog

## Unreleased
* Added
* Added a Prop `rotateAngle` for overlapping X-axis tick values.
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this entry

Suggested change
* Added a Prop `rotateAngle` for overlapping X-axis tick values.

* Added examples for the Xaxis Overlapping tick values.

## 1.6.0 - (September 25, 2023)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import Carbon from '@cerner/carbon-graphs/lib/js/carbon';

const getLineTimeseriesConfig = (id) => ({
bindTo: id,
rotateAngle: true,
axis: {
x: {
type: Carbon.helpers.AXIS_TYPE.TIME_SERIES,
label: 'Datetime',
lowerLimit: new Date(2016, 0, 1).toISOString(),
upperLimit: new Date(2016, 0, 12).toISOString(),
ticks: {
values: [
new Date(2016, 0, 1).toISOString(),
new Date(2016, 0, 2).toISOString(),
new Date(2016, 0, 3).toISOString(),
new Date(2016, 0, 4).toISOString(),
new Date(2016, 0, 5).toISOString(),
new Date(2016, 0, 6).toISOString(),
new Date(2016, 0, 7).toISOString(),
new Date(2016, 0, 8).toISOString(),
new Date(2016, 0, 9).toISOString(),
new Date(2016, 0, 10).toISOString(),
new Date(2016, 0, 11).toISOString(),
],
format: '%Y, %X',
},
},
y: {
label: 'Line Set A',
lowerLimit: 10,
upperLimit: 30,
},
y2: {
show: false,
label: 'Line Set B',
lowerLimit: 0,
upperLimit: 250,
},
},
showLabel: true,
showLegend: true,
showShapes: true,
showVGrid: true,
showHGrid: true,
locale: Carbon.helpers.LOCALE.de_DE,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the locale german?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that was not intentional, I was trying to recreate the issue with tick values(date formatted) having long text in it.

});

export default getLineTimeseriesConfig;
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ It is required for the following graphs:
const graphConfig = {
axis: <Axis object>,
bindTo: <string>,
rotateAngle: false,
allowCalibration: <bool>,
bindLegendTo: <string>,
clickPassThrough: {
Expand Down Expand Up @@ -70,6 +71,7 @@ const graphConfig = {
|------------------------|----------|------------------------------------------------|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| axis | object | - | yes | See [Axis](./Axes). |
| bindTo | string | - | yes | DOM ID to bind the graph container to. |
| rotateAngle | boolean | `false` | no | To rotate tickvalues anticlockwise `15`,`30`,`45` degrees with respect to the width of the x-axis. |
| allowCalibration | boolean | `true` | no | Toggle to allow calibration to adjust the y axis. |
| bindLegendTo | string | `null` | no | If DOM id provided, binds legend into that container (Example: `"#legendContainer"`). |
| clickPassThrough | object | `{}` | no | See [clickPassThrough](#clickpassthrough). |
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you generate wdio screenshots for this test?

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from 'react';
import utils from '@cerner/carbon-graphs/lib/js/helpers/utils';
import LineGraph from '@cerner/terra-graphs-docs/lib/terra-graphs-src/components/Line/LineGraph';
import '@cerner/terra-graphs-docs/lib/terra-dev-site/ExampleGraphContainer/ExampleGraphContainer.module.scss';
import getGraphConfig from '@cerner/terra-graphs-docs/lib/example-datasets/graphConfigObjects/General/generalDefaultXAxisOverlapping';
import exampleData1 from '@cerner/terra-graphs-docs/lib/example-datasets/dataObjects/Line/dataset1';
import exampleData2 from '@cerner/terra-graphs-docs/lib/example-datasets/dataObjects/Line/dataset2';
import exampleData3 from '@cerner/terra-graphs-docs/lib/example-datasets/dataObjects/Line/dataset3';
import exampleData4 from '@cerner/terra-graphs-docs/lib/example-datasets/dataObjects/Line/dataset4';
import exampleData5 from '@cerner/terra-graphs-docs/lib/example-datasets/dataObjects/Line/dataset5';
import exampleData6 from '@cerner/terra-graphs-docs/lib/example-datasets/dataObjects/Line/dataset6';

const graphConfig = utils.deepClone(getGraphConfig('#xAxisTicksVlauesOverlapping'));
const dataset = [
utils.deepClone(exampleData1),
utils.deepClone(exampleData2),
utils.deepClone(exampleData3),
utils.deepClone(exampleData4),
utils.deepClone(exampleData5),
utils.deepClone(exampleData6),
];

const timeoutArray = [0, 750, 750 * 2, 750 * 3, 750 * 4, 750 * 5, 750 * 6];

export default () => (
<>
<div id="tooltip" className="initial-tooltip" />
<LineGraph graphID="xAxisTicksVlauesOverlapping" graphConfig={graphConfig} dataset={dataset} timeout={timeoutArray} />
;
</>
);