grade upload/#9 #41

yougyung · 2024-03-03T04:27:52Z

📌 작업 내용

구현 내용 및 작업 했던 내역

use file hook을 통한 pdf 확장자 유효성 검사
file upload component 개발 및 테스트 추가
file-upload page view 작업

🤔 고민 했던 부분

성적 업로드 기능을 담당하지만 lecture 도메인에 소속된다는 점에서 네이밍에 대한 고민을 진행했는데,
1. taken-lecture.command.ts : taken lecture에 변경을 가하고, 또 다시 유저 lecture정보를 변경시키는 경우로 lecture custom 기능이 있는데 두 가지 케이스에 공통으로 적용될수있는 네이밍
2. ui/lecture/upload-taken-lecture : 기존 lecture 도메인에서 taken lecture라는 네이밍을 갖고있으므로 해당 일관성을 확보하며, pdf=taken lecture list이므로 pdf upload=upload taken lecture으로 명확한 의미 전달

해당 이유로 위와 같이 작성했습니다:)

🔊 전달하고 싶은 부분

기존 file-upload페이지에서 grade-upload로 페이지 명 변경
본래 file-upload페이지에서 사용되는 button이 저희 서비스에서 사용되는 가장 큰 규격(lg)의 버튼으로 알고있습니다. 적용해보니 다소 크기가 큰 것 같아서 md사이즈로 변경했으며 lg사이즈에 대한 재조정이 필요할 것으로 예상됩니다. 괜찮으시다면 제가 lg 버튼 size에 한하여 임의로 조정한 이후 피드백을 다시 받는 방식도 좋을 것 같습니다.
upload-grade-card 컴포넌트의 경우 서버 컴포넌트를 의도하고 작성했지만, form을 구성하는 form-number-input.tsx등의 컴포넌트에서 client component를 요구하므로 'use client'; 추가하여 클라이언트 컴포넌트로 작업했습니다.

./app/ui/view/molecule/form/form-number-input.tsx
Error: x You're importing a component that needs useFormStatus. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default.

github-actions · 2024-03-03T04:29:33Z

🚀storybook: https://65ccb85d5afe55a024495bc0-vhyfqfhnyr.chromatic.com/

github-actions · 2024-03-03T04:32:56Z

🚀storybook: https://65ccb85d5afe55a024495bc0-qjpweoztdr.chromatic.com/

github-actions · 2024-03-12T13:52:41Z

🚀storybook: https://65ccb85d5afe55a024495bc0-kffentrvve.chromatic.com/

github-actions · 2024-03-12T15:22:38Z

🚀storybook: https://65ccb85d5afe55a024495bc0-lhcmglljvc.chromatic.com/

seonghunYang

수고하셨습니다

seonghunYang · 2024-03-13T05:49:50Z

app/ui/view/molecule/upload-pdf/upload-pdf.tsx

+import { ChangeEvent, DragEvent } from 'react';
+
+function UploadPdf() {
+  const { file, changeFile } = useFile();


[comment]

이 hook이 존재하는 이유는 input type=file이 uncontrolled component 만 가능하기 때문인가요?

uncontrolled component가 의미하는 것이 무엇인지 조금 더 설명해주실 수 있을까요? 해당 hook이 존재하는 이유는 pdf 파일에 대한 관심사를 분리하고 캡슐화를 통한 관리를 위함입니다. 당시에는 저희 서비스에서 사용하는 file이 pdf파일밖에 존재하지않으므로 naming을 useFile로 가져갔는데, 명확함을 위해 usePdfFile로 수정하겠습니다.

React에서 input 태그 같은 것을 조작할 때, control 방식과 uncontrolled 방식이 있습니다. 제가 알기론, type=file인 경우에는 uncontrolled 컴포넌트만 가능하다고 알고 있습니다.

둘의 차이를 간단하게 설명하자면, control은 value를 직접 state로 관리하여 input에 넣는 방식이고, uncontrolled는 defaultValue를 지정한 후에는 기본 input처럼 동작하는 방식입니다.

질문한 이유는 type=file 태그는 uncontrolled 방식이지만, 파일의 유무에 따라 렌더링을 control해야 하기 때문에 이 hook을 만든 것인지 의도가 궁금해서 질문한 것입니다.

말씀해주신 관심사의 분리와 캡슐화에는 동의합니다.

usePdfFile로 변경하는 것도 괜찮아 보입니다.

https://legacy.reactjs.org/docs/uncontrolled-components.html

파일의 유무에 따라 렌더링을 control하려는 목적으로 useState를 사용한 것은 맞습니다. (제어 컴포넌트를 의도) 하지만, hook으로 분리한 목적은 앞서 말씀드린 관심사의 분리와 캡슐화 목적이 더 크게 작용했습니다.

아이러니하게도 의도는 일치했지만 Controlled Component, Uncontrolled Component의 개념적인 부분을 알고 개발에 적용했던 부분은 아니었는데 덕분에 공부해볼 수 있었습니다. 감사합니다 :)

seonghunYang · 2024-03-13T05:52:23Z

app/ui/view/molecule/upload-pdf/upload-pdf.tsx

+  const handleClickInputBox = (event: ChangeEvent) => {
+    const { files } = event.target as HTMLInputElement;
+    if (files) changeFile(files[0]);
+  };


[request change]

클릭 이벤트가 아닌 onChange 이벤트 핸들러로 명칭을 변경해야 할 것 같습니다.

seonghunYang · 2024-03-13T05:54:11Z

app/ui/view/molecule/upload-pdf/upload-pdf.tsx

+  const handleDrop = (event: DragEvent<HTMLDivElement>) => {
+    event.preventDefault();
+    const targetFile = event.dataTransfer.files[0];
+    changeFile(targetFile);
+  };


[comment]

changeFile 함수를 실행해도 input의 value 값이 변경되지 않는 것 같습니다. 이 경우, form을 통해 전송할 때 drag&drop 방식이 동작하나요?

registUserGrade에서 formData.get('file')을 확인했을 때, 정상적으로 동작하는 것을 확인하였습니다 :)

흠 그렇다면, 제 생각에는 input의 너비가 drag & drop이 있는 div를 모두 덮고 있어서, drag & drop 시에 onChange 이벤트가 작동하여 동작하는 것 같습니다.

따라서 2가지 문제가 있는 것으로 보입니다.

첫째, div에 onDrag와 onDragOver 이벤트가 작동하지 않는 것 같습니다. 이 이벤트 핸들러를 제거하고 테스트 해보시겠어요? 기능이 정상적으로 동작한다면, 이 이벤트 핸들러는 혼동을 방지하기 위해 제거해야 할 것 같습니다.

둘째, input의 className을 보면, 파일을 드래그 & 드랍하는 파란색 사각형의 X축 범위를 넘어서 파일을 드래그해도 파일이 업로드 될 것으로 예상됩니다. 이미지를 첨부하겠습니다. 해당 위치에 파일을 업로드할 때 파일이 업로드 되는지 확인해 주세요. 그리고 이것이 문제라고 생각한다면 수정이 필요할 것 같습니다.

중요한 부분을 놓치고 있었네요 반영해서 수정했습니다. 감사합니다 !

gahyuun

고생하셨습니다!

gahyuun · 2024-03-13T13:35:06Z

app/ui/view/molecule/upload-pdf/upload-pdf.tsx

+  const handleClickInputBox = (event: ChangeEvent) => {
+    const { files } = event.target as HTMLInputElement;


ChangeEvent<HTMLInputElement>로 작성하면 단언을 안해도 될 것 같네요!

gahyuun · 2024-03-13T13:37:30Z

app/ui/lecture/upload-taken-lecture/upload-grade-card.tsx

폴더명은 upload-taken-lecture인데 컴포넌트명은 upload-grade-card로 되어있네요
lecture와 grade 가 혼재되어있어서 헷갈릴 수 있을 것 같네요!
성적 업로드라고 부르긴 하지만 사실 자신이 수강한 과목을 업로드 하는 거랑 마찬가지이니 grade를 taken-lecture 또는 lecture로 변경하는 건 어떨까요?

upload-taken-lecture가 더 명확하게 와닿을 수 있겠네요. 감사합니다 :)

github-actions · 2024-03-14T03:33:25Z

🚀storybook: https://65ccb85d5afe55a024495bc0-cdxkaebova.chromatic.com/

github-actions · 2024-03-14T03:36:38Z

🚀storybook: https://65ccb85d5afe55a024495bc0-qfnvefscsl.chromatic.com/

gahyuun · 2024-03-14T03:48:21Z

app/(sub-page)/grade-upload/page.tsx

@@ -0,0 +1,12 @@
+import ContentContainer from '../../ui/view/atom/content-container';
+import Manual from './components/manual';
+import UploadGradeCard from '../../ui/lecture/upload-taken-lecture/upload-taken-lecture';


import문은 아직 upload grade card네요!

gahyuun · 2024-03-14T03:48:38Z

app/(sub-page)/grade-upload/page.tsx

+export default function GradeUploadPage() {
+  return (
+    <ContentContainer className="flex flex-col justify-center gap-8 min-h-[70vh]">
+      <Manual />
+      <UploadGradeCard />
+    </ContentContainer>
+  );


혹시 페이지는 그대로 gradeupload로 가져가는 이유가 있을까요?!

/taken-lecture-upload보다 사용자에게 /grade-upload 가 더 직관적일 것 같다는 생각에 변경을 진행하지 않았습니다 !

넵 알겠습니다!

github-actions · 2024-03-14T04:11:01Z

🚀storybook: https://65ccb85d5afe55a024495bc0-tflcikzmau.chromatic.com/

seonghunYang

조금 확인이 늦었네요.
수정 요청사항 남겨놨어요. 확인 부탁드릴게요

seonghunYang · 2024-03-14T11:46:27Z

app/__test__/ui/view/upload-pdf.test.tsx

+  it('pdf가 아닌 파일을 업로드할 수 없다', async () => {
+    render(<UploadPdf />);
+
+    const targetFile = new File(['grade'], 'grade.pdf', {
+      type: 'text/plain',
+    });
+
+    const uploadBox = await screen.findByTestId('upload-box');
+    fireEvent.upload(uploadBox, targetFile);
+
+    expect(screen.queryByText('추가')).not.toBeInTheDocument();
+  });


[request change]

이 부분은 지금 확인해서 코멘트를 남깁니다.

테스트명만 보면 pdf가 아닌 파일이 업로드될 때 업로드가 안 되는 것처럼 보이지만, 테스트 내용을 보면 pdf 파일을 업로드했을 때를 테스트하는 것 같습니다.

두 가지 수정사항을 제안하고 싶습니다.

첫째, 테스트명을 변경해야 할 것 같습니다. 테스트명만 보아도 어떤 테스트인지 알 수 있어야 합니다. 테스트명에 대한 의견은 PR #42에 남겨두었으니 참고하시면 좋을 것 같습니다.

둘째, 추가 테스트가 필요해 보입니다. pdf 파일을 업로드했을 때와 pdf가 아닌 파일을 업로드했을 때를 테스트하는 것이 필요해 보입니다.

현재는 처리가 안 되어 있는 것 같은데, 성적표가 아닌 pdf가 업로드되었을 때 어떻게 처리되는지에 대한 테스트 코드도 나중에 추가되면 좋을 것 같네요.

github-actions · 2024-03-16T09:55:08Z

🚀storybook: https://65ccb85d5afe55a024495bc0-uvbzimoxqb.chromatic.com/

gahyuun · 2024-03-16T14:41:45Z

app/__test__/ui/view/upload-pdf.test.tsx

+    expect(screen.getByText(targetFile.name)).toBeInTheDocument();
+  });
+
+  it('파일이 업로드 될 때, pdf가 아닌 file을 업로드 하면 변화가 발생하지않는다.', async () => {
+    render(<UploadPdf />);
+
+    const targetFile = new File(['grade'], 'grade.png', {
+      type: 'text/plain',
+    });
+
+    const uploadBox = await screen.findByTestId('upload-box');
+    await userEvent.upload(uploadBox, targetFile);
+    expect(screen.queryByText('마우스로 드래그 하거나 아이콘을 눌러 추가해주세요.')).toBeInTheDocument();


getByText랑 queryByText 사용하는 기준을 여쭤봐도 될까요!?

개발과정에서 로직을 위해 메소드를 다르게 가져갔는데, 변경이 발생하면서 현재는 어떤 메소드를 사용해도 적용이 되는 상황입니다!

seonghunYang

수고 많았어요.
그런데 도메인 컴포넌트인 upload-taken-lecture 컴포넌트가 아니라 뷰 컴포넌트인 upload-pdf 컴포넌트를 테스트한 이유가 있을까요?

new issue

yougyung · 2024-03-18T01:59:46Z

수고 많았어요.

그런데 도메인 컴포넌트인 upload-taken-lecture 컴포넌트가 아니라 뷰 컴포넌트인 upload-pdf 컴포넌트를 테스트한 이유가 있을까요?

테스트를 진행하고자하는 case가 view component인 upload-pdf에 종속된다는 생각이 들어 해당 컴포넌트를 대상으로 테스트를 진행했습니다 :)

seonghunYang · 2024-03-18T10:00:36Z

수고 많았어요.

그런데 도메인 컴포넌트인 upload-taken-lecture 컴포넌트가 아니라 뷰 컴포넌트인 upload-pdf 컴포넌트를 테스트한 이유가 있을까요?

테스트를 진행하고자하는 case가 view component인 upload-pdf에 종속된다는 생각이 들어 해당 컴포넌트를 대상으로 테스트를 진행했습니다 :)

확인했어요

yougyung added 5 commits February 26, 2024 16:49

feat: implement upload-file component

2dfa861

chore: commit for merge

75c9882

feat: implement file-upload manual

c3bc1ba

feat: implement useFile hook

8c37597

feat: add file-upload stories

d0548ce

yougyung requested review from seonghunYang and gahyuun as code owners March 3, 2024 04:27

yougyung linked an issue Mar 3, 2024 that may be closed by this pull request

기이수 성적 업로드 구현 #9

Closed

1 task

yougyung self-assigned this Mar 3, 2024

chore: fix example variable

1930ff7

yougyung marked this pull request as draft March 3, 2024 14:38

yougyung added 5 commits March 7, 2024 18:42

refactor: api request in server component

7ea12c6

chore: add todo comment

4dc8853

chore: commit for merge

edad423

fix: upload grade structure

8006519

refactor: use form component

39fe4d7

feat: add fileupload test

421c5d0

yougyung added the task label Mar 12, 2024

yougyung marked this pull request as ready for review March 12, 2024 15:47

seonghunYang reviewed Mar 13, 2024

View reviewed changes

gahyuun previously approved these changes Mar 13, 2024

View reviewed changes

yougyung added 3 commits March 14, 2024 11:47

fix: remove fileinput drag&drop handler

c87c9fc

rename: change to usePdfFile, uploadTakenLecture

b38ff02

style: add option truncate

b3dac84

yougyung dismissed gahyuun’s stale review via b3dac84 March 14, 2024 03:30

chore: remove unnecessary code

a7486e6

gahyuun reviewed Mar 14, 2024

View reviewed changes

fix: change variable name

f05c75e

gahyuun previously approved these changes Mar 14, 2024

View reviewed changes

seonghunYang requested changes Mar 14, 2024

View reviewed changes

fix: change target grade-upload test

c54c917

yougyung dismissed gahyuun’s stale review via c54c917 March 16, 2024 09:53

gahyuun approved these changes Mar 16, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

seonghunYang reviewed Mar 17, 2024

View reviewed changes

seonghunYang approved these changes Mar 18, 2024

View reviewed changes

yougyung merged commit 3fb4a86 into main Mar 18, 2024
2 of 3 checks passed

yougyung deleted the grade-upload/#9 branch March 18, 2024 10:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

grade upload/#9 #41

grade upload/#9 #41

yougyung commented Mar 3, 2024 •

edited

Loading

github-actions bot commented Mar 3, 2024

github-actions bot commented Mar 3, 2024

github-actions bot commented Mar 12, 2024

github-actions bot commented Mar 12, 2024

seonghunYang left a comment

seonghunYang Mar 13, 2024 •

edited

Loading

yougyung Mar 13, 2024 •

edited

Loading

seonghunYang Mar 14, 2024

yougyung Mar 14, 2024

seonghunYang Mar 13, 2024

seonghunYang Mar 13, 2024

yougyung Mar 13, 2024

seonghunYang Mar 14, 2024 •

edited

Loading

yougyung Mar 14, 2024

gahyuun left a comment

gahyuun Mar 13, 2024 •

edited

Loading

gahyuun Mar 13, 2024

yougyung Mar 13, 2024

github-actions bot commented Mar 14, 2024

github-actions bot commented Mar 14, 2024

gahyuun Mar 14, 2024

gahyuun Mar 14, 2024

yougyung Mar 14, 2024

gahyuun Mar 14, 2024

github-actions bot commented Mar 14, 2024

seonghunYang left a comment

seonghunYang Mar 14, 2024

github-actions bot commented Mar 16, 2024

gahyuun Mar 16, 2024

yougyung Mar 18, 2024

This comment was marked as outdated.

seonghunYang left a comment

yougyung commented Mar 18, 2024

seonghunYang commented Mar 18, 2024

		const handleClickInputBox = (event: ChangeEvent) => {
		const { files } = event.target as HTMLInputElement;

grade upload/#9 #41

grade upload/#9 #41

Conversation

yougyung commented Mar 3, 2024 • edited Loading

📌 작업 내용

🤔 고민 했던 부분

🔊 전달하고 싶은 부분

github-actions bot commented Mar 3, 2024

github-actions bot commented Mar 3, 2024

github-actions bot commented Mar 12, 2024

github-actions bot commented Mar 12, 2024

seonghunYang left a comment

Choose a reason for hiding this comment

seonghunYang Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

yougyung Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seonghunYang Mar 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gahyuun left a comment

Choose a reason for hiding this comment

gahyuun Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 14, 2024

github-actions bot commented Mar 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 14, 2024

seonghunYang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

seonghunYang left a comment

Choose a reason for hiding this comment

yougyung commented Mar 18, 2024

seonghunYang commented Mar 18, 2024

yougyung commented Mar 3, 2024 •

edited

Loading

seonghunYang Mar 13, 2024 •

edited

Loading

yougyung Mar 13, 2024 •

edited

Loading

seonghunYang Mar 14, 2024 •

edited

Loading

gahyuun Mar 13, 2024 •

edited

Loading