-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/5 rich text editor 도입 #34
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
base: develop
Are you sure you want to change the base?
The head ref may contain hidden characters: "feature/5-Rich-Text-Editor-\uB3C4\uC785"
Conversation
hwookim
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.
WYSIWYG 라이브러리를 넣으며 한층 멋져졌네요!
고생하셨습니다 👍🏼 👍🏼
간단한 리뷰 몇가지 남겨두었으니 한 번 확인해주세요~
|
|
||
| interface ButtonProps { | ||
| variant?: 'primary' | 'outline'; | ||
| variant?: 'primary' | 'outline' | 'tag'; |
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.
항상 고민되는 문제인 거 같아요.
기존의 button에 variant를 늘려 스타일을 확장할지, 아니면 기존의 버튼을 wrapping한 새로운 컴포넌트를 만들지...
태동님의 기준은 어떤거려나요?
저는 해당 버튼이 확장성이 필요하다면 기존 버튼에 스타일을 추가하는 편이고,
해당 버튼이 더 이상 확장할 필요가 없다면 새로운 컴포넌트를 만듭니다.
그 외에도 기존 버튼 컴포넌트가 너무 복잡해졌다거나, 이미 너무 많다거나... 뭐 다양한 이유가 있죠.
각자의 기준이 있으면 되는 문제 같기는 합니다 ㅋㅋ
| export const fetchUpdateReview = async ( | ||
| title: string, | ||
| tags: string[], | ||
| content: string | ||
| ) => { |
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.
body: { title: string, tags: string[], content: string }의 타입으로 인자를 받아온다면 httpClient에 body로 그대로 넘길 수 있을지도 모르겠네요.
일반적으로 api같은 함수에서는 단일 인자가 아닌이상 request에 대한 타입을 별도로 명시해 주는 편이 좋기는 합니다.
앞서 말한 것 처럼 코드가 조금 더 간결해지는 건 덤이지요 😄
| const modules = { | ||
| toolbar: [ | ||
| [{ header: [1, 2, false] }], | ||
| ['bold', 'italic', 'underline', 'strike'], | ||
| [{ list: 'ordered' }, { list: 'bullet' }], | ||
| ['code-block'], | ||
| ], | ||
| }; | ||
|
|
||
| const formats = [ | ||
| 'header', | ||
| 'bold', | ||
| 'italic', | ||
| 'underline', | ||
| 'strike', | ||
| 'list', | ||
| 'bullet', | ||
| 'code-block', | ||
| ]; |
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.
modules와 formats는 현재 컴포넌트가 리렌더링 될 때마다 다시 선언/정의되겠네요.
물론 큰 영향은 없겠지만, 어쨋거나 메모리에 일을 시키게 됩니다.
동적으로 생성되어야하는 내용이 아니라면, 컴포넌트 외부에서 선언하는 것도 하나의 방법이겠어요 👍🏼
| @@ -0,0 +1,10 @@ | |||
| export const TagItems = [ | |||
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.
태그가 추가되기도 하지 않나요?
api 명세서에 태그 조회가 없긴 했습니다만, 어떻게 관리해야할지도 한 번 생각해볼 문제겠네요 👀
만일 상수로 관리한다면, 일반적으로 상수는 TAG_ITEMS와 같이 대문자로 표기하는 게 관례기는 합니당
#️⃣연관된 이슈
📝작업 내용
스크린샷 (선택)
💬리뷰 요구사항(선택)