Skip to content

Add graph visualization for selected numbers #2884

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

Open
wants to merge 3 commits into
base: alpha
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
74 changes: 44 additions & 30 deletions src/components/Chart/Chart.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default class Chart extends React.Component {
}

render() {
const { width, height, data } = this.props;
const { width, height, data, xAxisType = 'time', hideXAxisLabels = false } = this.props;
const plotting = {};
let minX = Infinity;
let maxX = -Infinity;
Expand All @@ -83,7 +83,12 @@ export default class Chart extends React.Component {
}
plotting[key] = { data: ordered, index: data[key].index };
}
const timeBuckets = Charting.timeAxisBuckets(minX, maxX);
let timeBuckets;
if (xAxisType === 'index') {
timeBuckets = Charting.numericAxisBuckets(minX, maxX);
} else {
timeBuckets = Charting.timeAxisBuckets(minX, maxX);
}
const valueBuckets = Charting.valueAxisBuckets(maxY || 10);
const groups = [];
for (const key in plotting) {
Expand Down Expand Up @@ -134,23 +139,28 @@ export default class Chart extends React.Component {
t =>
(chartWidth * (t - timeBuckets[0])) / (timeBuckets[timeBuckets.length - 1] - timeBuckets[0])
);
let last = null;
const tickLabels = timeBuckets.map((t, i) => {
let text = '';
if (timeBuckets.length > 20 && i % 2 === 0) {
return '';
}
if (!last || t.getMonth() !== last.getMonth()) {
text += shortMonth(t.getMonth()) + ' ';
}
if (!last || t.getDate() !== last.getDate()) {
text += t.getDate();
} else if (last && t.getHours() !== last.getHours()) {
text += t.getHours() + ':00';
}
last = t;
return text;
});
let tickLabels;
if (xAxisType === 'index') {
tickLabels = timeBuckets.map(t => t);
} else {
let last = null;
tickLabels = timeBuckets.map((t, i) => {
let text = '';
if (timeBuckets.length > 20 && i % 2 === 0) {
return '';
}
if (!last || t.getMonth() !== last.getMonth()) {
text += shortMonth(t.getMonth()) + ' ';
}
if (!last || t.getDate() !== last.getDate()) {
text += t.getDate();
} else if (last && t.getHours() !== last.getHours()) {
text += t.getHours() + ':00';
}
last = t;
return text;
});
}
let popup = null;
if (this.state.hoverValue !== null) {
const style = {
Expand Down Expand Up @@ -191,17 +201,19 @@ export default class Chart extends React.Component {
</div>
))}
</div>
<div className={styles.xAxis}>
{tickLabels.map((t, i) => (
<div
key={t + '_' + i}
className={styles.tick}
style={{ left: tickPoints[i] + MARGIN_LEFT }}
>
{t}
</div>
))}
</div>
{!hideXAxisLabels && (
<div className={styles.xAxis}>
{tickLabels.map((t, i) => (
<div
key={t + '_' + i}
className={styles.tick}
style={{ left: tickPoints[i] + MARGIN_LEFT }}
>
{t}
</div>
))}
</div>
)}
<svg width={chartWidth + 10} height={chartHeight + 10}>
<g>
{labelHeights.map(h => (
Expand Down Expand Up @@ -245,4 +257,6 @@ Chart.propTypes = {
'It receives the numeric value of a point and label, and should return a string. ' +
'This is ideally used for providing descriptive units like "active installations."'
),
xAxisType: PropTypes.string.describe('Axis type: "time" or "index"'),
hideXAxisLabels: PropTypes.bool.describe('Hide labels on the x-axis'),
};
128 changes: 128 additions & 0 deletions src/components/GraphPanel/GraphPanel.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import React, { useState, useEffect } from 'react';
import Chart from 'components/Chart/Chart.react';
import { ChartColorSchemes } from 'lib/Constants';
import styles from './GraphPanel.scss';

function parseDate(val) {
if (!val) {
return null;
}
if (val instanceof Date) {
return val.getTime();
}
if (typeof val === 'string') {
const d = new Date(val);
return isNaN(d) ? null : d.getTime();
}
if (val.iso) {
const d = new Date(val.iso);
return isNaN(d) ? null : d.getTime();
}
return null;
}

export default function GraphPanel({ selectedCells, order, data, columns, width }) {
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add PropTypes validation.

The component is missing PropTypes validation, which would help catch type-related issues during development.

Add PropTypes validation:

+import PropTypes from 'prop-types';
+
+GraphPanel.propTypes = {
+  selectedCells: PropTypes.object,
+  order: PropTypes.array.isRequired,
+  data: PropTypes.array.isRequired,
+  columns: PropTypes.object.isRequired,
+  width: PropTypes.number.isRequired,
+};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export default function GraphPanel({ selectedCells, order, data, columns, width }) {
import PropTypes from 'prop-types';
export default function GraphPanel({ selectedCells, order, data, columns, width }) {
// ...existing implementation...
}
GraphPanel.propTypes = {
selectedCells: PropTypes.object,
order: PropTypes.array.isRequired,
data: PropTypes.array.isRequired,
columns: PropTypes.object.isRequired,
width: PropTypes.number.isRequired,
};
🤖 Prompt for AI Agents
In src/components/GraphPanel/GraphPanel.js at line 24, the GraphPanel component
lacks PropTypes validation. Add PropTypes import from 'prop-types' at the top of
the file, then define GraphPanel.propTypes after the component declaration
specifying the expected types for selectedCells, order, data, columns, and width
to ensure type checking during development.

if (!selectedCells || selectedCells.rowStart < 0) {
return null;
}
const { rowStart, rowEnd, colStart, colEnd } = selectedCells;
const columnNames = order.slice(colStart, colEnd + 1).map(o => o.name);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add bounds checking for array slicing.

The code slices the order array without checking if colStart and colEnd are within bounds, which could cause runtime errors.

Add bounds checking:

+  if (colStart < 0 || colEnd >= order.length) {
+    return <div className={styles.empty}>Invalid column selection.</div>;
+  }
   const columnNames = order.slice(colStart, colEnd + 1).map(o => o.name);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const columnNames = order.slice(colStart, colEnd + 1).map(o => o.name);
if (colStart < 0 || colEnd >= order.length) {
return <div className={styles.empty}>Invalid column selection.</div>;
}
const columnNames = order.slice(colStart, colEnd + 1).map(o => o.name);
🤖 Prompt for AI Agents
In src/components/GraphPanel/GraphPanel.js at line 29, the slice operation on
the order array uses colStart and colEnd without verifying if these indices are
within the valid range of the array. To fix this, add bounds checking to ensure
colStart is at least 0 and colEnd does not exceed order.length - 1 before
slicing. Adjust colStart and colEnd accordingly to prevent runtime errors from
invalid indices.

const columnTypes = columnNames.map(name => columns[name]?.type);

const isSingleColumn = columnNames.length === 1;

const initialUseXAxis =
!isSingleColumn &&
(columnTypes[0] === 'Date' || columnTypes[0] === 'Number') &&
columnTypes.slice(1).some(t => t === 'Number');

const [useXAxis, setUseXAxis] = useState(initialUseXAxis);

useEffect(() => {
setUseXAxis(initialUseXAxis);
}, [selectedCells?.rowStart, selectedCells?.rowEnd, selectedCells?.colStart, selectedCells?.colEnd]);

const chartData = {};

if (useXAxis) {
let seriesIndex = 0;
for (let j = 1; j < columnNames.length; j++) {
if (columnTypes[j] === 'Number') {
chartData[columnNames[j]] = {
color: ChartColorSchemes[seriesIndex],
points: [],
};
seriesIndex++;
}
}
for (let i = rowStart; i <= rowEnd; i++) {
const row = data[i];
if (!row) continue;

Check failure on line 60 in src/components/GraphPanel/GraphPanel.js

View workflow job for this annotation

GitHub Actions / Lint

Expected { after 'if' condition
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix linting issues: Add braces after if conditions.

The static analysis tool correctly identifies missing braces after if conditions, which violates the project's style guide.

Apply this diff to fix the linting issues:

-      if (!row) continue;
+      if (!row) {
+        continue;
+      }
       const ts = parseDate(row.attributes[columnNames[0]]);
-      if (ts === null) continue;
+      if (ts === null) {
+        continue;
+      }
       for (let j = 1; j < columnNames.length; j++) {
         const val = row.attributes[columnNames[j]];
         if (typeof val === 'number' && !isNaN(val)) {
           chartData[columnNames[j]].points.push([ts, val]);
         }
       }
     }
   } else {
     let seriesIndex = 0;
     columnNames.forEach((col, idx) => {
       if (columnTypes[idx] === 'Number') {
         chartData[col] = { color: ChartColorSchemes[seriesIndex], points: [] };
         seriesIndex++;
       }
     });
     let x = 0;
     for (let i = rowStart; i <= rowEnd; i++, x++) {
       const row = data[i];
-      if (!row) continue;
+      if (!row) {
+        continue;
+      }

Also applies to: 45-45, 64-64

🧰 Tools
🪛 GitHub Check: Lint

[failure] 43-43:
Expected { after 'if' condition

🤖 Prompt for AI Agents
In src/components/GraphPanel/GraphPanel.js at lines 43, 45, and 64, the if
statements lack braces which causes linting errors. Add curly braces {} around
the statements following each if condition to comply with the project's style
guide and fix the linting issues.

let x = row.attributes[columnNames[0]];
if (columnTypes[0] === 'Date') {
x = parseDate(x);
} else if (typeof x === 'string') {
x = parseFloat(x);
}
if (typeof x !== 'number' || isNaN(x)) {
continue;
}
for (let j = 1; j < columnNames.length; j++) {
if (columnTypes[j] !== 'Number') continue;

Check failure on line 71 in src/components/GraphPanel/GraphPanel.js

View workflow job for this annotation

GitHub Actions / Lint

Expected { after 'if' condition
const val = row.attributes[columnNames[j]];
if (typeof val === 'number' && !isNaN(val)) {
chartData[columnNames[j]].points.push([x, val]);
}
}
}
} else {
let seriesIndex = 0;
columnNames.forEach((col, idx) => {
if (columnTypes[idx] === 'Number') {
chartData[col] = { color: ChartColorSchemes[seriesIndex], points: [] };
seriesIndex++;
}
});
let x = 0;
for (let i = rowStart; i <= rowEnd; i++, x++) {
const row = data[i];
if (!row) continue;

Check failure on line 89 in src/components/GraphPanel/GraphPanel.js

View workflow job for this annotation

GitHub Actions / Lint

Expected { after 'if' condition
columnNames.forEach((col, idx) => {
if (columnTypes[idx] !== 'Number') return;

Check failure on line 91 in src/components/GraphPanel/GraphPanel.js

View workflow job for this annotation

GitHub Actions / Lint

Expected { after 'if' condition
const val = row.attributes[col];
if (typeof val === 'number' && !isNaN(val)) {
chartData[col].points.push([x, val]);
}
});
}
}

if (Object.keys(chartData).length === 0) {
return <div className={styles.empty}>No numeric data selected.</div>;
}

const chartWidth = width - 20;
const xAxisType = useXAxis ? 'time' : 'index';
return (
<div className={styles.graphPanel}>
<div className={styles.options}>
<label>
<input
type="checkbox"
checked={useXAxis}
onChange={() => setUseXAxis(!useXAxis)}
disabled={isSingleColumn}
/>{' '}
Use first column as X-axis
</label>
</div>
<Chart
width={chartWidth}
height={400}
data={chartData}
xAxisType={xAxisType}
hideXAxisLabels={isSingleColumn}
/>
</div>
);
}
15 changes: 15 additions & 0 deletions src/components/GraphPanel/GraphPanel.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
.graphPanel {
height: 100%;
overflow: auto;
background-color: #fefafb;
padding: 10px;
}

.options {
margin-bottom: 10px;
}

.empty {
padding: 10px;
color: #555;
}
29 changes: 20 additions & 9 deletions src/components/Toolbar/Toolbar.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { useNavigate, useNavigationType, NavigationType } from 'react-router-dom

const POPOVER_CONTENT_ID = 'toolbarStatsPopover';

const Stats = ({ data, classwiseCloudFunctions, className, appId, appName }) => {
const Stats = ({ data, classwiseCloudFunctions, className, appId, appName, toggleGraph, isGraphVisible }) => {
const [selected, setSelected] = React.useState(null);
const [open, setOpen] = React.useState(false);
const buttonRef = React.useRef();
Expand Down Expand Up @@ -108,14 +108,21 @@ const Stats = ({ data, classwiseCloudFunctions, className, appId, appName }) =>
return (
<>
{selected ? (
<button
ref={buttonRef}
className={styles.stats}
onClick={toggle}
style={{ marginRight: rightMarginStyle }}
>
{`${selected.label}: ${selected.getValue(data)}`}
</button>
<>
<button
ref={buttonRef}
className={styles.stats}
onClick={toggle}
style={{ marginRight: rightMarginStyle }}
>
{`${selected.label}: ${selected.getValue(data)}`}
</button>
{data.length > 1 ? (
<button onClick={toggleGraph} className={styles.graph}>
{isGraphVisible ? 'Hide Graph' : 'Show Graph'}
</button>
) : null}
</>
) : null}
{open ? renderPopover() : null}
</>
Expand Down Expand Up @@ -152,6 +159,8 @@ const Toolbar = props => {
className={props.className}
appId={props.appId}
appName={props.appName}
toggleGraph={props.toggleGraph}
isGraphVisible={props.isGraphVisible}
/>
) : null}
<div className={styles.actions}>{props.children}</div>
Expand Down Expand Up @@ -182,6 +191,8 @@ Toolbar.propTypes = {
details: PropTypes.string,
relation: PropTypes.object,
selectedData: PropTypes.array,
toggleGraph: PropTypes.func,
isGraphVisible: PropTypes.bool,
};

export default Toolbar;
13 changes: 13 additions & 0 deletions src/components/Toolbar/Toolbar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,19 @@ body:global(.expanded) {
border: none;
}

.graph {
position: absolute;
right: 120px;
bottom: 10px;
background: $blue;
border-radius: 3px;
padding: 2px 6px;
font-size: 14px;
color: white;
box-shadow: none;
border: none;
}

.stats_popover_container {
display: flex;
flex-direction: column;
Expand Down
4 changes: 4 additions & 0 deletions src/dashboard/Data/Browser/BrowserToolbar.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ const BrowserToolbar = ({

togglePanel,
isPanelVisible,
toggleGraph,
isGraphVisible,
classwiseCloudFunctions,
appId,
appName,
Expand Down Expand Up @@ -277,6 +279,8 @@ const BrowserToolbar = ({
selectedData={selectedData}
togglePanel={togglePanel}
isPanelVisible={isPanelVisible}
toggleGraph={toggleGraph}
isGraphVisible={isGraphVisible}
classwiseCloudFunctions={classwiseCloudFunctions}
appId={appId}
appName={appName}
Expand Down
Loading
Loading