151 continue refactoring UI into components#152
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 2 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughMoves import/export orchestration into a workspace component with service-based preview/import/export flows; removes legacy controls and imperative options panels; introduces a declarative FormatOptionsPanel system; updates Storybook stories, controllers, views, and tests to the new contracts. ChangesWorkspace & Format-Options Migration
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the Import/Export workspace to fully own its behavior via controller/view/services, removing remaining legacy *Controls orchestration and updating tests/stories/docs to match the new component boundaries (closes #151).
Changes:
- Removed legacy
ImportExportControls,ExportControls, and the export-actions adapter; workspace now coordinates preview/edit, import, file read, copy, and download via injected services. - Introduced workspace services + controller state model for preview row limiting, preview-then-import flow, clipboard/download, and “yield-to-UI”.
- Reworked Jest tests and Storybook stories/harnesses to exercise the new component-owned behavior and updated migration documentation.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core-ui/src/tests/utils/import-export-controls-mode.test.js | Removed legacy-control-focused tests after migrating behavior into workspace components/services. |
| packages/core-ui/src/tests/utils/export-controls.test.js | Removed legacy ExportControls tests now that download/copy are workspace services. |
| packages/core-ui/src/tests/app/import-export-workspace.test.js | Updated workspace tests to validate new component-owned behavior (preview limits, dirty state, file read, download/copy). |
| packages/core-ui/src/tests/app/import-export-workspace-services.test.js | Added unit tests for the new workspace service functions (normalization, preview text, preview-then-import). |
| packages/core-ui/src/tests/app/import-export-workspace-controller.test.js | Added controller tests for normalized limits and state transitions/guards. |
| packages/core-ui/src/tests/app/import-export-adapters.test.js | Removed adapter test coverage tied to deleted export-actions adapter. |
| packages/core-ui/js/gui_components/app/text-preview-editor/text-preview-editor-view.js | Added text-input + copy button event wiring and view helpers for text/copy. |
| packages/core-ui/js/gui_components/app/text-preview-editor/text-preview-editor-controller.js | Added controller callbacks for text input and copy actions. |
| packages/core-ui/js/gui_components/app/text-preview-editor/index.js | Exposed text/copy helpers on the component API for workspace coordination. |
| packages/core-ui/js/gui_components/app/import-export-workspace/index.js | Replaced legacy controls with workspace-owned orchestration (services, busy/status, preview/import/download/copy). |
| packages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-view.js | Added options panel + splitter behavior and integrates toolbar/editor updates via state. |
| packages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-services.js | Introduced shared service helpers (row limiting, preview text, clipboard/download, yield-to-UI, preview-then-import). |
| packages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-controller.js | Expanded state model and added state helpers (busy/status/support state, dirty tracking). |
| packages/core-ui/js/gui_components/app/import-export-toolbar/index.js | Extended toolbar component to accept callbacks/services for actions and file bindings. |
| packages/core-ui/js/gui_components/app/import-export-toolbar/import-export-toolbar-view.js | Toolbar now binds buttons, manages file bindings, and renders busy/status/support state. |
| packages/core-ui/js/gui_components/app/import-export-toolbar/import-export-toolbar-controller.js | Added toolbar state fields and canSetGridFromText() logic. |
| packages/core-ui/js/gui_components/app/import-export-controls.js | Deleted legacy coordinator (replaced by workspace controller/services). |
| packages/core-ui/js/gui_components/app/import-export-adapters/index.js | Removed exports for deleted export-actions adapter utilities. |
| packages/core-ui/js/gui_components/app/import-export-adapters/export-actions-adapter.js | Deleted legacy export/copy/download adapter. |
| packages/core-ui/js/gui_components/app/exportControls.js | Deleted legacy ExportControls class. |
| docs/frontend-legacy-ui-elimination-plan.md | Marked Phase 0/1 items complete and documented the new workspace/component-owned state. |
| docs/frontend-component-migration-plan.md | Added status note directing readers to the stricter legacy elimination plan. |
| docs/frontend-component-architecture.md | Updated Storybook exception note now that export-preview harness no longer requires document scoping hacks. |
| apps/web/src/stories/text-preview-editor.stories.js | Updated docs description to reflect expanded TextPreviewEditor responsibilities. |
| apps/web/src/stories/import-export-workspace.stories.js | Added stories/args for busy/error/unsupported states and aligned story setup with new API. |
| apps/web/src/stories/export-preview-story-harness.js | Updated harness to mount the workspace directly and removed scoped-document monkey patching. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/stories/export-preview-story-harness.js (2)
441-454:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
onSetGridFromTextFailedcannot fire from this listener.This handler only awaits
Promise.resolve(), so thecatchblock never observes the real import/parsing failure from the workspace click path. As written, the failure action is dead and the story cannot surface import errors through this callback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/stories/export-preview-story-harness.js` around lines 441 - 454, The click listener attached to setGridFromTextButton only awaits Promise.resolve(), so real parse/import errors never reach the catch and onSetGridFromTextFailed cannot fire; replace the noop await with the actual import/parsing call used by the workspace click flow (the same function that consumes markdownarea.value and the export type from getActiveExportSelection(surface)), await its promise inside the try, and on failure emitAction('onSetGridFromTextFailed', { type, message: error?.message || 'Unable to parse input into data table.', sourceTextLength: textToImport.length }) so errors from the import/parsing routine are surfaced.
419-438:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid double-emitting
onPreviewRenderedfor one button click.This button already goes through the workspace handler, and the overridden
workspace.renderTextFromGridemitsonPreviewRenderedonce. Emitting it again here makes one user action produce two preview-render events, which pollutes Storybook actions and makes interaction assertions flaky.Proposed fix
setTextFromGridButton?.addEventListener('click', () => { emitAction('onSetTextFromGrid', { ...getPreviewActionPayload( surface, exporter, workspace.getPreviewRowLimit(), workspace.isPreviewTextMode() ? 'preview' : 'edit' ), trigger: 'button', }); - emitAction( - 'onPreviewRendered', - getPreviewActionPayload( - surface, - exporter, - workspace.getPreviewRowLimit(), - workspace.isPreviewTextMode() ? 'preview' : 'edit' - ) - ); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/stories/export-preview-story-harness.js` around lines 419 - 438, The click handler for setTextFromGridButton is double-emitting onPreviewRendered; remove the extra emitAction('onPreviewRendered', ...) call in the setTextFromGridButton click listener (leave the emitAction('onSetTextFromGrid', ...) and the getPreviewActionPayload usage intact) so that the overridden workspace.renderTextFromGrid continues to emit onPreviewRendered once and the button click does not emit it again.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/stories/export-preview-story-harness.js`:
- Around line 332-339: The downloadService.downloadText mock ignores the
exported text argument and computes textLength from the DOM; update the
downloadText implementation (the downloadService.downloadText function in this
harness) to accept and use the second parameter (text) passed by callers and set
textLength based on that text (e.g., text?.length || 0) when calling
emitAction('onDownloadRequested'), while still preserving fileExtension via
filename.replace and including the export type from
getActiveExportSelection(surface).
In `@docs/frontend-component-migration-plan.md`:
- Around line 12-14: The Phase 6 status text in
docs/frontend-component-migration-plan.md is stale and contradicts the new
guidance that legacy UI may still be present; update the Phase 6 status section
to reflect current reality by removing the explicit claim that the workspace
still uses the old import/export control path (or clearly mark it as
partial/legacy-present) and add a line instructing maintainers to treat updates
to this document as part of the Definition of Done for migration tasks,
referencing the follow-on plan docs/frontend-legacy-ui-elimination-plan.md for
stricter legacy-elimination steps so readers see consistent, actionable status
and next-step ownership.
In
`@packages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-services.js`:
- Around line 115-116: The code currently uses optional chaining so a missing
importer.setGridFromGenericDataTable is treated as success; change this to
enforce the contract by verifying importer exists and typeof
importer.setGridFromGenericDataTable === 'function' before calling it (and if
missing, call setImportStatus with an error message and false/abort), then await
importer.setGridFromGenericDataTable(dataTable) normally and only call
setImportStatus('Import complete.', false) after the awaited call succeeds;
reference the importer object and setGridFromGenericDataTable method and the
setImportStatus call to locate where to add the guard and error path.
In `@packages/core-ui/js/gui_components/app/import-export-workspace/index.js`:
- Around line 372-379: renderOptionsPanel() is never called on initial mount or
when the exporter is changed, so format-specific options are missing until
update() runs; ensure renderOptionsPanel is invoked after view.mount() completes
and inside setExporter() (the exporter-change path) in addition to the existing
update() flow. Specifically, after view.mount() and after setExporter(...)
finishes, call renderOptionsPanel() (and any helper like
setOptionsViewForFormatType() if needed) so the options panel initializes on
first render and when the exporter changes; keep the existing
controller.updateProps/ syncSupportState/ setOptionsViewForFormatType calls in
update() untouched.
- Around line 75-82: syncSupportState currently enables import/export when
adapters are missing by defaulting supportsImport/supportsExport to true; change
the logic so missing importer/exporter are treated as unsupported: compute
supportsImport by returning false if importer is falsy, otherwise call
importer.canImport(type) if it’s a function (false if no function), and compute
supportsExport similarly for exporter; for fileExtension only call
importer.getFileExtensionFor or exporter.getFileExtensionFor when those adapters
exist and fall back to `.${type || 'csv'}` if neither provides one; then call
controller.setSupportState({ supportsImport, supportsExport, fileExtension })
and render() as before.
- Around line 137-176: The edit-mode path in importTextArea calls await
importer.importText(type, text) without setting the busy state, updating
importStatus, or catching errors; wrap that call in the same busy/error flow
used by preview: call setImportBusy(true) before the async call, await the
import inside a try/catch, on success call setImportStatus('Import complete',
true) and render(), and in finally call setImportBusy(false); in the catch log
the error, call setImportStatus('Import failed. Check file format/options.',
false) and return undefined (or false) to mirror the preview branch behavior;
ensure you reference importTextArea, importer.importText, setImportBusy,
setImportStatus, render, and yieldToUi if needed.
- Around line 407-409: setFileFormatType currently just aliases syncSupportState
so calling setFileFormatType('json') doesn't change selectedFormat; modify
setFileFormatType to perform the same state update path used by the format
selector (i.e. set the controller's selectedFormat state to the passed value)
and then call syncSupportState if a re-sync is required. Specifically, change
setFileFormatType to update the selectedFormat variable/state used by the format
selector (the same setter or dispatch used in the selector UI) and only delegate
to syncSupportState for resync behavior, ensuring the controller reflects the
requested format immediately.
---
Outside diff comments:
In `@apps/web/src/stories/export-preview-story-harness.js`:
- Around line 441-454: The click listener attached to setGridFromTextButton only
awaits Promise.resolve(), so real parse/import errors never reach the catch and
onSetGridFromTextFailed cannot fire; replace the noop await with the actual
import/parsing call used by the workspace click flow (the same function that
consumes markdownarea.value and the export type from
getActiveExportSelection(surface)), await its promise inside the try, and on
failure emitAction('onSetGridFromTextFailed', { type, message: error?.message ||
'Unable to parse input into data table.', sourceTextLength: textToImport.length
}) so errors from the import/parsing routine are surfaced.
- Around line 419-438: The click handler for setTextFromGridButton is
double-emitting onPreviewRendered; remove the extra
emitAction('onPreviewRendered', ...) call in the setTextFromGridButton click
listener (leave the emitAction('onSetTextFromGrid', ...) and the
getPreviewActionPayload usage intact) so that the overridden
workspace.renderTextFromGrid continues to emit onPreviewRendered once and the
button click does not emit it again.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e5bb0ef-992c-48b6-b431-46f678efc832
📒 Files selected for processing (26)
apps/web/src/stories/export-preview-story-harness.jsapps/web/src/stories/import-export-workspace.stories.jsapps/web/src/stories/text-preview-editor.stories.jsdocs/frontend-component-architecture.mddocs/frontend-component-migration-plan.mddocs/frontend-legacy-ui-elimination-plan.mdpackages/core-ui/js/gui_components/app/exportControls.jspackages/core-ui/js/gui_components/app/import-export-adapters/export-actions-adapter.jspackages/core-ui/js/gui_components/app/import-export-adapters/index.jspackages/core-ui/js/gui_components/app/import-export-controls.jspackages/core-ui/js/gui_components/app/import-export-toolbar/import-export-toolbar-controller.jspackages/core-ui/js/gui_components/app/import-export-toolbar/import-export-toolbar-view.jspackages/core-ui/js/gui_components/app/import-export-toolbar/index.jspackages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-controller.jspackages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-services.jspackages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-view.jspackages/core-ui/js/gui_components/app/import-export-workspace/index.jspackages/core-ui/js/gui_components/app/text-preview-editor/index.jspackages/core-ui/js/gui_components/app/text-preview-editor/text-preview-editor-controller.jspackages/core-ui/js/gui_components/app/text-preview-editor/text-preview-editor-view.jspackages/core-ui/src/tests/app/import-export-adapters.test.jspackages/core-ui/src/tests/app/import-export-workspace-controller.test.jspackages/core-ui/src/tests/app/import-export-workspace-services.test.jspackages/core-ui/src/tests/app/import-export-workspace.test.jspackages/core-ui/src/tests/utils/export-controls.test.jspackages/core-ui/src/tests/utils/import-export-controls-mode.test.js
💤 Files with no reviewable changes (7)
- packages/core-ui/src/tests/utils/import-export-controls-mode.test.js
- packages/core-ui/src/tests/utils/export-controls.test.js
- packages/core-ui/js/gui_components/app/import-export-adapters/export-actions-adapter.js
- packages/core-ui/src/tests/app/import-export-adapters.test.js
- packages/core-ui/js/gui_components/app/import-export-controls.js
- packages/core-ui/js/gui_components/app/exportControls.js
- packages/core-ui/js/gui_components/app/import-export-adapters/index.js
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-definition-shared.js`:
- Around line 163-170: The default for the asPropertyNamed option is an empty
string causing {"": [...] } when asObject is used; change the defaultValue for
the option with key 'asPropertyNamed' (name 'propertynamed', className
'propertynamed') from '' to a sensible default like 'data' so object-mode output
uses a valid property name when users don't edit it.
In
`@packages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-definition.js`:
- Around line 9-15: resolveMappedValue currently returns fallback directly when
value is undefined, breaking the expected {selected, custom} shape used by the
consumers; update resolveMappedValue so it always returns an object with
selected and custom properties—if fallback is provided and already matches the
shape accept it, otherwise return a safe default like { selected: 'custom',
custom: '' }—so callers of resolveMappedValue (the function named
resolveMappedValue referenced around the select/custom input logic) never
receive a raw non-object fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dc14a79e-1fef-4bdf-96d5-ffed7a57acd6
📒 Files selected for processing (78)
apps/web/src/stories/export-preview-story-harness.jsapps/web/src/stories/format-option-panel-basic.stories.jsapps/web/src/tests/jest/utils/help-content-coverage.test.jsdocs/frontend-component-architecture.mddocs/frontend-legacy-ui-elimination-plan.mdpackages/core-ui/js/gui_components/generator/options/options-ui-schema.jspackages/core-ui/js/gui_components/options_panels/html-options-data-utils.jspackages/core-ui/js/gui_components/options_panels/options-ascii-table.jspackages/core-ui/js/gui_components/options_panels/options-csharp-panel.jspackages/core-ui/js/gui_components/options_panels/options-csv-delimited-controls.jspackages/core-ui/js/gui_components/options_panels/options-delimited-controls.jspackages/core-ui/js/gui_components/options_panels/options-gherkin-panel.jspackages/core-ui/js/gui_components/options_panels/options-help-tips.jspackages/core-ui/js/gui_components/options_panels/options-html-panel.jspackages/core-ui/js/gui_components/options_panels/options-java-panel.jspackages/core-ui/js/gui_components/options_panels/options-javascript-panel.jspackages/core-ui/js/gui_components/options_panels/options-json-panel.jspackages/core-ui/js/gui_components/options_panels/options-kotlin-panel.jspackages/core-ui/js/gui_components/options_panels/options-markdown-panel.jspackages/core-ui/js/gui_components/options_panels/options-perl-panel.jspackages/core-ui/js/gui_components/options_panels/options-php-panel.jspackages/core-ui/js/gui_components/options_panels/options-python-panel.jspackages/core-ui/js/gui_components/options_panels/options-ruby-panel.jspackages/core-ui/js/gui_components/options_panels/options-sql-panel.jspackages/core-ui/js/gui_components/options_panels/options-test-framework-panel.jspackages/core-ui/js/gui_components/options_panels/options-typescript-panel.jspackages/core-ui/js/gui_components/options_panels/options-xml-panel.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/asciitable-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/csv-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/dsv-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/gherkin-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/html-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/index.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/json-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/markdown-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/sql-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/base/xml-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code-unit-test/test-framework-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/csharp-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/index.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/java-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/javascript-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/kotlin-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/perl-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/php-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/python-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/ruby-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/code/typescript-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/format-option-help.jspackages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-definition-shared.jspackages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-definition.jspackages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-dom.jspackages/core-ui/js/gui_components/shared/format-options-panel/format-options-panel-view.jspackages/core-ui/js/gui_components/shared/format-options-panel/index.jspackages/core-ui/src/tests/shared/format-option-panel-definition.test.jspackages/core-ui/src/tests/shared/format-options-panel-view.test.jspackages/core-ui/src/tests/utils/html-options-data-utils.test.jspackages/core-ui/src/tests/utils/options-ascii-table.test.jspackages/core-ui/src/tests/utils/options-csharp-panel.test.jspackages/core-ui/src/tests/utils/options-csv-delimited-controls.test.jspackages/core-ui/src/tests/utils/options-delimited-controls.test.jspackages/core-ui/src/tests/utils/options-gherkin-panel.test.jspackages/core-ui/src/tests/utils/options-help-parity.test.jspackages/core-ui/src/tests/utils/options-html-panel.test.jspackages/core-ui/src/tests/utils/options-java-panel.test.jspackages/core-ui/src/tests/utils/options-javascript-panel.test.jspackages/core-ui/src/tests/utils/options-json-panel.test.jspackages/core-ui/src/tests/utils/options-kotlin-panel.test.jspackages/core-ui/src/tests/utils/options-markdown-panel.test.jspackages/core-ui/src/tests/utils/options-panel-nesting.test.jspackages/core-ui/src/tests/utils/options-perl-panel.test.jspackages/core-ui/src/tests/utils/options-php-panel.test.jspackages/core-ui/src/tests/utils/options-python-panel.test.jspackages/core-ui/src/tests/utils/options-ruby-panel.test.jspackages/core-ui/src/tests/utils/options-sql-panel.test.jspackages/core-ui/src/tests/utils/options-test-framework-panel.test.jspackages/core-ui/src/tests/utils/options-typescript-panel.test.jspackages/core-ui/src/tests/utils/options-xml-panel.test.js
💤 Files with no reviewable changes (44)
- packages/core-ui/js/gui_components/options_panels/options-csharp-panel.js
- packages/core-ui/src/tests/utils/options-javascript-panel.test.js
- packages/core-ui/src/tests/utils/options-python-panel.test.js
- packages/core-ui/js/gui_components/options_panels/options-gherkin-panel.js
- packages/core-ui/js/gui_components/options_panels/options-kotlin-panel.js
- packages/core-ui/js/gui_components/options_panels/options-help-tips.js
- packages/core-ui/src/tests/utils/options-ruby-panel.test.js
- packages/core-ui/src/tests/utils/options-delimited-controls.test.js
- packages/core-ui/js/gui_components/options_panels/options-json-panel.js
- packages/core-ui/src/tests/utils/options-json-panel.test.js
- packages/core-ui/src/tests/utils/options-kotlin-panel.test.js
- packages/core-ui/js/gui_components/options_panels/options-ruby-panel.js
- packages/core-ui/src/tests/utils/options-perl-panel.test.js
- packages/core-ui/js/gui_components/options_panels/options-xml-panel.js
- packages/core-ui/src/tests/utils/options-panel-nesting.test.js
- packages/core-ui/src/tests/utils/options-html-panel.test.js
- packages/core-ui/src/tests/utils/options-sql-panel.test.js
- packages/core-ui/js/gui_components/options_panels/options-ascii-table.js
- packages/core-ui/src/tests/utils/options-ascii-table.test.js
- packages/core-ui/src/tests/utils/options-xml-panel.test.js
- packages/core-ui/src/tests/utils/options-help-parity.test.js
- packages/core-ui/js/gui_components/options_panels/options-python-panel.js
- packages/core-ui/js/gui_components/options_panels/options-javascript-panel.js
- packages/core-ui/js/gui_components/options_panels/options-typescript-panel.js
- packages/core-ui/src/tests/utils/options-test-framework-panel.test.js
- packages/core-ui/js/gui_components/options_panels/options-html-panel.js
- packages/core-ui/src/tests/utils/options-php-panel.test.js
- packages/core-ui/src/tests/utils/options-markdown-panel.test.js
- packages/core-ui/js/gui_components/options_panels/options-markdown-panel.js
- packages/core-ui/src/tests/utils/options-gherkin-panel.test.js
- packages/core-ui/js/gui_components/options_panels/options-csv-delimited-controls.js
- packages/core-ui/src/tests/utils/options-typescript-panel.test.js
- packages/core-ui/js/gui_components/options_panels/options-delimited-controls.js
- packages/core-ui/js/gui_components/options_panels/options-java-panel.js
- packages/core-ui/js/gui_components/options_panels/options-sql-panel.js
- packages/core-ui/js/gui_components/options_panels/options-perl-panel.js
- packages/core-ui/js/gui_components/options_panels/options-test-framework-panel.js
- packages/core-ui/src/tests/utils/html-options-data-utils.test.js
- packages/core-ui/js/gui_components/options_panels/options-php-panel.js
- packages/core-ui/src/tests/utils/options-java-panel.test.js
- packages/core-ui/src/tests/utils/options-csv-delimited-controls.test.js
- packages/core-ui/js/gui_components/options_panels/html-options-data-utils.js
- packages/core-ui/src/tests/utils/options-csharp-panel.test.js
- apps/web/src/stories/export-preview-story-harness.js
✅ Files skipped from review due to trivial changes (9)
- packages/core-ui/js/gui_components/shared/format-options-panel/base/xml-definition.js
- packages/core-ui/js/gui_components/shared/format-options-panel/base/dsv-definition.js
- packages/core-ui/js/gui_components/shared/format-options-panel/base/markdown-definition.js
- packages/core-ui/src/tests/shared/format-option-panel-definition.test.js
- packages/core-ui/js/gui_components/shared/format-options-panel/base/csv-definition.js
- packages/core-ui/js/gui_components/shared/format-options-panel/code/kotlin-definition.js
- docs/frontend-component-architecture.md
- apps/web/src/stories/format-option-panel-basic.stories.js
- docs/frontend-legacy-ui-elimination-plan.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core-ui/src/tests/app/import-export-workspace-services.test.js`:
- Line 107: Replace the flaky last-call assertion with a never-emitted
assertion: in the test that references setImportStatus (in
import-export-workspace-services.test.js), change the assertion using
not.toHaveBeenLastCalledWith('Import complete.', false) to
not.toHaveBeenCalledWith('Import complete.', false) so the test guarantees that
the exact status tuple was never emitted at any point (locate the expect call
referencing setImportStatus and update the matcher accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ebaabb6f-939e-4a7d-8c28-cee680081bd4
📒 Files selected for processing (10)
apps/web/src/stories/export-preview-story-harness.jsdocs/frontend-component-migration-plan.mdpackages/core-ui/js/gui_components/app/import-export-toolbar/import-export-toolbar-view.jspackages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-services.jspackages/core-ui/js/gui_components/app/import-export-workspace/index.jspackages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-definition-shared.jspackages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-definition.jspackages/core-ui/src/tests/app/import-export-workspace-services.test.jspackages/core-ui/src/tests/app/import-export-workspace.test.jspackages/core-ui/src/tests/shared/format-option-panel-definition.test.js
✅ Files skipped from review due to trivial changes (1)
- docs/frontend-component-migration-plan.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/core-ui/src/tests/app/import-export-workspace.test.js
- packages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-definition-shared.js
- packages/core-ui/js/gui_components/app/import-export-toolbar/import-export-toolbar-view.js
- packages/core-ui/js/gui_components/app/import-export-workspace/import-export-workspace-services.js
- packages/core-ui/js/gui_components/shared/format-options-panel/format-option-panel-definition.js
- packages/core-ui/js/gui_components/app/import-export-workspace/index.js
- apps/web/src/stories/export-preview-story-harness.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #151
Summary by CodeRabbit
Refactor
Stories
Documentation
Tests