fix: 表格第一个回车编辑器无法正常展示 #517
Conversation
|
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/table-module/src/module/plugin.ts (1)
176-176: Update documentation for line break behaviorPlease update the documentation to reflect the new line break behavior in table cells. This will help users and developers understand the expected behavior.
Consider:
- Adding a comment explaining why
\nis used instead of\n\r- Updating user documentation if table cell behavior is documented
- Adding a code comment about cross-platform considerations
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/table-module/src/module/plugin.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: test-e2e
🔇 Additional comments (1)
packages/table-module/src/module/plugin.ts (1)
176-176: Consider platform-specific line endingsThe change from
\n\rto\nfixes the display issue, but we should ensure consistent line ending handling across platforms.Let's verify how line endings are handled elsewhere in the codebase:
✅ Verification successful
The change to use '\n' is correct and consistent with the codebase
The codebase already handles all line ending variants ('\n', '\r\n', '\n\r', '\r') during text processing, while consistently using '\n' for text insertions. This change aligns with that pattern and fixes the display issue without compromising cross-platform compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for line ending patterns in the codebase echo "Searching for line ending patterns..." rg -n "\\\\n\\\\r|\\\\r\\\\n|\\\\n|\\\\r" --type ts # Check if there are any platform-specific line ending configurations echo "Checking for line ending configurations..." fd -t f ".(git|editor)config$"Length of output: 8307
| @@ -173,7 +173,7 @@ function withTable<T extends IDomEditor>(editor: T): T { | |||
|
|
|||
| if (selectedNode != null) { | |||
| // 选中了 table ，则在 cell 内换行 | |||
| newEditor.insertText('\n\r') | |||
| newEditor.insertText('\n') | |||
There was a problem hiding this comment.
Ensure consistent line break handling
The insertion now uses \n, but the deletion logic in deleteCellBreak and isHalfBreak still expects \n\r. This inconsistency could lead to issues.
Consider updating the deletion logic to match the new line break format:
function isHalfBreak(newEditor: IDomEditor, location: Point): boolean {
const offset = location.offset
if (offset === 0) { return false }
const node = Editor.node(newEditor, location)
if (!Text.isText(node[0])) { return false }
const text = Node.string((node[0]))
if (offset >= text.length) { return false }
- return text[offset - 1] === '\n' && text[offset] === '\r'
+ return text[offset - 1] === '\n'
}Similar updates would be needed in deleteCellBreak function.
Also applies to: 31-108
💡 Codebase verification
Add tests for line break handling in table cells
The existing tests in plugin.test.ts only verify method calls but don't cover the actual line break behavior. Please add tests to verify:
- Single line break insertion in table cells
- Multiple line break insertions
- Line break deletion
- Cross-platform compatibility (different line ending types)
Location requiring tests:
packages/table-module/__tests__/plugin.test.ts
🔗 Analysis chain
Add tests for line break handling
This change fixes a bug with line breaks, but there are no corresponding test updates. Please add tests to verify:
- Single line break insertion in table cells
- Multiple line break insertions
- Line break deletion
- Cross-platform compatibility
Let's check existing tests:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for table-related tests
echo "Searching for table-related test files..."
fd -t f "table.*test.*\.(ts|js|tsx|jsx)$"
# Search for line break tests within these files
echo "Searching for line break tests..."
rg -l "insertBreak|\\\\n|\\\\r" $(fd -t f "table.*test.*\.(ts|js|tsx|jsx)$")
Length of output: 636
Script:
#!/bin/bash
# Search for cell-related test files
echo "Searching for cell-related test files..."
fd -t f "(cell|edit|content).*test.*\.(ts|js|tsx|jsx)$" packages/table-module/
# Search with broader patterns
echo "Searching for line break related tests with broader patterns..."
rg -l "newEditor\.insert|lineBreak|\\n|\\r|enter|newline" packages/table-module/__tests__/
Length of output: 591
Script:
#!/bin/bash
# Search for editor-related tests with proper escaping
echo "Searching test content..."
rg -U "newEditor|insertText|'\\\\n'|\"\\\\n\"|lineBreak|newline" packages/table-module/__tests__/
Length of output: 1025
| @@ -173,7 +173,7 @@ function withTable<T extends IDomEditor>(editor: T): T { | |||
|
|
|||
| if (selectedNode != null) { | |||
| // 选中了 table ，则在 cell 内换行 | |||
| newEditor.insertText('\n\r') | |||
There was a problem hiding this comment.
不知道之前是出于什么原因 \n 后还加了个 \r，没去看这个有什么用了，删掉 \r 看起来一切正常
|
对其他内容有影响，先 Close 了 |
Changes Overview
Implementation Approach
Testing Done
Verification Steps
Additional Notes
Checklist
Related Issues
Close #516
Summary by CodeRabbit