-
Notifications
You must be signed in to change notification settings - Fork 37
[최성락] Sprint7 #260
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: "React-\uCD5C\uC131\uB77D-sprint7"
[최성락] Sprint7 #260
Changes from all commits
de581c2
fd68cfc
d883dfd
40fecea
ae2ea4d
bdf381d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| import { fetchProducts } from "../../api/product"; | ||
| import { useCallback, useEffect, useState } from "react"; | ||
| import { Link } from "react-router-dom"; | ||
| import { HttpException } from "../../utils/exceptions"; | ||
| import ItemCard from "../ui/Item/ItemCard"; | ||
| import "./AllProductsSection.css"; | ||
|
|
||
| const getPageSize = () => { | ||
| const width = window.innerWidth; | ||
| const getPageLimit = (width) => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네이밍 변경 굳 👍🏻 |
||
| if (width > 1199) { | ||
| return 10; | ||
| } else if (width > 768) { | ||
|
|
@@ -17,15 +17,24 @@ const getPageSize = () => { | |
|
|
||
| function AllProductsSection({ sortOption }) { | ||
| const [items, setItems] = useState([]); | ||
| const [pageSize, setPageSize] = useState(getPageSize()); | ||
| const [pageSize, setPageSize] = useState(getPageLimit(window.innerWidth)); | ||
| const [error, setError] = useState(null); | ||
|
|
||
| const getProducts = async () => { | ||
| const { list } = await fetchProducts(sortOption, pageSize); | ||
| setItems(list); | ||
| const getProducts = async (limit, sort) => { | ||
| try { | ||
| const { list } = await fetchProducts(sort, limit); | ||
|
Comment on lines
+23
to
+25
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 함수들처렴 인자가 여러개 들어가기 시작한다면, 인자를 객체로 전달받는 형태도 고려해보면 좋습니다 |
||
| setItems(list); | ||
| } catch (error) { | ||
| if (error instanceof HttpException) { | ||
| setError(error.message); | ||
| } else { | ||
| setError("알 수 없는 오류가 발생했습니다."); | ||
| } | ||
|
Comment on lines
+28
to
+32
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사실 이미 fetchproduct에서 던져지는 에러 문구 핸들링은 다 하고 있잖아요 |
||
| } | ||
| }; | ||
|
|
||
| const handleResize = useCallback(() => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사실 콜백으로 감싸서 재평가 자체는 안되도록 할 수 있지만 실질적으로 onResizeEvent가 와르르 발생할때에 대한 퍼포먼스 핸들링은 어려울거에요. 따라서 쓰로틀링이나 디바운싱을 활용해보면 좋겠는데요 // utils.js
function debounce(callback, delay = 300) {
let timeoutId = null;
return (...args) => {
if (timeoutId) {
clearTimeout(timeoutId);
}
timeoutId = setTimeout(() => {
callback(...args);
}, delay);
};
}로 구현해두고 useEffect(() => {
const handleResize = debounce(() => {
const newPageSize = getPageLimit(window.innerWidth);
if (newPageSize !== pageSize) {
setPageSize(newPageSize);
}
}, 300); // 300ms 지연 시간 설정
window.addEventListener('resize', handleResize);
return () => {
window.removeEventListener('resize', handleResize);
};
}, [pageSize]);이런식으로 활용해보면 어떨까 합니다 |
||
| const newPageSize = getPageSize(); | ||
| const newPageSize = getPageLimit(window.innerWidth); | ||
| if (newPageSize !== pageSize) { | ||
| setPageSize(newPageSize); | ||
| } | ||
|
|
@@ -37,9 +46,15 @@ function AllProductsSection({ sortOption }) { | |
| }, [handleResize]); | ||
|
|
||
| useEffect(() => { | ||
| getProducts(); | ||
| if (pageSize !== null) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. page size null check가 필요한가요? |
||
| getProducts(pageSize, sortOption); | ||
| } | ||
| }, [sortOption, pageSize]); | ||
|
|
||
| if (error) { | ||
| return <div>오류: {error}</div>; | ||
| } | ||
|
|
||
| return ( | ||
| <section className="all-products"> | ||
| <div className="all-products-list"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| .comments-list { | ||
| display: flex; | ||
| flex-direction: column; | ||
| } | ||
|
|
||
| .comment-wrapper { | ||
| display: flex; | ||
| flex-direction: column; | ||
| padding-bottom: 12px; | ||
| border-bottom: 1px solid #e5e7eb; | ||
| } | ||
|
|
||
| .comment-author { | ||
| display: flex; | ||
| } | ||
|
|
||
| .comment-options { | ||
| width: 16px; | ||
| height: 16px; | ||
| cursor: pointer; | ||
| margin-left: auto; | ||
| } | ||
|
|
||
| .comment-content { | ||
| font-size: 14px; | ||
| color: #1f2937; | ||
| padding-top: 14px; | ||
| } | ||
|
|
||
| .comment-profile-image { | ||
| padding-right: 15px; | ||
| } | ||
|
|
||
| .comment-author-detail { | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 4px; | ||
| } | ||
|
|
||
| .comment-nickname { | ||
| font-size: 12px; | ||
| color: #4b5563; | ||
| line-height: 18px; | ||
| } | ||
|
|
||
| .comment-date { | ||
| font-size: 12px; | ||
| line-height: 18px; | ||
| color: #9ca3af; | ||
| } | ||
|
|
||
| .comment-empty { | ||
| margin: 0 auto; | ||
| text-align: center; | ||
| } | ||
|
|
||
| .comment-empty-text { | ||
| color: #9ca3af; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { formatRelativeTime } from "../../../utils/formatRelativeTime"; | ||
| import "./CommentList.css"; | ||
| import profile from "../../../assets/images/profile.svg"; | ||
| import ic_kebab from "../../../assets/icons/ic_kebab.svg"; | ||
| import inquiry_empty from "../../../assets/images/Img_inquiry_empty.svg"; | ||
|
|
||
| function CommentList({ comments }) { | ||
| const getRelativeTime = (createdAt, updatedAt) => { | ||
| if (updatedAt && updatedAt !== createdAt) { | ||
| return formatRelativeTime(updatedAt); | ||
| } | ||
| return formatRelativeTime(createdAt); | ||
| }; | ||
|
Comment on lines
+8
to
+13
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 하나 의견있는데요, 지금 이 로직의 경우
근데 결국 위 두 로직 다 결국 수정일을 기준으로 날짜 파싱해도 동일한 결과가 나오는 것으로 생각됩니다. 애초에 디비 설계상 데이터가 최초 생성되면 수정일도 해당일과 동일하게 생성이 될거고, 만약 코드잇측에서 디비 설계를 좀 요상하게 해서 최초 생성시 수정시간이 등록되지 않는다면 지금처럼 두어도 무난하지만 그게 아니라면 updatedAt 기준만 활용해서 로직 파싱해도 될것 같습니다
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그리고 아마 이 코드가 성락님이 여쭤보신 ui가 아닌곳에서 필요한 값을 꺼내오는 부분이라고 생각되어요. 이건 사실 개인 취향에 따라 달려있는기능이긴 한데요, 날짜 데이터의 경우, 되도록이면 UI 콤포넌트에게 날짜형데이터는 날짜형태로 전달을 해주고, 환경에 따라서 UI로 파싱하는건 UI 콤포넌트에서 util function을 활용해 풀어주는걸 선호하고 있어요! 왜냐면 나중에 콤포넌트 관리할때, 날짜를 보여주는 부분을 수정하고자 한다면 당연히 UI 콤포넌트로 먼저 이동하게 될텐데, 거기에서 이미 다 형처리가 완료된 데이터를 prop으로 전달받는다면 UI 관리 책임이 조금 어긋난다고 생각하기 때문이에요. 이거는 한번 성락님 취향에 맞춰 탐색해보시면 어떨까요? |
||
|
|
||
| return ( | ||
| <div className="comments-list"> | ||
| {comments?.length > 0 ? ( | ||
| comments.map((comment) => ( | ||
| <div key={comment.id} className="comment-wrapper"> | ||
| <p className="comment-content">{comment.content}</p> | ||
| <img src={ic_kebab} alt="수정 삭제 버튼" className="comment-options" /> | ||
| <div className="comment-author"> | ||
| <img src={profile} alt={`${comment.writer.nickname} 프로필 이미지`} className="comment-profile-image" /> | ||
| <div className="comment-author-detail"> | ||
| <p className="comment-nickname">{comment.writer.nickname}</p> | ||
| <p className="comment-date">{getRelativeTime(comment.createdAt, comment.updatedAt)}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )) | ||
| ) : ( | ||
| <div className="comment-empty"> | ||
| <img src={inquiry_empty} alt="댓글 없음" className="comment-empty-image" /> | ||
| <p className="comment-empty-text">아직 문의가 없어요</p> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export default CommentList; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| .inquiry-form { | ||
| width: 100%; | ||
| max-width: 1200px; | ||
| margin: 0 auto; | ||
| position: relative; | ||
| padding: 30px 0; | ||
| } | ||
|
|
||
| .custom-inquiry-input { | ||
| height: 104px; | ||
| } | ||
|
|
||
| .inquiry-button { | ||
| padding: 12px 23px; | ||
| border-radius: 8px; | ||
| border: none; | ||
| background-color: #9ca3af; | ||
| color: #f3f4f6; | ||
| transition: background-color 0.3s ease; | ||
| } | ||
|
|
||
| .inquiry-button.active { | ||
| background-color: #3692ff; | ||
| } | ||
|
|
||
| .inquiry-button-wrapper { | ||
| display: flex; | ||
| justify-content: flex-end; | ||
| } |
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.
너무 좋습니다. 이렇게 서버단에서 오류던져준건 그에 맞춰서 전파해주고,
네트워크 또는 알수없는 오류의 경우 다르게 처리해서 던져줌으로 훨씬 더 ui쪽에서 디버깅하기가 수월해질거에요