Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a configurable ChangesClipboard Range-Fill Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant OverlaySelection
participant ColumnService
participant DataStoreRow
participant DataStoreCol
participant RevoGrid
User->>OverlaySelection: paste event (clipboard data)
OverlaySelection->>OverlaySelection: normalize via getRangeFillClipboardData
OverlaySelection->>ColumnService: paste({ start, data, targetRange? })
ColumnService->>DataStoreRow: compute per-row changes (skip readonly)
ColumnService->>DataStoreCol: compute per-column mappings
ColumnService-->>OverlaySelection: return { changed, range|null }
OverlaySelection->>RevoGrid: apply changed cells / rerender
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: c6bf2b0f91
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/data/column.service.ts (1)
285-316:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn
nullwhen every target cell is skipped.Lines 304-305 advance
maxColandlastRoweven when the inner loop never writes a cell, so a fully read-only paste target still returns a concreterange. That breaks the newRangeArea | nullcontract and makessrc/components/overlay/revogr-overlay-selection.tsxLine 864 treat a no-op paste as applied.Suggested fix
const endCol = Math.min( colLength - 1, targetRange?.x1 ?? start.x + copyColLength - 1, ); // columns + let rowWritten = false; let colIndex = startCol; for (let j = 0; colIndex <= endCol; colIndex++, j++) { const p = this.columns[colIndex].prop; const currentCol = j % copyColLength; /** if can write */ if (!this.isReadOnly(rowIndex, colIndex)) { /** to show before save */ if (!changed[rowIndex]) { changed[rowIndex] = {}; } changed[rowIndex][p] = copyRow[currentCol]; + rowWritten = true; + maxCol = Math.max(maxCol, colIndex); } } - maxCol = Math.max(maxCol, colIndex - 1); - lastRow = rowIndex; + if (rowWritten) { + lastRow = rowIndex; + } }🤖 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 `@src/components/data/column.service.ts` around lines 285 - 316, The loop currently advances maxCol and lastRow even when no cells were written (e.g., all targets are read-only), causing a false non-null range; fix by tracking whether any write occurred for each row (e.g., a local boolean like wroteInRow and an overall wroteAny) inside the inner loop that checks isReadOnly and only update maxCol and lastRow when wroteInRow is true (and compute/getRange only when wroteAny is true), ensuring the function returns null when every target cell was skipped; reference variables/methods: isReadOnly, changed, maxCol, lastRow, getRange, startCol, startRow.src/components/overlay/revogr-overlay-selection.tsx (1)
848-865:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop before applying a
nullpaste range.
getTransformedDataToApplynow explicitly returnsrange: nullwhen there is no applicable target, but Line 864 still forwards that result intoonRangeApply. Short-circuit on!rangehere so the overlay honors the new service contract instead of pushing a no-op/invalid target downstream.Suggested fix
const { defaultPrevented: canPaste } = this.rangeClipboardPaste.emit({ data: changed, models: collectModelsOfRange(changed, this.dataStore), range, ...this.types, }); - if (canPaste) { + if (canPaste || !range) { return; } this.autoFillService?.onRangeApply(changed, range, range);🤖 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 `@src/components/overlay/revogr-overlay-selection.tsx` around lines 848 - 865, After calling getClipboardPasteTargetRange and columnService.getTransformedDataToApply (producing changed and range), short-circuit if range is falsy (range === null) and return early rather than continuing; do this before emitting rangeClipboardPaste.emit and before calling autoFillService.onRangeApply so we do not forward a null/invalid range downstream (refer to getClipboardPasteTargetRange, columnService.getTransformedDataToApply, rangeClipboardPaste.emit, autoFillService.onRangeApply and the changed/range variables).
🤖 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.
Outside diff comments:
In `@src/components/data/column.service.ts`:
- Around line 285-316: The loop currently advances maxCol and lastRow even when
no cells were written (e.g., all targets are read-only), causing a false
non-null range; fix by tracking whether any write occurred for each row (e.g., a
local boolean like wroteInRow and an overall wroteAny) inside the inner loop
that checks isReadOnly and only update maxCol and lastRow when wroteInRow is
true (and compute/getRange only when wroteAny is true), ensuring the function
returns null when every target cell was skipped; reference variables/methods:
isReadOnly, changed, maxCol, lastRow, getRange, startCol, startRow.
In `@src/components/overlay/revogr-overlay-selection.tsx`:
- Around line 848-865: After calling getClipboardPasteTargetRange and
columnService.getTransformedDataToApply (producing changed and range),
short-circuit if range is falsy (range === null) and return early rather than
continuing; do this before emitting rangeClipboardPaste.emit and before calling
autoFillService.onRangeApply so we do not forward a null/invalid range
downstream (refer to getClipboardPasteTargetRange,
columnService.getTransformedDataToApply, rangeClipboardPaste.emit,
autoFillService.onRangeApply and the changed/range variables).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3d7c3ddb-d63f-45e6-8f31-e2f7f5d5326e
📒 Files selected for processing (9)
docse2e/helpers/grid.tse2e/helpers/types.tse2e/range.spec.tssrc/components.d.tssrc/components/data/column.service.tssrc/components/overlay/revogr-overlay-selection.tsxsrc/components/revoGrid/revo-grid.tsxsrc/types/interfaces.ts
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7490850826
ℹ️ 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".
|
@codex review |
|
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Summary by CodeRabbit
New Features
Behavior Change
Refactor
Tests
Chores