-
Notifications
You must be signed in to change notification settings - Fork 2
refactor: #137/api 함수 에러 처리 & data fetching 시 로딩, 에러 처리 #138
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -17,19 +17,28 @@ export default function InvitationSection({ | |
| token: string; | ||
| }) { | ||
| const [items, setItems] = useState<Invitation[]>([]); | ||
| const [loading, setLoading] = useState(false); | ||
| const [page, setPage] = useState(1); | ||
| const [totalCount, setTotalCount] = useState(0); | ||
| const totalPage = Math.ceil(totalCount / PAGE_SIZE); | ||
|
|
||
| const handleLoad = async () => { | ||
| const { invitations, totalCount } = await fetchInvitationList({ | ||
| token, | ||
| id, | ||
| page, | ||
| size: PAGE_SIZE, | ||
| }); | ||
| setItems(invitations); | ||
| setTotalCount(totalCount); | ||
| setLoading(true); | ||
|
|
||
| try { | ||
| const { invitations, totalCount } = await fetchInvitationList({ | ||
| token, | ||
| id, | ||
| page, | ||
| size: PAGE_SIZE, | ||
| }); | ||
| setItems(invitations); | ||
| setTotalCount(totalCount); | ||
| } catch (error) { | ||
| console.error("Failed to load invitation:", error); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
|
|
@@ -48,6 +57,8 @@ export default function InvitationSection({ | |
| } | ||
| }; | ||
|
|
||
| if (loading) return <p>Loading...</p>; | ||
|
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가 자연스럽게 표시되도록 하는 방법도 좋겠어요!
Owner
Author
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. 맞아요 우선은 loading 상태 사용을 위해서 임시로 이렇게 해놨는데, 금요일 리팩토링 때 아름님이 말씀하신 것처럼 스켈레톤을 사용하거나 하는 식으로 UI를 바꿔볼 생각입니다! |
||
|
|
||
| return ( | ||
| <div className="flex flex-col gap-[26px] w-full p-4 rounded-lg bg-white tablet:gap-[17px] tablet:p-6"> | ||
| <div className="flex justify-between items-center tablet:items-start"> | ||
|
|
||
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.
저도 이 부분 고민중인 부분이에요..! setLoading(true)와 setLoading(false)가 여러 곳에서 반복적으로 사용되니까 custom hook으로 분리해서 사용하는 방법을 고민하고 있었어요..!
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.
확실히 반복되는 코드가 많더라구요... 커스텀 훅이 있어도 좋겠네요!