feat : create aside component #81

SamTheKorean · 2024-11-22T03:49:04Z

참고 사항

여러 곳곳 스타일에 손봐야할 곳이 많지만 데모를 위해 틀만 잡아놓았고 데이터 연결까지 완료한 뒤에 재방문하여 적용할 예정입니다!. 인터페이스 설계도 임의로 했는데 실제 훅과 연결하면서 리팩토링할 예정입니다.

데모 사진

데모 영상

Screen.Recording.2024-11-25.at.11.18.55.PM.mov

체크리스트

이슈가 연결되어 있나요?

src/components/Aside/Aside.tsx

SamTheKorean · 2024-11-23T18:32:15Z

실제 데이터 연결까지 완료한 상황입니다! 다만 progress test가 깨져서 공유를 위해 draft pr로 전환하고 푸시했습니다!

src/components/Progress/Progress.test.tsx

src/components/Progress/Progress.tsx

SamTheKorean · 2024-11-28T00:16:29Z

공유해주신 피드백 반영하고 pr다시 오픈했습니다! 시간내어 피드백주셔서 감사드립니다!

Sunjae95 · 2024-11-28T05:43:39Z

src/components/Aside/Aside.stories.tsx

+
+export const HighProgress: StoryObj<typeof meta> = {
+  args: {
+    ...Default.args,


https://github.com/DaleStudy/leaderboard/pull/72/files#r1845802404
달레님 리뷰개선안인데요. meta에서 args를 선언해주면 매번 초기화를 안할 수 있어요!

Sunjae95 · 2024-11-28T06:08:31Z

src/components/Progress/Progress.tsx

+  const {
+    easy: easyProblemsCount,
+    medium: mediumProblemsCount,
+    hard: hardProblemsCount,
+  } = problemCounts;
+
+  const solvedCounts = member.solvedProblems.reduce(
+    (acc, problem) => {
+      acc[problem.difficulty] = (acc[problem.difficulty] || 0) + 1;
+      acc.total += 1;
+      return acc;
+    },
+    { easy: 0, medium: 0, hard: 0, total: 0 },
+  );
+
+  const {
+    easy: easySolved,
+    medium: mediumSolved,
+    hard: hardSolved,
+    total: totalSolved,
+  } = solvedCounts;
+
+  const easyProgress = `${easySolved}/${easyProblemsCount}`;
+  const mediumProgress = `${mediumSolved}/${mediumProblemsCount}`;
+  const hardProgress = `${hardSolved}/${hardProblemsCount}`;


저는 개인적으로 이 로직은 aside에서만 사용하기에 aside에 있어야한다고 생각해요.
왜냐하면 Progress에서 easyProgress, mediumProgress등은 다른 컴포넌트에서 사용하지 않기 때문에 Progress에서 관리할 필요가 없다고 생각합니다. sam님이 보시기엔 어떠신가요??

피드백 감사합니다! 저는 member를 넘겨주고 aside에 다시 member를 위한 인터페이스를 만들고 또 prop으로 받아서 데이터를 정제해서 사용하는 것보단 member를 page 래밸에서 받아서 필요한 정보들을 내려주는게 더 간단하다고 생각했습니다. Progress 페이지에서 진행상황에 관련된 로직을 처리해주고 aside는 유저 프로필 정보들을 표출하는 것에 초점을 맞춰도 문맥에 어색하지 않다고 느껴집니다.

선재님 말씀대로 각각 컴포넌트들이 필요한 로직들을 전부 처리하는게 로직이 분리되고 progress 페이지가 간소화되고 프로젝트가 커진다면 분리가 필연적이라고 생각합니다! 다만 현재 프로젝트 요구사항 범위를 볼 때는 아직은 조금이라도 간단한 구현이 이득이라고 판단했습니다!

추가적으로 아까 테스트 작성의 간편함을 근거로 제시해서 로직을 옮기는 걸 추천해주셨는데 어떤 측면에서 테스트가 용이해지는 막상 코드를 작성하려다 보니 이해가 안됩니다..! 혹시 제가 놓치는게 있다면 피드백 부탁드리겠습니다!

@SamTheKorean 님 정성스런 코멘트 답변 감사합니다.
저도 항상 컴포넌트 내부에서 데이터를 다룰지 여부를 많이 고민하는데요. 정답은 없는거 같더라구요... sam님께서 작성해주신 이유라면 저역시 동의합니다!

저의 의도는 find가 완료된 member자체를 props로 받아서 처리해보는건 어떨까요?라는 질문의도였는데요.

type정의는 기존 api 타입을 prop로 받으면 되는 부분이며 테스트는 member 단일 목데이터를 추가하면 테스트가 용이하다고 생각해요.
테스트 항목은 문제 표기(current/total)을 member만 넣으면 잘 표기되는지를 확인하자였습니다. 그러면 정제되지 않는 데이터가 컴포넌트를 통해 출력을 테스트할 수 있다고 생각해요.

여기까지가 저의 의도였는데, 코멘트달면서 member.solvedProblem의 방대한 목데이터 어떻게 만들지?라고 생각드네요. 현재 sam님께서 작성한 방법이 옳다고 생각합니다!

수고하셨습니다!

넵 말씀하신대로 type은 그대로 받으면 되는데 solvedProblem의 목데이터들을 만드는게 번거롭게 느껴지는 것 같아서 제차 질문 드렸습니다! 덕분에 설계에 대해서 다시 한 번 생각해볼 수 있었습니다! 시간내어 피드백 내어 주셔서 감사합니다!

sounmind · 2024-11-29T03:03:12Z

src/constants/problems.ts

+export const problemCounts = problems.reduce(
+  (acc, problem) => {
+    acc[problem.difficulty] += 1;
+    return acc;
+  },
+  { easy: 0, medium: 0, hard: 0 },
+);


SamTheKorean self-assigned this Nov 22, 2024

SamTheKorean requested a review from a team as a code owner November 22, 2024 03:49

SamTheKorean linked an issue Nov 22, 2024 that may be closed by this pull request

Aside Component #65

Open

SamTheKorean force-pushed the sam/create-aside-component branch from 11940ef to 8c365cf Compare November 22, 2024 03:55

sounmind requested changes Nov 23, 2024

View reviewed changes

src/components/Aside/Aside.tsx Show resolved Hide resolved

src/components/Aside/Aside.tsx Outdated Show resolved Hide resolved

Sunjae95 reviewed Nov 23, 2024

View reviewed changes

src/components/Aside/Aside.tsx Outdated Show resolved Hide resolved

SamTheKorean force-pushed the sam/create-aside-component branch from 8c365cf to 3a717e3 Compare November 23, 2024 18:30

SamTheKorean marked this pull request as draft November 23, 2024 18:30

SamTheKorean force-pushed the sam/create-aside-component branch from 3a717e3 to bedb21c Compare November 23, 2024 18:34

SamTheKorean commented Nov 23, 2024

View reviewed changes

src/components/Progress/Progress.test.tsx Show resolved Hide resolved

SamTheKorean force-pushed the sam/create-aside-component branch 2 times, most recently from aeb3650 to 85719e5 Compare November 23, 2024 20:56

SamTheKorean changed the title ~~Sam/create aside component~~ feat : create aside component Nov 23, 2024

SamTheKorean force-pushed the sam/create-aside-component branch 7 times, most recently from 9ac1093 to 0a73e43 Compare November 26, 2024 04:21

sounmind reviewed Nov 26, 2024

View reviewed changes

src/components/Progress/Progress.tsx Outdated Show resolved Hide resolved

sounmind reviewed Nov 26, 2024

View reviewed changes

src/components/Progress/Progress.tsx Outdated Show resolved Hide resolved

sounmind reviewed Nov 26, 2024

View reviewed changes

src/components/Progress/Progress.tsx Outdated Show resolved Hide resolved

SamTheKorean marked this pull request as ready for review November 27, 2024 01:59

SamTheKorean commented Nov 27, 2024

View reviewed changes

src/components/Progress/Progress.tsx Outdated Show resolved Hide resolved

SamTheKorean force-pushed the sam/create-aside-component branch 4 times, most recently from c5642e2 to 34d57b1 Compare November 28, 2024 00:01

SamTheKorean requested review from Sunjae95 and sounmind November 28, 2024 00:16

Sunjae95 reviewed Nov 28, 2024

View reviewed changes

sounmind reviewed Nov 29, 2024

View reviewed changes

Sunjae95 approved these changes Nov 30, 2024

View reviewed changes

SamTheKorean force-pushed the sam/create-aside-component branch from 34d57b1 to 13e9c8b Compare November 30, 2024 05:36

SamTheKorean temporarily deployed to chromatic November 30, 2024 05:36 — with GitHub Actions Inactive

SamTheKorean force-pushed the sam/create-aside-component branch from 13e9c8b to 546f0cb Compare November 30, 2024 05:43

SamTheKorean temporarily deployed to chromatic November 30, 2024 05:43 — with GitHub Actions Inactive

SamTheKorean force-pushed the sam/create-aside-component branch from 546f0cb to 4a6a66c Compare November 30, 2024 05:45

SamTheKorean temporarily deployed to chromatic November 30, 2024 05:45 — with GitHub Actions Inactive

SamTheKorean force-pushed the sam/create-aside-component branch from 4a6a66c to 62cd576 Compare November 30, 2024 05:47

SamTheKorean temporarily deployed to chromatic November 30, 2024 05:47 — with GitHub Actions Inactive

SamTheKorean added 9 commits November 30, 2024 00:48

feat : aside component

53871d4

feat : implement aside component

e827c79

feat : made modules and adjust progress styles

368d354

feat : test and adjusted progress page

97c6514

feat : add stories

61364ab

feat : add sample image

6ddcd89

fix : convert button to link and adjusted test accordingly

7a313a1

feat : convert profileurl to pass as prop

43b5631

feat : add png files

aa4d1ad

SamTheKorean force-pushed the sam/create-aside-component branch from 62cd576 to aa4d1ad Compare November 30, 2024 05:48

SamTheKorean deployed to chromatic November 30, 2024 05:48 — with GitHub Actions View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat : create aside component #81

feat : create aside component #81

SamTheKorean commented Nov 22, 2024 •

edited

Loading

SamTheKorean commented Nov 23, 2024

SamTheKorean commented Nov 28, 2024

Sunjae95 Nov 28, 2024

Sunjae95 Nov 28, 2024

SamTheKorean Nov 29, 2024 •

edited

Loading

Sunjae95 Nov 29, 2024

SamTheKorean Nov 30, 2024

sounmind Nov 29, 2024

feat : create aside component #81

Are you sure you want to change the base?

feat : create aside component #81

Conversation

SamTheKorean commented Nov 22, 2024 • edited Loading

참고 사항

데모 사진

데모 영상

체크리스트

SamTheKorean commented Nov 23, 2024

SamTheKorean commented Nov 28, 2024

Sunjae95 Nov 28, 2024

Choose a reason for hiding this comment

Sunjae95 Nov 28, 2024

Choose a reason for hiding this comment

SamTheKorean Nov 29, 2024 • edited Loading

Choose a reason for hiding this comment

Sunjae95 Nov 29, 2024

Choose a reason for hiding this comment

SamTheKorean Nov 30, 2024

Choose a reason for hiding this comment

sounmind Nov 29, 2024

Choose a reason for hiding this comment

SamTheKorean commented Nov 22, 2024 •

edited

Loading

SamTheKorean Nov 29, 2024 •

edited

Loading