Result refactor/#137 #140

yougyung · 2024-09-05T13:16:49Z

📌 작업 내용

결과페이지 및 로그인페이지에 있는 개선 사항들을 수정했습니다

2024-09-05.10.05.33.mov

구현 내용 및 작업 했던 내역

카테고리별 진행률 0% 의 경우 게이지 hidden
아이디 찾기, 비밀번호 찾기 redirect
result page 스켈레톤 높이 조절
성적표 결과 일반교양의 채플관련 안내
카테고리별 만족도에 따른 차트 색상 처리(파란색)
이수과목 없는 경우 안내문구 제공

🔊 도움이 필요한 부분

1. `UserInfoCard`의 타입가드

UserInfoCard의 타입가드를 목적으로 생성한 renderUserInfo함수를 보면, 사용자의 성적표 입력여부(user type)에 따라 다른 컴포넌트를 노출하는 것을 확인할 수 있습니다.

하지만 init user는 결과페이지에 진입하지 못하기 때문에 InitUserAnnounce는 실제로 노출될 수 없는 특징을 갖고 있고, 노출될 수 없는 InitUserAnnounce 컴포넌트가 존재하는 것이 추후 함께하게 될 개발자의 입장에서 혼란스러울 수 있겠다는 생각이 들어 제거를 진행했어요. 더불어 고민을 진행하면서 타입 단언이 오히려 해당 컴포넌트에 적절하지 않을까? 라는 생각이 들었습니다. 아래 두가지 의견 때문인데, 팀원분들의 의견이 궁금해요.

type에 따른 분기처리가 요구되지 않는 컴포넌트에 해당한다
새로운 동료개발자의 도메인 이해를 돕기 쉽다

2. 이수과목 없는 경우 안내문구 제공

이수과목 없는 경우 안내문구 제공하는 경우 일관된 UI를 노출하기 위해서 Table컴포넌트를 제거하지 않았는데, 팀원분들의 의견을 통해 조정해보고 싶어요!

github-actions · 2024-09-05T13:19:15Z

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

github-actions · 2024-09-05T14:08:17Z

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

github-actions · 2024-09-05T14:13:00Z

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

gahyuun

-21 학점으로 노출되는 텍스트, 학점이 부족한데 100%로 보이는 것에 대해 수정이 필요할 것 같아요

gahyuun

2024-09-06.12.38.24.mov

일반 교양이 두 번 보입니다 확인 부탁드립니다 !

gahyuun · 2024-09-06T03:31:12Z

app/ui/user/user-info-card/user-info-card.tsx

  function renderUserInfo(data: UserInfoResponse | InitUserInfoResponse) {
-    return isInitUser(data) ? <InitUserAnnounce /> : <UserInfoContent data={data} />;
+    return isInitUser(data) ? <></> : <UserInfoContent data={data} />;


분기처리를 하는 컴포넌트가 아니므로 저도 단언이 더 적절하다고 생각합니다

gahyuun · 2024-09-06T03:34:25Z

app/ui/result/result-category-detail-lecture/result-cagegory-detail-lecture.tsx

+          {showNoneLecture && <AnnounceMessageBox message="이수한 과목 정보가 존재하지 않습니다." />}
+        </>


이수한 과목이 없을 때 나타나는 선을 지워야할 것 같습니다. 또한 이수한 과목이 없는 건데, 체크 이모지가 적절하지 않은 것 같습니다!
UI가 일관되는 게 더 나을 것 같아 테이블 컴포넌트를 제거 할 필요는 없어보입니다

List component에서 props로 받고 있는emptyDataRender를 Table 컴포넌트에서도 props로 받아 전달할 수 있게 변경하였습니다. List 컴포넌트를 잘 만들어주셔서, Table 컴포넌트를 통해서 data가 빈경우를 처리할 수 있게되어 코드가 훨씬 깔끔하게 작성될 수 있었던 것 같아요. 86f417d

gahyuun · 2024-09-06T03:38:02Z

app/ui/result/result-category-detail-content/result-category-detail-content.tsx

기준학점보다 이수학점이 더 많을 때 파란색으로 숫자를 표기하는 게 더 낫지않을까요?

gahyuun

2024-09-06.12.38.24.mov

일반 교양이 두 번 보입니다 확인 부탁드립니다 !

yougyung · 2024-09-06T04:47:46Z

2024-09-06.12.38.24.mov
일반 교양이 두 번 보입니다 확인 부탁드립니다 !

정말정말 감사합니다 .......! e3915c2

github-actions · 2024-09-06T04:48:01Z

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

github-actions · 2024-09-06T04:49:20Z

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

yougyung · 2024-09-06T04:49:38Z

-21 학점으로 노출되는 텍스트, 학점이 부족한데 100%로 보이는 것에 대해 수정이 필요할 것 같아요

다음 PR로 졸업만족여부에 따라 다르게 텍스트를 노출하는 기능이 올라갈 예정입니다. 해당 과정에서 관련 문제를 해결하도록 할게요 !

seonghunYang · 2024-09-06T05:51:29Z

app/ui/user/user-info-card/user-info-card.tsx

  function renderUserInfo(data: UserInfoResponse | InitUserInfoResponse) {
-    return isInitUser(data) ? <InitUserAnnounce /> : <UserInfoContent data={data} />;
+    return isInitUser(data) ? <></> : <UserInfoContent data={data} />;


도움이 필요한 부분 잘 읽어봤습니다. 제 생각에는 타입 단언보다 기존 코드가 더 적절합니다. 이유는 타입 단언을 사용하면 논리적 문제가 생기고 예기치 않은 오류가 발생할 수 있기 때문입니다.

user-info-card는 user 도메인에 위치한 도메인 컴포넌트이므로, 이 컴포넌트가 어느 페이지에서 사용되는지는 관심사를 벗어납니다. 현재 논의에서는 이 컴포넌트가 결과 페이지에서만 사용된다고 가정하고 initUser를 고려하지 않아도 된다고 생각하지만, 실제로는 어느 페이지에서 이 컴포넌트가 재사용될지 모르기 때문에 타입 단언을 사용하면 재사용성이 떨어질 수 있습니다.

물론 타입 단언을 해도 당장은 문제가 생기지 않겠지만, 이 컴포넌트는 result 페이지와 같이 유저가 이미 초기화된 곳에서만 사용할 수 있게 됩니다. 오히려 이렇게 되면 코드를 이해하기 더 어렵게 만들고 재사용성을 떨어트린다 생각합니다. 예를 들어, 새로운 동료 개발자가 새 페이지를 개발하면서 이 컴포넌트를 재사용하려 할 때, 초기화되지 않은 유저로 인해 예상치 못한 문제가 발생할 수 있습니다.

해결 방법은 두 가지가 있습니다. 첫째, 기존대로 초기화되지 않은 유저를 처리하는 로직을 만드는 것입니다. 둘째, 만약 이 컴포넌트가 결과 페이지 외에는 사용되지 않을 것이 확실하기에 재사용성을 고려하고 싶지 않다면, 아키텍처상 위치를 변경하여 결과 페이지 전용 컴포넌트임을 명확히 하는 것입니다.

번외로 저는 기존 InitUserAnnounce 컴포넌트처럼 (아니면 컴포넌트 명을 차라리 NoData와 같이 문제가 있음을 나타내는 방식으로 표현해도 됩니다) "성적표 PDF를 업로드해야 졸업사정 결과를 확인할 수 있어요."라고 알려주는 게 동료 개발자가 코드를 읽을 때도 더 이해가 잘 될 거라고 생각이 드네요.

user-info-card가 도메인 컴포넌트라는 가정을 놓치고 있었네요. 의견 감사합니다! bbb0101

github-actions · 2024-09-06T06:21:46Z

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

This reverts commit e511297.

github-actions · 2024-09-06T06:30:59Z

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

seonghunYang

수고하셨습니다

yougyung added 6 commits August 27, 2024 01:14

fix: change result-detail skeleton

52e55ed

fix: cover pie-chart to over 100%

1cf2132

fix: set find-auth redirect

8caf438

feat: add chaple announce

89bf6a5

feat: add announce text in none lecture case

5682e15

chore: change credit mock data

fe2f9d9

yougyung self-assigned this Sep 5, 2024

yougyung requested review from seonghunYang and gahyuun as code owners September 5, 2024 13:16

yougyung marked this pull request as draft September 5, 2024 13:16

fix: remove unused component

e511297

yougyung marked this pull request as ready for review September 5, 2024 14:08

chore: remove unused favicon

351c0a8

gahyuun requested changes Sep 6, 2024

View reviewed changes

yougyung added 2 commits September 6, 2024 13:46

fix: set percentage max 100

a606dca

fix: change typo 일반교양 -> 공통교양

e3915c2

seonghunYang reviewed Sep 6, 2024

View reviewed changes

yougyung added 3 commits September 6, 2024 15:10

style: remove focus classname

a140969

refactor: add emptyDataRender in table

86f417d

fix: change showCompleted state

eca4647

Revert "fix: remove unused component"

bbb0101

This reverts commit e511297.

yougyung requested review from gahyuun and seonghunYang September 6, 2024 06:31

gahyuun approved these changes Sep 7, 2024

View reviewed changes

seonghunYang approved these changes Sep 7, 2024

View reviewed changes

yougyung merged commit 168e144 into main Sep 7, 2024
2 of 3 checks passed

yougyung deleted the result-refactor/#137 branch September 7, 2024 12:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Result refactor/#137 #140

Result refactor/#137 #140

yougyung commented Sep 5, 2024 •

edited

Loading

github-actions bot commented Sep 5, 2024

github-actions bot commented Sep 5, 2024

github-actions bot commented Sep 5, 2024

gahyuun left a comment

gahyuun left a comment

gahyuun Sep 6, 2024

gahyuun Sep 6, 2024

yougyung Sep 6, 2024 •

edited

Loading

gahyuun Sep 6, 2024

yougyung Sep 6, 2024

gahyuun left a comment

yougyung commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

yougyung commented Sep 6, 2024

seonghunYang Sep 6, 2024

yougyung Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

seonghunYang left a comment

		{showNoneLecture && <AnnounceMessageBox message="이수한 과목 정보가 존재하지 않습니다." />}
		</>

Result refactor/#137 #140

Result refactor/#137 #140

Conversation

yougyung commented Sep 5, 2024 • edited Loading

📌 작업 내용

🔊 도움이 필요한 부분

1. UserInfoCard의 타입가드

2. 이수과목 없는 경우 안내문구 제공

github-actions bot commented Sep 5, 2024

github-actions bot commented Sep 5, 2024

github-actions bot commented Sep 5, 2024

gahyuun left a comment

Choose a reason for hiding this comment

gahyuun left a comment

Choose a reason for hiding this comment

gahyuun Sep 6, 2024

Choose a reason for hiding this comment

gahyuun Sep 6, 2024

Choose a reason for hiding this comment

yougyung Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

gahyuun Sep 6, 2024

Choose a reason for hiding this comment

yougyung Sep 6, 2024

Choose a reason for hiding this comment

gahyuun left a comment

Choose a reason for hiding this comment

yougyung commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

yougyung commented Sep 6, 2024

seonghunYang Sep 6, 2024

Choose a reason for hiding this comment

yougyung Sep 6, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

seonghunYang left a comment

Choose a reason for hiding this comment

yougyung commented Sep 5, 2024 •

edited

Loading

1. `UserInfoCard`의 타입가드

yougyung Sep 6, 2024 •

edited

Loading