Skip to content
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

Feature/#104 마일스톤 실적 일괄등록 페이지 구현 #126

Merged

Conversation

amaran-th
Copy link
Contributor

@amaran-th amaran-th commented Aug 9, 2024

@amaran-th amaran-th added ✨ 기능 개발 새로운 기능을 구현하는 데 필요한 작업 또는 변경 사항 🌕 프론트엔드 프론트엔드 개발 labels Aug 9, 2024
@amaran-th amaran-th requested a review from llddang August 9, 2024 01:30
@amaran-th amaran-th self-assigned this Aug 9, 2024
Copy link
Contributor

@llddang llddang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인부탁드립니다.

registerHistories(values.file, {
onSuccess: () => {
window.alert('실적 등록에 성공하였습니다.');
router.push('/admin/milestone/list');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

실적 등록 성공 시 마일스톤 목록으로 이동하여 확인하는 것은 좋습니다.

보통 선생님들이 일괄 등록할 때 다른 활동들에 대해 여러 파일을 가질 것 이라고 생각됩니다.
그러할 경우에 자동으로 이동되면 다시 돌아와야 함으로 귀찮을 것 같습니다.

파일 한 개만을 등록할 경우에는 있으면 좋은 기능이지만 없어도 되는 기능입니다.
하지만 파일 여러개를 등록해야할 경우에는 있으면 불편한 기능이라고 생각되어 없는 것이 좋을 것 같습니다.

onSubmit={(values, { setSubmitting }) => {
registerHistories(values.file, {
onSuccess: () => {
window.alert('실적 등록에 성공하였습니다.');
Copy link
Contributor

@llddang llddang Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window.alert의 경우 자동으로 사라지지 않고 사용자에게 클릭을 하여 모달을 사라지도록 요구합니다.

이보다는 몇 초뒤면 자동으로 사라지는 React-Toastify를 사용하는 건 어떨까요?

Comment on lines +51 to +56
const sampleFileUrl = useMemo(() => {
if (sampleFile) {
return URL.createObjectURL(sampleFile);
}
return '';
}, [sampleFile]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

궁금한 것이 있는데요 어떨 경우에 useMemo를 사용하는 걸까요?

여기에서 재생성이 될 경우는 standardFile 과 sampleFile이 변경될 때 뿐인것 같습니다.
standardFile와 sampleFile의 결과로는 undefined 혹은 그에 해당하는 값일 것 같은데요.

여러 번 재생성을 요구하지 않으므로 값을 저장하지는 않는 useEffect를 저는 사용할 것 같습니다.

세연님이 useMemouseCallback을 많이 사용함으로 어떤 기준에서 사용하는지 궁금합니다!

useMemo가 아닌 useEffect로 변경하는 말은 절대 아닙니다!!
그냥 저도 useMemo와 useCallback을 언제 사용할지 고민을 많이 하고 있어서 이에 대해 논의하고 싶어서 묻는겁니당!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 서버 데이터를 가공할 필요가 있을 때 주로 useMemo를 사용해서 서버 데이터가 변경될 때마다 가공한 값도 변경사항이 적용되도록 해주고 있습니다.
useCallback은 state, props에 접근하는 함수를 구현할 때 사용합니다!

개인적으로 useEffect보다 useMemo, useCallback이 값, 함수를 관리하는 데 특화되어 있다고 생각해서 이렇게 사용하고 있습니다.

+성능적으로는 useEffect는 렌더링 후에 실행되는 반면 useMemo는 렌더링 중에 실행되기 때문에 좀 더 빠르다는 특징이 있다고 합니다.

@amaran-th amaran-th merged commit 55347d8 into main Aug 12, 2024
1 check passed
@amaran-th amaran-th deleted the Feature/#104-마일스톤_실적_일괄등록_페이지_구현 branch August 12, 2024 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ 기능 개발 새로운 기능을 구현하는 데 필요한 작업 또는 변경 사항 🌕 프론트엔드 프론트엔드 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FRONT] 마일스톤 실적 일괄등록 페이지 구현
2 participants