변경된 등급 이미지 적용 #151

DaleSeo · 2024-12-20T23:05:47Z

변경된 6단계 등급에 대한 이미지를 카드와 사이드바 컴포넌트에 적용하였습니다. 추가로 카드와 사이드바 간에 중복 코드가 있길래 별도 이미지 컴포넌트로 뽑아내서 깔끔하게 정리해보았습니다. 자세한 내용은 인라인 코멘트 참고 바랍니다.

체크리스트

이슈가 연결되어 있나요? fix 리더보드 UI에 변경된 등급 체계 적용 #150
배포 후 브라우저 콘솔에 경고나 오류가 있나요?

DaleSeo · 2024-12-20T23:09:01Z

src/pages/Leaderboard/Leaderboard.test.tsx

-  const userName = faker.internet.userName();
+  const userName = faker.internet.username();


테스트를 실행했더니 다음과 같은 depreaction waring이 뜨고 있어서 조치하였습니다.

[@faker-js/faker]: faker.internet.userName() is deprecated since v9.1.0 and will be removed in v10.0.0. Please use faker.internet.username() instead.

DaleSeo · 2024-12-21T00:11:03Z

src/components/Card/Card.module.css

-  > img {
-    width: 105px;
-    height: 128px;
-    object-fit: fill;


object-fit의 기본값은 fill이기 때문에 GradeImage 컴포넌트로 가져오지 않았습니다.

DaleSeo · 2024-12-21T00:12:46Z

src/components/Card/Card.tsx

-const imageTable = {
-  SEED: Seed,
-  SPROUT: Sprout,
-  LEAF: Sprout,
-  BRANCH: Sprout,
-  FRUIT: YoungTree,
-  TREE: LargeTree,
-};


제일 위에 등급 이미지를 import하는 부분과 여기에 있는 맵핑 로직이 Card 컴포넌트에도 있고 Sidebar 컴포넌트에도 있길래 GradeImage 컴포넌트로 뽑아내었습니다.

DaleSeo · 2024-12-21T00:14:01Z

src/components/Sidebar/Sidebar.tsx

@@ -93,9 +81,7 @@ export default function Sidebar({
        </section>

        <section className={styles.currentStatus}>
-          <figure>


여기에 사용된 <figure>는 별다른 역할이 없는 것 같아서 GradeImage 컴포넌트로 가져오지 않았습니다.

Oops 😓 알고보니 Chromatic의 UI Tests 결과를 보니 <figure>에 브라우저 디폴트 스타일시트가 먹고 있었네요. @SamTheKorean 님, 의도하신 것인지 확인 부탁드립니다. 의도하셨다면 부모 요소인 <section>의 스타일을 좀 조정해야겠네요.

아닙니다! 의도한게 아닌데 figure에 스타일을 따로 줬어야했는데 놓쳤군요...! 이 부분에 어떻게 해결하면 될까요?

@SamTheKorean 그럼 달라진 UI를 Accept 하도록 하겠습니다. 기존 UI와 피그마 디자인�을 대조해보니 <figure> 요소 때문에 위 아래로 불필요하게 16px의 공백이 더 들어있네요.

DaleSeo · 2024-12-21T00:17:21Z

src/components/Card/Card.stories.tsx

스토리북에서 grade prop을 정확히 입력해야되서 다른 등급의 카드를 보기가 어렵더라고요. 그래서 등급 별로 스토리를 나누었습니다.

Shot.2024-12-20.at.19.15.03.mp4

Sunjae95 · 2024-12-22T05:47:24Z

src/components/GradeImage/GradeImage.stories.tsx

+    grade: Grade.SEED,
+    width: 105,
+    height: 128,
+  },


Suggested change

},

argTypes: {

grade: {

options: [

Grade.SEED,

Grade.SPROUT,

Grade.LEAF,

Grade.BRANCH,

Grade.FRUIT,

Grade.TREE,

],

control: { type: "select" },

},

},

미팅에서 input으로 핸들링하는게 보기 좀 그렇다고 하셨는데, select로 핸들링하는 방법은 어떨까요?
storybook controls

이 방법은 순수 자바스크립트 프로젝트에서는 유용한데요. 타입스크립트를 쓰는 프로젝트에서는 이상적인 접근이 아닌 것 같아요. 스토리북에 자동으로 grade의 argTypes를 추론할 수 있어야 하는데, 하필 grade가 enum 타입이라서 문자열로 잘못 추론되고 있어요. grade의 타입을 union으로 변경해주면 굳이 이렇게 스토리만을 위해서 argTypes을 따로 명시하고 관리하지 않아도 해결이 될텐데 어떻게 생각하세용?

오호 options의 타입이 any[] | undefined 로 추론되는군요..?
@DaleSeo님 코멘트보고 union타입으로 GradeImageProps의 grade타입을 유니온으로 변경해봤는데 동일하게 any[] | undefined로 추론되는걸 확인했어요.
그런점에서 타입추론이 되지않는 argTypes를 사용해야하는 의문점과 argTypes는 자바스크립트에서 유용하다는건 새롭게 깨닫게 됐어요!
저는 현재 상황에서 typesafe하게 사용하려면 각 grade에 따른 스토리를 작성해야한다고 생각해요.

그런데 여러개의 컴포넌트를 합성하여 새로운 컴포넌트를 만들어 스토리를 만드는 경우는 다를거 같아요.
각 컴포넌트의 타입에 따른 경우의수와 다른 컴포넌트와 조합이 됐을 때 경우의수는 방대해지기에 타입추론이 되지 않는 argTypes를 사용해야하는 날은 올거 같다는 개인적인 의견입니다.😶

아 유니온이라면 혹시 배열로 나열하는것을 의미하는 걸 까요?
그에 대안이라면 Object.keys를 사용하는건 어떨까요?

argTypes: { grade: { control: "select", options: Object.keys(Grade), }, },

그런데 여러개의 컴포넌트를 합성하여 새로운 컴포넌트를 만들어 스토리를 만드는 경우는 다를거 같아요.
각 컴포넌트의 타입에 따른 경우의수와 다른 컴포넌트와 조합이 됐을 때 경우의수는 방대해지기에 타입추론이 되지 않는 argTypes를 사용해야하는 날은 올거 같다는 개인적인 의견입니다.😶

네네, 충분히 일리가 있는 의견이십니다. 그런 날이 오면 다시 얘기해보시죠 😉

아 유니온이라면 혹시 배열로 나열하는것을 의미하는 걸 까요?
그에 대안이라면 Object.keys를 사용하는건 어떨까요?

에궁... 제 설명이 제대로 전달이 안 된 것 같습니다. 요점은 스토리북이 추론해주는 argTypes를 덮어쓰는 것은 가급적 피하는 것이 좋다는 것입니다. 덮어쓰는 순간 컴포넌트 함수의 입력 타입과 스토리의 argTypes가 일치된 상태로 유지해야하는 오버헤드가 발생하거든요.

원래 티켓의 목표에서 벗어나는 주제로 PR 쓰레드가 너무 길어지고 있는 것 같아서 본 PR은 병합을 하도록 할께요. 그 다음, 제가 이상적이라고 생각하는 방식으로 코드를 수정해서 별도의 PR을 한번 올리고 리뷰 요청을 드리겠습니다.

위 논의와 관련하여 티켓 #157 을 생성하였습니다.

SamTheKorean

변경된 등급을 적용해주셔서 감사합니다!

DaleSeo linked an issue Dec 20, 2024 that may be closed by this pull request

리더보드 UI에 변경된 등급 체계 적용 #150

Closed

DaleSeo temporarily deployed to chromatic December 20, 2024 23:05 — with GitHub Actions Inactive

github-actions bot assigned DaleSeo Dec 20, 2024

DaleSeo force-pushed the 150-new-badges branch from 6f81529 to be656ca Compare December 20, 2024 23:07

DaleSeo temporarily deployed to chromatic December 20, 2024 23:07 — with GitHub Actions Inactive

DaleSeo commented Dec 20, 2024

View reviewed changes

DaleSeo force-pushed the 150-new-badges branch from be656ca to 07ba270 Compare December 20, 2024 23:22

DaleSeo had a problem deploying to chromatic December 20, 2024 23:22 — with GitHub Actions Failure

DaleSeo force-pushed the 150-new-badges branch from 07ba270 to b77f484 Compare December 20, 2024 23:23

DaleSeo temporarily deployed to chromatic December 20, 2024 23:23 — with GitHub Actions Inactive

DaleSeo force-pushed the 150-new-badges branch from b77f484 to 5ddc5a9 Compare December 20, 2024 23:29

DaleSeo temporarily deployed to chromatic December 20, 2024 23:29 — with GitHub Actions Inactive

DaleSeo temporarily deployed to chromatic December 21, 2024 00:07 — with GitHub Actions Inactive

DaleSeo changed the title ~~새로운 등급 이미지 적용~~ 변경된 등급 이미지 적용 Dec 21, 2024

DaleSeo commented Dec 21, 2024

View reviewed changes

DaleSeo marked this pull request as ready for review December 21, 2024 00:24

DaleSeo requested a review from a team as a code owner December 21, 2024 00:24

DaleSeo force-pushed the 150-new-badges branch from 7aa8c95 to 3304a40 Compare December 22, 2024 01:44

DaleSeo temporarily deployed to chromatic December 22, 2024 01:44 — with GitHub Actions Inactive

Sunjae95 reviewed Dec 22, 2024

View reviewed changes

DaleSeo force-pushed the 150-new-badges branch from 3304a40 to 6e9f6ba Compare December 22, 2024 21:05

DaleSeo temporarily deployed to chromatic December 22, 2024 21:05 — with GitHub Actions Inactive

SamTheKorean approved these changes Dec 22, 2024

View reviewed changes

DaleSeo added 2 commits December 23, 2024 16:09

replace grade images

c4fc543

refactor: add GradeImage

18abda1

DaleSeo force-pushed the 150-new-badges branch from 6e9f6ba to 18abda1 Compare December 23, 2024 21:09

DaleSeo deployed to chromatic December 23, 2024 21:09 — with GitHub Actions View deployment

DaleSeo mentioned this pull request Dec 23, 2024

스토리북에서 grade 컨트롤이 select 타입으로 표시되지 않는 현상 #157

Closed

DaleSeo merged commit e5f777a into main Dec 23, 2024
5 of 6 checks passed

DaleSeo deleted the 150-new-badges branch December 23, 2024 21:15

DaleSeo mentioned this pull request Dec 23, 2024

Grade의 타입을 Enum에서 Union으로 변경 #158

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

변경된 등급 이미지 적용 #151

변경된 등급 이미지 적용 #151

DaleSeo commented Dec 20, 2024 •

edited

Loading

DaleSeo Dec 20, 2024

DaleSeo Dec 21, 2024

DaleSeo Dec 21, 2024

DaleSeo Dec 21, 2024

DaleSeo Dec 21, 2024 •

edited

Loading

SamTheKorean Dec 22, 2024

DaleSeo Dec 22, 2024

DaleSeo Dec 21, 2024

Sunjae95 Dec 22, 2024

DaleSeo Dec 22, 2024 •

edited

Loading

Sunjae95 Dec 23, 2024

Sunjae95 Dec 23, 2024

DaleSeo Dec 23, 2024

DaleSeo Dec 23, 2024 •

edited

Loading

SamTheKorean left a comment

		const userName = faker.internet.userName();
		const userName = faker.internet.username();

-  },
+argTypes: {
+    grade: {
+      options: [
+        Grade.SEED,
+        Grade.SPROUT,
+        Grade.LEAF,
+        Grade.BRANCH,
+        Grade.FRUIT,
+        Grade.TREE,
+      ],
+      control: { type: "select" },
+    },
+  },

변경된 등급 이미지 적용 #151

변경된 등급 이미지 적용 #151

Conversation

DaleSeo commented Dec 20, 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

DaleSeo Dec 21, 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

DaleSeo Dec 22, 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

DaleSeo Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

SamTheKorean left a comment

Choose a reason for hiding this comment

DaleSeo commented Dec 20, 2024 •

edited

Loading

DaleSeo Dec 21, 2024 •

edited

Loading

DaleSeo Dec 22, 2024 •

edited

Loading

DaleSeo Dec 23, 2024 •

edited

Loading