feat: Add context menu in data browser to add cell content or selected text to Cloud Config parameter #3123
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. |
✅ 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. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds "Add to config parameter..." context-menu flow: threads arrayConfigParams and onAddToArrayConfig through UI components, captures Alt+right-click in cells, string editors, and aggregation panels, shows a parameter-selection menu, opens AddArrayEntryDialog, and performs AddUnique updates to cloud config. Changes
Sequence DiagramsequenceDiagram
participant User
participant Cell as BrowserCell / StringEditor
participant Context as ContextMenu
participant DataBrowser as DataBrowser / AggregationPanel
participant Browser as Browser (state & handlers)
participant Dialog as AddArrayEntryDialog
participant Config as Config Store / Server
User->>Cell: Alt + Right-click (selected text)
Cell->>Cell: handleContextMenu(e) / build menu items
Cell->>Context: setContextMenu({position, items})
User->>Context: Click "Add to config parameter..." → choose param
Context->>Browser: onAddToArrayConfig(paramName, value)
Browser->>Browser: handleAddToArrayConfig(param, value) (open dialog)
Browser->>Dialog: show dialog with initialValue
User->>Dialog: Confirm add entry
Dialog->>Browser: handleConfirmAddToConfig(finalValue)
Browser->>Config: PUT config with AddUnique operation
Config-->>Browser: Success / Error
Browser->>Browser: fetchArrayConfigParams() refresh & close dialog
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
mtrezza
changed the title
Add to config parameter to add cell content or selected text to Cloud Config parameter
mtrezza
changed the title
Add to config parameter to add cell content or selected text to Cloud Config parameter![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/AggregationPanel/AggregationPanel.js (1)
153-190: Wire the context menu for nested panels, too.
onContextMenuis forwarded (Line 141), but the depth>0 render path never attaches it, so Alt+right‑click won’t surface the add‑to‑config menu inside nested panels.🐛 Proposed fix
- {isExpanded && ( - <div className={styles.nestedPanelContent}> + {isExpanded && ( + <div className={styles.nestedPanelContent} onContextMenu={onContextMenu}>
🤖 Fix all issues with AI agents
In `@src/components/BrowserCell/BrowserCell.react.js`:
- Around line 633-657: The menu builder getAddToConfigContextMenuOption can leak
real values for hidden cells because it only checks the stringified
copyableValue; add an explicit guard that returns early when the cell is hidden
(e.g., check this.props.hidden or an equivalent per-cell hidden flag) before
computing cellValue so the "Add to config parameter..." option is suppressed for
hidden fields; update the guard alongside arrayConfigParams and
onAddToArrayConfig checks and leave the rest of the mapping
(arrayConfigParams.map and onAddToArrayConfig callback) unchanged.
In `@src/components/ContextMenu/ContextMenu.react.js`:
- Around line 123-131: The submenu vertical offset is currently computed from
the index (path[level] * 30), which assumes fixed item height and breaks when
separators exist; update the mouse/hover handler that opens submenus (e.g., the
function handling item hover like
handleMouseEnter/openSubmenu/setSubmenuPosition) to compute the submenu top
using the hovered element's actual position (use event.currentTarget.offsetTop
relative to the parent menu or subtract parent.getBoundingClientRect().top from
event.currentTarget.getBoundingClientRect().top) and store that value instead of
index*30 so submenu alignment accounts for separators and variable item heights.
In `@src/dashboard/Data/Browser/Browser.react.js`:
- Around line 327-329: The component currently calls
this.fetchArrayConfigParams() only on mount, so arrayConfigParams can go stale
when the passed-in appId changes; add logic to componentDidUpdate(prevProps)
that compares prevProps.appId !== this.props.appId and, when different, calls
this.fetchArrayConfigParams() (and any related refresh methods) so the context
menu targets the correct app; apply the same change to the other Browser
instance/section that uses fetchArrayConfigParams (lines referenced around the
other block) to ensure both locations refresh on appId changes.
In `@src/dashboard/Data/Config/AddArrayEntryDialog.react.js`:
- Around line 17-21: The component's constructor sets state.value from
props.initialValue but doesn't normalize to a string or keep it in sync when
props change; update the constructor to coerce props.initialValue to a string
for state.value (e.g., String(props.initialValue || '')) and add a
componentDidUpdate(prevProps) that checks if prevProps.initialValue !==
this.props.initialValue and, if so, sets state.value to the normalized string of
this.props.initialValue; make sure this fixes TextInput's expectation of a
string and prevents stale state while the dialog is mounted.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/dashboard/Data/Browser/Browser.react.js`:
- Around line 2573-2600: The async method fetchArrayConfigParams sets state
after an await without checking component mount status; before calling
this.setState({ arrayConfigParams: arrayParams }) in fetchArrayConfigParams,
guard the update with the existing mount flag (this._isMounted) used elsewhere
(e.g., loadAllClassFilters) so you only call this.setState when this._isMounted
is true to avoid setting state on an unmounted component.
♻️ Duplicate comments (2)
src/components/BrowserCell/BrowserCell.react.js (1)
633-663: Hidden cell guard has been properly implemented.The method now correctly checks for
hiddenprop at the beginning (line 637-639) and returns early, preventing sensitive hidden field values from being exposed through the config menu. This addresses the security concern from the previous review.The filtering of special values (
(undefined),(null),(hidden)) at line 650 provides additional defense-in-depth.src/dashboard/Data/Browser/Browser.react.js (1)
327-328: Refetch config params when switching apps.
fetchArrayConfigParams()is only called on mount. When the user switches apps (appId changes), thearrayConfigParamsstate will contain stale data from the previous app, causing the context menu to target wrong config parameters.Add the call in
componentWillReceivePropswhen appId changes:🐛 Proposed fix
componentWillReceiveProps(nextProps, nextContext) { if (nextProps.params.appId !== this.props.params.appId) { this.removeLocation(); this.addLocation(nextProps.params.appId); + this.fetchArrayConfigParams(); }
🧹 Nitpick comments (1)
src/dashboard/Data/Browser/Browser.react.js (1)
2612-2632: Consider escaping quotes in the prefill value.When
lastType === 'string', the code wraps the selected text in quotes (line 2623). IfselectedTextcontains embedded quotes (e.g.,foo"bar), the resulting prefill value"foo"bar"would be invalid JSON. WhileAddArrayEntryDialoghandles this gracefully by falling back to the raw string, this may confuse users expecting the value to be pre-parsed correctly.♻️ Optional: Escape embedded quotes
// If the last item in the array is a string, wrap the prefilled text in quotes let prefillValue = selectedText; if (lastType === 'string') { - prefillValue = `"${selectedText}"`; + prefillValue = JSON.stringify(selectedText); }Using
JSON.stringifywill properly escape embedded quotes, newlines, and other special characters.
# [8.3.0-alpha.6](8.3.0-alpha.5...8.3.0-alpha.6) (2026-01-17) ### Features * Add context menu in data browser to add cell content or selected text to Cloud Config parameter ([#3123](#3123)) ([9bc5197](9bc5197))
|
🎉 This change has been released in version 8.3.0-alpha.6 |
2 participants
New Pull Request Checklist
Feature
Adds "Add to config parameter..." context menu item to add cell content to Cloud Config array. When right-clicking on selected text inside the cell while in edit mode, or on selected text in the info panel, hold the
Altkey while clicking to show the custom context menu.Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.