Issue 159: datagrid right click#170
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe PR centralizes Jest test configuration into a shared root config and implements a right-click context menu for the data grid editor with auto-fit column width and preview/export format actions. Import/export UI is enhanced to support opening details and triggering workflows from the context menu. ChangesJest Configuration Consolidation
Data Grid Context Menu with Auto-fit and Preview/Export
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/api/jest.config.cjsOops! Something went wrong! :( ESLint: 10.2.1 A config object is using the "root" key, which is not supported in flat config system. Flat configs always act as if they are the root config file, so this key can be safely removed. apps/cli/jest.config.cjsOops! Something went wrong! :( ESLint: 10.2.1 A config object is using the "root" key, which is not supported in flat config system. Flat configs always act as if they are the root config file, so this key can be safely removed. apps/mcp/jest.config.cjsOops! Something went wrong! :( ESLint: 10.2.1 A config object is using the "root" key, which is not supported in flat config system. Flat configs always act as if they are the root config file, so this key can be safely removed.
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Await the grid import before sizing columns
When the sample/instructions actions run against the Tabulator adapter, importer.setGridFromGenericDataTable(dataTable) returns the promise from GridExtensionTabulator.setGridFromGenericDataTable, which enqueues async setColumns/setData. This newly added call runs immediately afterward, so it sizes the previous grid before the imported rows and columns exist; once the import completes, the requested all-column fit is never applied.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds a right-click context menu to the data grid (including preview/export hooks and an “auto fit columns” action), while also moving Jest configuration into dedicated jest.config.cjs files to improve test execution across worktrees and Windows, and updating affected unit/browser tests accordingly.
Changes:
- Introduces a data-grid context menu UI + handlers, plus runtime wiring for preview/export actions via injected services.
- Adds a Tabulator
sizeColumnsToFit()implementation and exposes it through the component/controller APIs. - Centralizes Jest configuration into
jest.config.cjsand updates workspace scripts/tests to reference it; adds/updates Playwright + Jest expectations.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/package.json | Updates core workspace Jest invocation to use the shared config and path patterns. |
| packages/core-ui/src/tests/grid/data-grid-component-view.test.js | Adds view-level test coverage for context menu behavior and routed actions. |
| packages/core-ui/src/tests/grid/data-grid-component-controller.test.js | Extends controller tests to cover the new sizeColumnsToFit() call-through. |
| packages/core-ui/js/gui_components/data-grid-editor/tabulator/gridExtension-tabulator.js | Adds Tabulator implementation for sizeColumnsToFit() and refactors column width estimation. |
| packages/core-ui/js/gui_components/data-grid-editor/index.js | Wires public component API sizeColumnsToFit() through to the controller. |
| packages/core-ui/js/gui_components/data-grid-editor/data-grid-component-view.js | Implements context menu DOM, rendering, event handling, and preview/export routing. |
| packages/core-ui/js/gui_components/data-grid-editor/data-grid-component-controller.js | Adds controller method sizeColumnsToFit() that delegates to grid extras. |
| packages/core-ui/js/gui_components/app/page/app-page-runtime.js | Injects context-menu format items and routes preview/export actions through the import/export workspace. |
| packages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-view.js | Adds view helpers to open the toolbar <details> section. |
| packages/core-ui/js/gui_components/app/import-export-workspace/create-import-export-workspace-runtime.js | Exposes openDetails(), previewAs(), and downloadAs() controller-style helpers. |
| package.json | Removes inline Jest config and updates scripts to use jest.config.cjs. |
| jest.config.cjs | Adds centralized Jest configuration previously embedded in package.json. |
| apps/web/styles.css | Adds styling for the new data-grid context menu. |
| apps/web/src/tests/jest/script-module-init.test.js | Updates bootstrap expectations to allow injected context menu services. |
| apps/web/src/tests/jest/script-bootstrap.test.js | Updates bootstrap expectations to allow injected context menu services. |
| apps/web/src/tests/browser/app/functional/grid-editor/context-menu/right-click-context-menu.spec.js | Adds Playwright functional coverage for right-click menu actions (add row/preview/export). |
| apps/web/src/tests/browser/app/abstractions/components/grid-editor.component.js | Extends browser test abstraction with context-menu helpers. |
| apps/mcp/package.json | Updates MCP workspace test script to use a local Jest config. |
| apps/mcp/jest.config.cjs | Adds workspace Jest config extending the base config. |
| apps/cli/src/tests/integration.cli-params.test.js | Adjusts integration test timeout to reduce flakiness. |
| apps/cli/jest.config.cjs | Switches CLI workspace Jest config to extend the base config and adjust rootDir/testMatch. |
| apps/api/jest.config.cjs | Switches API workspace Jest config to extend the base config and adjust rootDir/testMatch. |
| openContextMenu(x, y) { | ||
| this.contextMenuState = { open: true, x, y }; | ||
| this.renderContextMenu(); | ||
| } |
Greptile SummaryThis PR adds a right-click context menu to the data-grid editor, wiring row-management, filter, sort, and column-fit actions through the existing controller layer. It also centralises Jest configuration into a root
Confidence Score: 5/5Safe to merge; no correctness regressions on the changed paths. The context-menu feature is self-contained: event listeners are attached and cleaned up symmetrically, actions delegate to the existing controller API without bypassing any guards, and the new browser spec exercises the two most important flows. The Jest config consolidation is mechanical and well-tested by the existing suite. The only issues found are minor edge-case observations on rarely-hit code paths. No files require special attention. Important Files Changed
Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'origin/mas..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…click # Conflicts: # apps/api/jest.config.cjs # apps/cli/jest.config.cjs # apps/cli/src/tests/integration.cli-params.test.js # apps/mcp/jest.config.cjs # package.json
Summary
Verification
Summary by CodeRabbit
New Features
Tests