-
Notifications
You must be signed in to change notification settings - Fork 2
[SRLT-118] 에디터 이슈 수정 #95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "SRLT-118-\uB9AC\uD329\uD1A0\uB9C1"
Conversation
개요편집기 UI 컴포넌트의 이미지 높이 제약을 제거하고, 목록 항목 내 빈 단락을 필터링하는 새로운 확장을 추가하며, 목록 렌더링 로직과 이미지 스타일을 업데이트합니다. 변경사항
예상 검토 난이도🎯 3 (중간) | ⏱️ ~28분 관련 PR
검토자 제안
🐰 축하 시
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/lib/business/editor/extensions.ts:
- Around line 1052-1067: The deletion logic incorrectly uses a fixed length and
global positions: when iterating newState.doc.descendants for listItem nodes,
collect empty paragraph offsets relative to the listItem (using node.forEach and
the child offset), then delete each empty paragraph by calling tr.delete(pos +
offset, pos + offset + child.nodeSize) (i.e., use the child.nodeSize, not +1)
and iterate the offsets in descending order so earlier deletions don’t
invalidate later positions; update the code paths around
newState.doc.descendants, node.forEach, emptyParagraphs, and tr.delete
accordingly.
🧹 Nitpick comments (1)
src/lib/business/converter/editorToHtml.ts (1)
102-105: 불필요한.filter(Boolean)호출Line 103에서
<li>${itemContent}</li>는itemContent가 비어있어도 항상 truthy한 문자열을 반환합니다. 따라서 Line 105의.filter(Boolean)은 아무 효과가 없습니다. 주석에 "리스트 아이템은 내용이 없어도 유지"라고 명시되어 있으므로 filter를 제거하거나, 의도가 다르다면 로직을 수정하세요.♻️ 불필요한 filter 제거
// 리스트 아이템은 내용이 없어도 유지 (빈 불릿 점 표시) return `<li>${itemContent}</li>`; - }) - .filter(Boolean); + });동일한 패턴이 Line 128-130의
orderedList에도 적용됩니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/business/components/editor/GeneralSection.tsxsrc/app/business/components/editor/OverviewSection.tsxsrc/lib/business/converter/editorToHtml.tssrc/lib/business/editor/editorConstants.tssrc/lib/business/editor/extensions.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/business/editor/editorConstants.ts (1)
src/lib/business/editor/extensions.ts (1)
CleanEmptyParagraphsInListItems(1039-1077)
🔇 Additional comments (5)
src/app/business/components/editor/GeneralSection.tsx (1)
27-27: LGTM!이미지 최대 높이 제한(
max-h-[400px])을 제거하여 PDF 이미지 비율 이슈를 해결했습니다.object-contain으로 비율을 유지하면서max-w-full로 너비 제한은 유지되어 적절합니다.src/lib/business/editor/editorConstants.ts (1)
18-19: LGTM!
CleanEmptyParagraphsInListItems확장을 올바르게 import하고COMMON_EXTENSIONS에 추가했습니다. 리스트 아이템 내 빈 paragraph 정리 기능이 모든 에디터에서 활성화됩니다.Also applies to: 40-40
src/app/business/components/editor/OverviewSection.tsx (1)
91-91: LGTM!
GeneralSection.tsx와 동일하게 세 개의 에디터(features, skills, goals)에서 이미지 최대 높이 제한을 제거하여 일관성 있게 적용되었습니다.Also applies to: 118-118, 144-144
src/lib/business/editor/extensions.ts (1)
1001-1036: LGTM!
EnsureTrailingParagraph가 이제 표나 이미지 다음에만 빈 paragraph를 추가하도록 개선되었습니다. 불필요한 빈 줄 생성을 방지하는 좋은 변경입니다.src/lib/business/converter/editorToHtml.ts (1)
240-248: LGTM!모든 이미지 렌더링 케이스에
object-fit: contain을 일관되게 적용하여 이미지 비율이 유지됩니다. PDF 출력 시 이미지가 왜곡되지 않도록 하는 좋은 개선입니다.
| newState.doc.descendants((node, pos) => { | ||
| if (node.type.name === 'listItem') { | ||
| const emptyParagraphs: number[] = []; | ||
|
|
||
| node.forEach((child, offset) => { | ||
| if (child.type.name === 'paragraph' && child.content.size === 0) { | ||
| emptyParagraphs.push(pos + offset + 1); | ||
| } | ||
| }); | ||
|
|
||
| // 빈 paragraph가 여러 개 있으면 하나만 남기고 나머지 제거 | ||
| if (emptyParagraphs.length > 1) { | ||
| for (let i = emptyParagraphs.length - 1; i > 0; i--) { | ||
| tr.delete(emptyParagraphs[i], emptyParagraphs[i] + 1); | ||
| modified = true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
빈 paragraph 삭제 시 position 계산 오류 가능성
두 가지 잠재적 문제가 있습니다:
-
삭제 범위 오류: 빈 paragraph의
nodeSize는 2 (시작 토큰 + 종료 토큰)이지만,tr.delete(pos, pos + 1)로 1만 삭제하고 있습니다. -
여러 listItem 처리 시 position 무효화:
descendants()가 전체 문서를 순회하면서 첫 번째 listItem에서 삭제가 발생하면, 이후 listItem들의 position이 shift되지만 이미 수집된 position은 원본 문서 기준입니다.
🔧 제안된 수정
export const CleanEmptyParagraphsInListItems = Extension.create({
name: 'clean-empty-paragraphs-in-list-items',
addProseMirrorPlugins() {
return [
new Plugin({
appendTransaction: (transactions, oldState, newState) => {
if (!transactions.some((tr) => tr.docChanged)) {
return null;
}
const tr = newState.tr;
let modified = false;
+
+ // 삭제할 범위를 {from, to} 형태로 수집
+ const toDelete: Array<{from: number, to: number}> = [];
newState.doc.descendants((node, pos) => {
if (node.type.name === 'listItem') {
- const emptyParagraphs: number[] = [];
+ const emptyParagraphs: Array<{from: number, size: number}> = [];
node.forEach((child, offset) => {
if (child.type.name === 'paragraph' && child.content.size === 0) {
- emptyParagraphs.push(pos + offset + 1);
+ emptyParagraphs.push({
+ from: pos + offset + 1,
+ size: child.nodeSize
+ });
}
});
- // 빈 paragraph가 여러 개 있으면 하나만 남기고 나머지 제거
if (emptyParagraphs.length > 1) {
- for (let i = emptyParagraphs.length - 1; i > 0; i--) {
- tr.delete(emptyParagraphs[i], emptyParagraphs[i] + 1);
- modified = true;
+ for (let i = 1; i < emptyParagraphs.length; i++) {
+ toDelete.push({
+ from: emptyParagraphs[i].from,
+ to: emptyParagraphs[i].from + emptyParagraphs[i].size
+ });
}
}
}
});
+
+ // 역순으로 정렬하여 뒤에서부터 삭제
+ toDelete.sort((a, b) => b.from - a.from);
+ for (const range of toDelete) {
+ tr.delete(range.from, range.to);
+ modified = true;
+ }
return modified ? tr : null;
},
}),
];
},
});🤖 Prompt for AI Agents
In @src/lib/business/editor/extensions.ts around lines 1052 - 1067, The deletion
logic incorrectly uses a fixed length and global positions: when iterating
newState.doc.descendants for listItem nodes, collect empty paragraph offsets
relative to the listItem (using node.forEach and the child offset), then delete
each empty paragraph by calling tr.delete(pos + offset, pos + offset +
child.nodeSize) (i.e., use the child.nodeSize, not +1) and iterate the offsets
in descending order so earlier deletions don’t invalidate later positions;
update the code paths around newState.doc.descendants, node.forEach,
emptyParagraphs, and tr.delete accordingly.
Chasyuss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다! 수고하셨습니당
#️⃣ 관련 이슈
✍️ 작업 내용 요약
💡 작업 내용
기능
🖥️ 구현 화면
기타 사항
Summary by CodeRabbit
릴리스 노트
버그 수정
개선
✏️ Tip: You can customize this high-level summary in your review settings.