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 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
84 changes: 84 additions & 0 deletions src/components/GraphPanel/GraphPanel.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import React 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 timeSeries =
columnTypes.length > 1 &&
columnTypes[0] === 'Date' &&
columnTypes.slice(1).every(t => t === 'Number');

const chartData = {};
if (timeSeries) {
for (let j = 1; j < columnNames.length; j++) {
chartData[columnNames[j]] = { color: ChartColorSchemes[j - 1], points: [] };
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify ChartColorSchemes array has sufficient colors.

The code uses ChartColorSchemes[j - 1] and ChartColorSchemes[seriesIndex] without checking if the array has enough colors for the number of series.

Also applies to: 57-57


🏁 Script executed:

#!/bin/bash
# Search for ChartColorSchemes definition to verify available colors
ast-grep --pattern 'ChartColorSchemes = $_'

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Locate all references and the definition of ChartColorSchemes
rg -n "ChartColorSchemes"

Length of output: 2197


🏁 Script executed:

#!/bin/bash
# Display the contents of ChartColorSchemes in lib/Constants.js
sed -n '1,100p' src/lib/Constants.js | sed -n '22,50p'

Length of output: 515


🏁 Script executed:

#!/bin/bash
# Show context around ChartColorSchemes usage in GraphPanel.js
sed -n '1,200p' src/components/GraphPanel/GraphPanel.js

Length of output: 2548


Ensure ChartColorSchemes won’t be indexed out-of-bounds

ChartColorSchemes currently has 12 entries, but GraphPanel can request more if the user selects >12 series. To prevent undefined colors (or runtime errors), clamp or wrap your index and/or provide a fallback:

• File: src/components/GraphPanel/GraphPanel.js
– Line 39:
```diff

  • chartData[columnNames[j]] = { color: ChartColorSchemes[j - 1], points: [] };
  • const tsColorIndex = (j - 1) % ChartColorSchemes.length;
  • chartData[columnNames[j]] = {
  • color: ChartColorSchemes[tsColorIndex],
    
  • points: [],
    
  • };
    
    

– Line 57:
```diff

  • chartData[col] = { color: ChartColorSchemes[seriesIndex], points: [] };
  • const idx = seriesIndex % ChartColorSchemes.length;
  • chartData[col] = {
  • color: ChartColorSchemes[idx],
    
  • points: [],
    
  • };
    
    

Alternatively, check seriesCount <= ChartColorSchemes.length before rendering and display a warning or disable extra series.

📝 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
chartData[columnNames[j]] = { color: ChartColorSchemes[j - 1], points: [] };
// — at around line 39 in src/components/GraphPanel/GraphPanel.js —
const tsColorIndex = (j - 1) % ChartColorSchemes.length;
chartData[columnNames[j]] = {
color: ChartColorSchemes[tsColorIndex],
points: [],
};
// — at around line 57 in src/components/GraphPanel/GraphPanel.js —
const idx = seriesIndex % ChartColorSchemes.length;
chartData[col] = {
color: ChartColorSchemes[idx],
points: [],
};
🤖 Prompt for AI Agents
In src/components/GraphPanel/GraphPanel.js at line 39, the code indexes
ChartColorSchemes with j - 1, which can exceed the array length if more than 12
series are selected. To fix this, modify the index to wrap around using modulo
operation with ChartColorSchemes.length or clamp it to the maximum valid index
to avoid out-of-bounds access. Alternatively, add a check before rendering to
ensure seriesCount does not exceed ChartColorSchemes.length and handle excess
series by showing a warning or disabling them.

}
for (let i = rowStart; i <= rowEnd; i++) {
const row = data[i];
if (!row) continue;

Check failure on line 43 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.

const ts = parseDate(row.attributes[columnNames[0]]);
if (ts === null) continue;

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

View workflow job for this annotation

GitHub Actions / Lint

Expected { after 'if' condition
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;

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

View workflow job for this annotation

GitHub Actions / Lint

Expected { after 'if' condition
columnNames.forEach(col => {
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;
return (
<div className={styles.graphPanel}>
<Chart width={chartWidth} height={400} data={chartData} />
</div>
);
}
11 changes: 11 additions & 0 deletions src/components/GraphPanel/GraphPanel.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.graphPanel {
height: 100%;
overflow: auto;
background-color: #fefafb;
padding: 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
55 changes: 54 additions & 1 deletion src/dashboard/Data/Browser/DataBrowser.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import styles from './Databrowser.scss';

import AggregationPanel from '../../../components/AggregationPanel/AggregationPanel';
import GraphPanel from 'components/GraphPanel/GraphPanel';

const BROWSER_SHOW_ROW_NUMBER = 'browserShowRowNumber';

Expand Down Expand Up @@ -80,7 +81,7 @@
const storedRowNumber =
window.localStorage?.getItem(BROWSER_SHOW_ROW_NUMBER) === 'true';

this.state = {
this.state = {

Check failure on line 84 in src/dashboard/Data/Browser/DataBrowser.react.js

View workflow job for this annotation

GitHub Actions / Lint

Expected indentation of 4 spaces but found 6
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 indentation to match coding standards.

The static analysis tool correctly identifies an indentation issue. The line should use 4 spaces instead of 6.

-      this.state = {
+    this.state = {
📝 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
this.state = {
this.state = {
🧰 Tools
🪛 GitHub Check: Lint

[failure] 84-84:
Expected indentation of 4 spaces but found 6

🤖 Prompt for AI Agents
In src/dashboard/Data/Browser/DataBrowser.react.js at line 84, the indentation
of the line initializing this.state uses 6 spaces instead of the required 4
spaces. Adjust the indentation to use exactly 4 spaces to comply with the coding
standards.

order: order,
current: null,
editing: false,
Expand All @@ -99,6 +100,8 @@
showAggregatedData: true,
frozenColumnIndex: -1,
showRowNumber: storedRowNumber,
graphVisible: false,
graphWidth: 300,
};

this.handleResizeDiv = this.handleResizeDiv.bind(this);
Expand All @@ -109,6 +112,9 @@
this.handleHeaderDragDrop = this.handleHeaderDragDrop.bind(this);
this.handleResize = this.handleResize.bind(this);
this.togglePanelVisibility = this.togglePanelVisibility.bind(this);
this.handleGraphResizeStart = this.handleGraphResizeStart.bind(this);
this.handleGraphResizeStop = this.handleGraphResizeStop.bind(this);
this.handleGraphResizeDiv = this.handleGraphResizeDiv.bind(this);
this.setCurrent = this.setCurrent.bind(this);
this.setEditing = this.setEditing.bind(this);
this.handleColumnsOrder = this.handleColumnsOrder.bind(this);
Expand All @@ -120,6 +126,7 @@
this.unfreezeColumns = this.unfreezeColumns.bind(this);
this.setShowRowNumber = this.setShowRowNumber.bind(this);
this.handleCellClick = this.handleCellClick.bind(this);
this.toggleGraphVisibility = this.toggleGraphVisibility.bind(this);
this.saveOrderTimeout = null;
}

Expand Down Expand Up @@ -215,6 +222,21 @@
this.setState({ panelWidth: size.width });
}

handleGraphResizeStart() {
this.setState({ isResizing: true });
}

handleGraphResizeStop(event, { size }) {
this.setState({
isResizing: false,
graphWidth: size.width,
});
}

handleGraphResizeDiv(event, { size }) {
this.setState({ graphWidth: size.width });
}

setShowAggregatedData(bool) {
this.setState({
showAggregatedData: bool,
Expand Down Expand Up @@ -264,6 +286,10 @@
}
}

toggleGraphVisibility() {
this.setState(prevState => ({ graphVisible: !prevState.graphVisible }));
}

getAllClassesSchema(schema) {
const allClasses = Object.keys(schema.data.get('classes').toObject());
const schemaSimplifiedData = {};
Expand Down Expand Up @@ -667,6 +693,7 @@
},
selectedObjectId: undefined,
selectedData,
graphVisible: true,
});
} else {
this.setCurrent({ row, col });
Expand All @@ -677,6 +704,7 @@
selectedData: [],
current: { row, col },
firstSelectedCell: clickedCellKey,
graphVisible: false,
});
}
}
Expand Down Expand Up @@ -758,6 +786,29 @@
</div>
</ResizableBox>
)}
{this.state.graphVisible && (
<ResizableBox
width={this.state.graphWidth}
height={Infinity}
minConstraints={[100, Infinity]}
maxConstraints={[this.state.maxWidth, Infinity]}
onResizeStart={this.handleGraphResizeStart}
onResizeStop={this.handleGraphResizeStop}
onResize={this.handleGraphResizeDiv}
resizeHandles={['w']}
className={styles.resizablePanel}
>
<div className={styles.aggregationPanelContainer}>
<GraphPanel
selectedCells={this.state.selectedCells}
order={this.state.order}
data={this.props.data}
columns={this.props.columns}
width={this.state.graphWidth}
/>
</div>
</ResizableBox>
)}
</div>

<BrowserToolbar
Expand Down Expand Up @@ -787,6 +838,8 @@
allClassesSchema={this.state.allClassesSchema}
togglePanel={this.togglePanelVisibility}
isPanelVisible={this.state.isPanelVisible}
toggleGraph={this.toggleGraphVisibility}
isGraphVisible={this.state.graphVisible}
appId={this.props.app.applicationId}
appName={this.props.appName}
{...other}
Expand Down
Loading