-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add custom dashboard canvas with graphs and data tables #3126
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
feat: Add custom dashboard canvas with graphs and data tables #3126
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
📝 WalkthroughWalkthroughAdds a CustomDashboard canvas feature with draggable/resizable elements (StaticText, Graph, DataTable), UI dialogs (add/save/load/config), routes at /canvas, a CanvasPreferencesManager for server/local persistence and migration, related styles, and small store/subscribe fixes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as CustomDashboard
participant AddDlg as AddElementDialog
participant Config as ConfigDialog
participant Elem as CanvasElement
participant Prefs as CanvasPreferencesManager
participant Storage as Server/Local Storage
User->>UI: Click "Add Element"
UI->>AddDlg: open()
AddDlg->>User: show element types
User->>AddDlg: select type
AddDlg->>UI: onSelectType(type)
UI->>Config: open(type-specific config)
User->>Config: submit config
Config->>UI: onSave(config)
UI->>Elem: create element(state)
UI->>User: render new element
User->>Elem: drag/resize
Elem->>UI: onPosition/SizeChange
UI->>Prefs: saveCanvas(appId, canvas)
Prefs->>Storage: persist (server or local)
Storage-->>Prefs: success/fail
Prefs-->>UI: result
UI->>User: show save result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/dashboard/Data/CustomDashboard/CanvasElement.react.js`:
- Around line 12-20: The CanvasElement component currently declares props
onDelete and onEdit but never uses them; remove these unused props from the
function signature (the destructured props list in CanvasElement) and from any
related prop-type or interface declarations (if present) to fix the lint error,
or alternatively wire them into handlers where they should be used; update
references to CanvasElement calls only if they relied on passing those props.
In `@src/dashboard/Data/CustomDashboard/CustomDashboard.react.js`:
- Around line 8-11: The import for Button is unused and causing a lint failure;
remove the unused import statement "import Button from
'components/Button/Button.react';" from CustomDashboard.react.js so only the
needed imports (React, DashboardView, EmptyState) remain, ensuring no other
references to the Button symbol exist in functions/components within this file
(e.g., the CustomDashboard component) before saving.
In `@src/dashboard/Data/CustomDashboard/elements/DataTableConfigDialog.react.js`:
- Around line 56-70: The effect in useEffect that reads initialConfig (checking
initialConfig?.className and initialConfig?.columns) can close over stale values
because initialConfig is not in the dependency array; update the dependency
array of the useEffect that resets selected columns to include initialConfig (or
a stable derivative like initialConfig?.className and initialConfig?.columns) so
the effect re-runs when initialConfig changes, referencing the same symbols:
useEffect, initialConfig, className, availableColumns, setSelectedColumns.
In `@src/dashboard/Data/CustomDashboard/elements/DataTableElement.react.js`:
- Around line 81-108: The prop columns is declared but unused; update the
displayed-column derivation to fall back to the incoming columns prop when
config.columns is absent by changing the displayColumns assignment to use
config.columns || columns || Object.keys(data[0]).filter(k => k !== 'ACL');
ensure the prop name columns is present on the component signature so the
variable is available, and keep using displayColumns in the existing JSX
(thead/tbody) as before.
In `@src/dashboard/Data/CustomDashboard/elements/GraphConfigDialog.react.js`:
- Around line 16-24: The GraphConfigDialog component destructures an unused prop
classSchemas; remove classSchemas from the GraphConfigDialog parameter list and
any associated references (e.g., PropTypes/defaultProps) so the component
signature only accepts used props (initialConfig, availableGraphs,
availableFilters, classes, onClose, onSave); also update any export or PropTypes
declaration named GraphConfigDialog to drop classSchemas to prevent the lint
error.
In `@src/dashboard/Data/CustomDashboard/elements/StaticTextElement.react.js`:
- Around line 18-24: The component StaticTextElement currently destructures
props from config which will throw if config is null/undefined; update the
component to guard against missing config by providing a safe default (e.g., set
a default empty object for config in the function signature or before
destructuring) so that const { text, textSize, isBold, isItalic } is always from
an object and downstream uses (sizeMap[textSize], styles lookups) won’t crash;
locate StaticTextElement and replace the destructuring to use config || {} or
set ({ config = {} }) in the parameter list.
In `@src/dashboard/Data/CustomDashboard/LoadCanvasDialog.react.js`:
- Around line 72-111: The canvas row div rendered in the sortedCanvases map is
not keyboard-accessible; update that element (the one using selectedId and
setSelectedId) to include role="button", tabIndex={0}, and an onKeyDown handler
that triggers the same selection logic as onClick when Enter or Space are
pressed (handle Space with preventDefault to avoid scrolling). Also add an
explicit aria-label to the delete button (the button invoking handleDeleteClick
with canvas.id) to describe the action (e.g., `aria-label="Delete canvas
{canvas.name || 'Untitled Canvas'}"`). Ensure existing handlers
handleConfirmDelete and handleCancelDelete remain unchanged.
In `@src/lib/CanvasPreferencesManager.js`:
- Around line 72-90: The current saveCanvas method silently falls back to local
storage on _saveCanvasToServer failure without updating the user's preference,
causing local saves to become unretrievable by getCanvases; modify the catch
block in saveCanvas so that when _saveCanvasToServer throws you either re-throw
the error (to let UI handle it) or explicitly switch the preference to local by
calling setStoragePreference(appId, STORAGE_TYPES.LOCAL) (and invalidate
serverCanvasesCache[appId] if present) before calling _saveCanvasesToLocal,
ensuring subsequent getCanvases will read from local storage.
🧹 Nitpick comments (4)
src/dashboard/Data/CustomDashboard/elements/StaticTextConfigDialog.react.js (1)
27-32: Consider trimming the text value before saving.The validation at line 28 uses
config.text.trim()butonSave(config)passes the untrimmed text. This could lead to saving text with unintended leading/trailing whitespace. For consistency with other dialogs (e.g.,DataTableConfigDialogwhich usestitle.trim() || null), consider trimming:Suggested fix
const handleSave = () => { if (!config.text.trim()) { return; } - onSave(config); + onSave({ ...config, text: config.text.trim() }); };src/lib/CanvasPreferencesManager.js (1)
225-267: Duplicate server fetch in migration logic.
_migrateCanvasesToServerfetchesexistingCanvasConfigsagain (line 228), but this data was already fetched in the callermigrateToServer(line 137). Consider passingexistingCanvasIdsas a parameter to avoid the redundant network call.Suggested approach
- async _migrateCanvasesToServer(appId, localCanvases, overwriteConflicts) { + async _migrateCanvasesToServer(appId, localCanvases, overwriteConflicts, existingCanvasIds) { try { - // Get existing canvases from server - const existingCanvasConfigs = await this.serverStorage.getConfigsByPrefix('canvases.canvas.', appId); - const existingCanvasIds = Object.keys(existingCanvasConfigs).map(key => - key.replace('canvases.canvas.', '') - ); - // Save local canvases to server // ... rest of the codeThen update the caller in
migrateToServerto pass the already-computedexistingCanvasIds.src/dashboard/Data/CustomDashboard/elements/StaticTextElement.scss (1)
3-8: Consider using a CSS variable for the text color.The hardcoded color
#0e0e1amay not work well if dark theme support is added later. Consider using a CSS variable fromglobals.scssfor better maintainability.Example
.staticText { - color: `#0e0e1a`; + color: var(--color-text-primary, `#0e0e1a`); word-wrap: break-word; overflow-wrap: break-word; white-space: pre-wrap; }src/dashboard/Data/CustomDashboard/CustomDashboard.react.js (1)
692-697: Avoid deprecatedcomponentWillMountfor event listeners.
componentWillMountis deprecated in React 16.14 and can lead to warnings. Move listener registration intocomponentDidMount.♻️ Proposed refactor
componentDidMount() { const { schema } = this.props; schema.dispatch(ActionTypes.FETCH); this.initCanvasPreferencesManager(); // Load classes if schema data is already available (e.g., when navigating back) if (schema && schema.data) { this.loadClasses(); } + document.addEventListener('keydown', this.handleKeyDown); + document.addEventListener('fullscreenchange', this.handleFullscreenChange); + document.addEventListener('webkitfullscreenchange', this.handleFullscreenChange); + document.addEventListener('msfullscreenchange', this.handleFullscreenChange); } ... - componentWillMount() { - document.addEventListener('keydown', this.handleKeyDown); - document.addEventListener('fullscreenchange', this.handleFullscreenChange); - document.addEventListener('webkitfullscreenchange', this.handleFullscreenChange); - document.addEventListener('msfullscreenchange', this.handleFullscreenChange); - }
src/dashboard/Data/CustomDashboard/elements/DataTableConfigDialog.react.js
Outdated
Show resolved
Hide resolved
src/dashboard/Data/CustomDashboard/elements/DataTableElement.react.js
Outdated
Show resolved
Hide resolved
src/dashboard/Data/CustomDashboard/elements/StaticTextElement.react.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/dashboard/Data/CustomDashboard/CustomDashboard.react.js`:
- Around line 306-365: The fetchElementData method can set elementData for an
element that was deleted while the Parse query was in-flight; before calling
setState in both the success and catch blocks, re-check that the element still
exists (e.g., find by elementId against this.state.elements or maintain a local
canceled flag tied to elementId) and skip the state update if it no longer
exists; update the success and error setState paths in fetchElementData to guard
on presence of the element id (or a cancellation marker) so you don't repopulate
elementData for removed elements.
- Around line 678-690: The Delete/Backspace guard in handleKeyDown wrongly
checks document.activeElement === document.body so it ignores a focusable canvas
(tabIndex={0}); update handleKeyDown to instead allow delete when an element is
selected unless the currently focused element is an editable control (e.g.
tagName 'INPUT' or 'TEXTAREA' or has contentEditable='true') or is a form
control; locate handleKeyDown and replace the body-only check with a helper
check like "isEditable(document.activeElement)" (or inline conditions) that
returns true for inputs/textareas/contentEditable and false otherwise so
Delete/Backspace works when the canvas (or other non-editable focusable
elements) has focus while preserving editing fields protection.
- Around line 692-710: The event listener registrations currently in
componentWillMount should be moved to componentDidMount: remove the
document.addEventListener calls from componentWillMount and add them to
componentDidMount so handleKeyDown and handleFullscreenChange are attached after
mount (avoiding deprecated/unsafe lifecycle usage and StrictMode
double-invocation); keep the corresponding document.removeEventListener calls in
componentWillUnmount and retain the existing cleanup for autoReloadTimer and
autoReloadProgressTimer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/GraphPanel/GraphPanel.react.js`:
- Around line 176-179: The memoized chart options (chartOptions) use
disableAnimation when building baseOptions but disableAnimation is missing from
the useMemo dependency array; update the useMemo that returns chartOptions to
include disableAnimation in its dependency list so changes to the
disableAnimation prop re-compute baseOptions and the chart options correctly
(look for the chartOptions useMemo and baseOptions/animation usage and add
disableAnimation to that memo's dependencies).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/dashboard/Data/CustomDashboard/CustomDashboard.react.js`:
- Around line 401-413: The current conditional uses truthiness for limit and
treats 0 or numeric strings as falsy; update the logic around the limit variable
so you explicitly check for null/undefined and coerce/validate numeric values
before calling query.limit: in the block where you reference limit and call
query.limit (use the same identifier "limit" and the "query.limit" method), if
limit is not null/undefined convert it to a Number, ensure it is a finite
integer (or a valid number per your constraints) and pass that numeric value to
query.limit (allow 0), otherwise call query.limit(1000); if conversion fails,
fall back to the default or handle the error consistently.
- Around line 684-713: In handleDeleteCanvas, when deleting the active canvas
(currentCanvasId === canvasId) also clear or mark the in-memory canvas state so
the UI doesn't continue showing its elements: after computing
resetCurrentCanvas, include clearing of current canvas data in setState (e.g.,
set currentCanvasId: null, currentCanvasName: null and also clear any canvas
content state such as canvasElements, canvasData, or set an isUnsaved/isEmpty
flag) and ensure navigateToCanvas(null) remains called in the callback; update
references to the specific state keys used in this component (e.g.,
canvasElements, canvasJSON, isDirty) so the active canvas visuals are removed
when the canvas is deleted.
In `@src/dashboard/Data/CustomDashboard/elements/DataTableElement.react.js`:
- Around line 60-68: In the DataTableElement component, avoid rendering a
non-functional Retry button when the onRefresh prop is not provided: inside the
error render block (where styles.error and styles.retryButton are used),
conditionally render the <button> only if onRefresh is truthy (or alternatively
render a disabled button with aria-disabled and an explanatory title) so the
button is either hidden or clearly disabled when onRefresh is absent; update the
JSX around the existing button to check onRefresh before rendering and ensure
className and onClick are only applied when onRefresh exists.
🧹 Nitpick comments (1)
src/dashboard/Data/CustomDashboard/CustomDashboard.react.js (1)
209-257: Consider parallelizing graph/filter loads to reduce latency.Sequential awaits can add up with many classes.
♻️ Possible refactor
async loadAvailableGraphs() { if (!this.context || !this.context.applicationId) { return; } const graphPreferencesManager = new GraphPreferencesManager(this.context); const graphsByClass = {}; - for (const className of this.state.classes) { - try { - const graphs = await graphPreferencesManager.getGraphs( - this.context.applicationId, - className - ); - if (graphs && graphs.length > 0) { - graphsByClass[className] = graphs; - } - } catch (e) { - console.error(`Error loading graphs for ${className}:`, e); - } - } + await Promise.all( + this.state.classes.map(async className => { + try { + const graphs = await graphPreferencesManager.getGraphs( + this.context.applicationId, + className + ); + if (graphs && graphs.length > 0) { + graphsByClass[className] = graphs; + } + } catch (e) { + console.error(`Error loading graphs for ${className}:`, e); + } + }) + ); this.setState({ availableGraphs: graphsByClass }); } @@ async loadAvailableFilters() { if (!this.context || !this.context.applicationId) { return; } const filterPreferencesManager = new FilterPreferencesManager(this.context); const filtersByClass = {}; - for (const className of this.state.classes) { - try { - const filters = await filterPreferencesManager.getFilters( - this.context.applicationId, - className - ); - if (filters && filters.length > 0) { - filtersByClass[className] = filters; - } - } catch (e) { - console.error(`Error loading filters for ${className}:`, e); - } - } + await Promise.all( + this.state.classes.map(async className => { + try { + const filters = await filterPreferencesManager.getFilters( + this.context.applicationId, + className + ); + if (filters && filters.length > 0) { + filtersByClass[className] = filters; + } + } catch (e) { + console.error(`Error loading filters for ${className}:`, e); + } + }) + ); this.setState({ availableFilters: filtersByClass }); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/dashboard/Data/CustomDashboard/CustomDashboard.react.js`:
- Around line 904-1035: The toolbar uses anchor tags for actions in
renderToolbar which are not keyboard-accessible; replace each interactive <a>
(those that call this.handleNewCanvas, the inline setState opens,
this.handleEditElement, this.handleDeleteElement, this.handleReloadAll, the
auto-reload <select> handler this.handleAutoReloadChange is fine, and
this.toggleFullscreen) with semantic <button type="button"> elements, preserving
className={styles.toolbarButton}, children (Icon + span), and onClick handlers,
and ensure separators and conditional rendering stay unchanged so styling and
behavior remain the same.
- Around line 373-449: The fetchElementData method can update state with stale
async responses or after unmount; add a per-element sequence token (e.g.,
this._elementSeq[elementId] incremented at start of fetch and captured as
localToken) and check that localToken === this._elementSeq[elementId] before any
setState or state write, and also add an instance mounted flag (e.g.,
this._isMounted true in componentDidMount and false in componentWillUnmount) and
ensure you only call setState when this._isMounted is true; update references
inside fetchElementData (and the callers in the forEach reload loops) to
increment the token before starting each request, and check both the token and
mounted flag before writing to elementData in state.
🧹 Nitpick comments (1)
src/dashboard/Data/CustomDashboard/CustomDashboard.react.js (1)
184-257: Consider parallelizing graph/filter loading to avoid waterfall latency.
The per-classawaitinside the loop makes large schemas noticeably slower;Promise.allkeeps UI snappier.♻️ Proposed pattern (apply similarly to filters)
- const graphPreferencesManager = new GraphPreferencesManager(this.context); - const graphsByClass = {}; - - for (const className of this.state.classes) { - try { - const graphs = await graphPreferencesManager.getGraphs( - this.context.applicationId, - className - ); - if (graphs && graphs.length > 0) { - graphsByClass[className] = graphs; - } - } catch (e) { - console.error(`Error loading graphs for ${className}:`, e); - } - } - - this.setState({ availableGraphs: graphsByClass }); + const graphPreferencesManager = new GraphPreferencesManager(this.context); + const results = await Promise.all( + this.state.classes.map(async className => { + try { + const graphs = await graphPreferencesManager.getGraphs( + this.context.applicationId, + className + ); + return [className, graphs]; + } catch (e) { + console.error(`Error loading graphs for ${className}:`, e); + return null; + } + }) + ); + const graphsByClass = {}; + results.forEach(entry => { + if (entry && entry[1]?.length) { + graphsByClass[entry[0]] = entry[1]; + } + }); + this.setState({ availableGraphs: graphsByClass });
# [8.3.0-alpha.9](8.3.0-alpha.8...8.3.0-alpha.9) (2026-01-18) ### Features * Add custom dashboard canvas with graphs and data tables ([#3126](#3126)) ([d45c27b](d45c27b))
|
🎉 This change has been released in version 8.3.0-alpha.9 |
New Pull Request Checklist
Summary by CodeRabbit
New Features
Refactor
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.