[#407][독서 모임] 참여중인 모임 포스트 컴포넌트 #422

WooDaeHyun · 2023-10-18T15:28:58Z

구현 내용

내가 가입한 독서모임 simple ui 컴포넌트 구현

스크린샷

pr 포인트

모임 제목을 표현하는 곳에 줄바꿈을 현재 단어를 기준으로 해놓았는데, figma에서는 글자를 기준으로 줄바꿈을 하고 있습니다. 시각적으로 단어를 기준으로 줄바꿈을 하는 것이 더 괜찮아 보여 우선 구현을 했는데 이 부분과 관련하여 의견 공유 해주시면 감사하겠습니다.
next img를 사용했는데 width, height가 정상 적용되지 않아 인라인 스타일로 적용했습니다. 여러분의 많은 관심이 필요합니다.

Help

관련 이슈

Close [독서모임] 참여중인 모임 포스트 컴포넌트 #407

vercel · 2023-10-18T15:29:04Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
dadok	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 5, 2023 0:30am

WooDaeHyun · 2023-10-18T15:32:49Z

@minjongbaek @gxxrxn @hanyugeon

github-actions · 2023-10-18T15:33:26Z

🚀 Storybook: https://64aba10cd478ad51307a8997-osglevdeoo.chromatic.com/

src/v1/bookgroup/SimpleGroup.tsx

minjongbaek · 2023-10-18T16:36:55Z

src/v1/bookgroup/SimpleGroup.tsx

+  return (
+    <div
+      className="flex h-[15rem] w-[10rem] flex-col gap-[0.5rem]"
+      onClick={eventHandler}


click 됐을때 사용자는 어떤 동작을 기대할 수 있나요?

이 부분과 관련해서는 의견을 공유한 적이 없어
click을 한 경우에는 모임 상세 페이지로 이동하는 것을 생각했습니다.
하지만 정확하게 정해진 내용이 없어 우선 사용하는 쪽에서 원하는 동작을 할 수 있도록
공통적인 의미로 eventHandler라고 네이밍 했습니다. 혹 제가 놓친 정해진 바가 있을까요?!

우선 click 이벤트에 대한 이벤트 핸들러이기 때문에 보다 클릭과 관련된 handler라는 이름으로 네이밍을 변경했습니다. 참고 부탁드려요~

페이지 이동만 하는거라면 를 통해 prefetch 하는게 더 좋을 것 같아서 코멘트를 남겼어요.

우선 click 이벤트에 대한 이벤트 핸들러이기 때문에 보다 클릭과 관련된 handler라는 이름으로 네이밍을 변경했습니다. 참고 부탁드려요~

dx 측면에서 컴포넌트를 사용할 때 <SimpleGroup eventHandler={...} /> 보다 <SimpleGroup onClick={...} /> 가 컴포넌트가 어떤 이벤트 동작을 수행할 수 있는지 유추하기 더 쉽고 명확하다고 생각해요.

https://stackoverflow.com/questions/60048249/what-is-the-right-name-of-event-handler-onclick-or-handleclick 를 읽어봐도 좋을 것 같아요.

src/v1/bookgroup/SimpleGroup.tsx

hanyugeon

고생하셨습니다!
코멘트 확인해주세요!

hanyugeon · 2023-10-19T11:33:29Z

src/stories/bookgroup/SimpleGroup.stories.tsx

+import SimpleGroup from '@/v1/bookgroup/SimpleGroup';
+
+const meta: Meta<typeof SimpleGroup> = {
+  title: 'Base/SimpleGroup',


p1;

Storybook에 타이틀을 BookGroup/SimpleGroup 으로 바꿔주세요!

hanyugeon · 2023-10-19T11:34:37Z

src/v1/bookgroup/SimpleGroup.tsx

+          <Image
+            src={imageSource}
+            alt="bookgroup"
+            width={64}
+            height={84}
+            style={{ width: '6.4rem', height: '8.4rem' }}
+          />


p1;

Image 스타일에 borderRadius: 0.412rem이 빠진것 같습니다

className={rounded-[0.412rem] w-[6.4rem] h-[8.4rem]} 와 같이 지정해줄 수도 있을것같아요.

대신 가변적인 이미지는 아니기 때문에
width={64} height={84}는 명시되어야 할 것 같군요 🤔

확인했습니다. 수정하겠습니다!~

@WooDaeHyun

이 코멘트에서 className으로 스타일링 하면 좋을것 같다고 했던 이유는
TailwindCSS로 className으로 스타일링을 하고있다보니
Image 컴포넌트를 스타일링할때도 코드 통일성을 가져간다면 더 좋지 않을까서 였습니다!!

hanyugeon · 2023-10-19T11:36:07Z

src/v1/bookgroup/SimpleGroup.tsx

+      className="flex h-[15rem] w-[10rem] flex-col gap-[0.5rem]"
+      onClick={eventHandler}
+    >
+      <div className="flex h-[11.6rem] w-[10rem] items-center justify-center rounded-[0.534rem] bg-orange-100">


p3;

스타일 속성에 items-center justify-center 대신
px-[1.8rem] py-[1.6rem]를 넣어서
컨테이너와 내부 이미지의 간격을 명시적으로 나타내줄 수 있을것같아요!

just ask;
그런 경우에 더 좋은 점이 있나요??

일단 명시적으로 주는것이 명확하게 어떤 스타일을 지정하면 코드를 보면서도 정확하게 레이아웃을 이해할 수 있다는 측면에서 수정하는것으로 이해해 수정작업을 했습니다! 확인 부탁드려요~

src/v1/bookgroup/SimpleGroup.tsx

WooDaeHyun · 2023-10-19T15:40:06Z

@minjongbaek @hanyugeon

WooDaeHyun · 2023-11-04T09:28:16Z

@gxxrxn @hanyugeon 컴포넌트명 변경 및 리뷰들 반영완료했습니다~! 확인 부탁드려요

minjongbaek · 2023-11-04T11:58:32Z

src/v1/bookgroup/SimpleBookGroupCard.tsx

+  title: string;
+  isOwner: boolean;
+  imageSource: string;
+  handleClick: () => void;


@WooDaeHyun 저는 개인적으로 이벤트에 대한 핸들링을 prop 을 통해 전달받는 경우 onEventName 과 같은 DOM 이벤트 핸들러에 대한 기본 명명 규칙을 따르는 것을 선호해요.

컴포넌트 입장에서 생각했을 때 어떤 일을 하는 것이 중요한 것이 아닌, 어떤 이벤트가 발생했는지가 중요하다고 생각하기 때문이에요.

그래서 외부에서 넘겨주어야 하는 함수를 handleEventName 과 같은 형태로 작성하는게 더 명확하지 않을까 생각해요! <SimpeBookGroup onClick={handleClick} /> 처럼요.

이 부분은 주관적인 요소가 강해서 다른 분들의 이야기를 들어보고 싶기도 하네요. @gxxrxn @hanyugeon
이런 사소하게 보여지는 부분도 잘 정리해두면 코드 퀄리티 향상으로 이어질 수 있을 것 같아요.

그리고 클릭하면 페이지를 이동시켜주는 액션만 필요할 것 같은데 <Link /> 컴포넌트를 통해 서버 컴포넌트로의 활용과 prefetch를 통한 성능 향상도 고려해보면 좋을 것 같아요!

저는 개인적으로 이벤트에 대한 핸들링을 prop 을 통해 전달받는 경우 onEventName 과 같은 DOM 이벤트 핸들러에 대한 기본 명명 규칙을 따르는 것을 선호해요.
컴포넌트 입장에서 생각했을 때 어떤 일을 하는 것이 중요한 것이 아닌, 어떤 이벤트가 발생했는지가 중요하다고 생각하기 때문이에요.
그래서 외부에서 넘겨주어야 하는 함수를 handleEventName 과 같은 형태로 작성하는게 더 명확하지 않을까 생각해요! <SimpeBookGroup onClick={handleClick} /> 처럼요.

좋은 의견이라고 생각해요! 저도 onClick 혹은 더 세부적인 핸들러를 제공하고 싶다면 on**Click 등의 형태를 선호하는 것 같아요!!

클릭하면 페이지를 이동시켜주는 액션만 필요할 것 같은데 컴포넌트를 통해 서버 컴포넌트로의 활용과 prefetch를 통한 성능 향상도 고려해보면 좋을 것 같아요!

개인적으로는 SimpleBookGroupCard 내부에서 Link 태그를 사용한다면, 재사용성이 낮은 컴포넌트가 될 수 있다고 생각해요. 지금은 클릭하면 페이지 이동만 하는 컴포넌트일 수 있지만, 추후 모달을 띄우는 등의 다른 액션이 필요할 수도 있지 않을까요..?!

개인적으로는 SimpleBookGroupCard 내부에서 Link 태그를 사용한다면, 재사용성이 낮은 컴포넌트가 될 수 있다고 생각해요. 지금은 클릭하면 페이지 이동만 하는 컴포넌트일 수 있지만, 추후 모달을 띄우는 등의 다른 액션이 필요할 수도 있지 않을까요..?!

@gxxrxn 그럴 수 있겠네요. 모임 특성상 비밀번호나, 문제의 답을 요구하는 경우도 있으니 다양한 상황에 대응 가능한 구조가 좋을 것 같아요.

제가 handleClick으로 핸들러 명칭을 정한 이유는 컴포넌트 입장에서 어떤 기능을 하는 함수를 prop으로 넘겨 받는지를 명시적으로 표현할 수 있겠다는 생각에 명명하게 되었습니다. 하지만 두 분의 이야기를 듣고 보니 컴포넌트 입장에서 어떤 이벤트가 발생했는지를 기준으로 이벤트 핸들러 명칭을 정하는게 좋을 것 같네요!! 해당 부분 수정해서 다시 언급하도록 할게요~

그리고 두 번째 Link 태그와 관련된 부분은 @gxxrxn 님 말대로 여러 방향으로 활용가능할 수 있으니 현재 수정사항에서는 제외하고, 추후 별다른 기능을 부여하지 않는다면 @minjongbaek 말씀대로 Link 태그를 통해서 페이지를 이동시키는 용도로 변경하면 좋을 것 같습니다.

@gxxrxn @minjongbaek @hanyugeon

interface ... { .... .... onClick: () => void; }
<SimpeBookGroupCard onClick={handleClick} />

형태로 이벤트 핸들러 명칭을 변경했습니다!
확인 부탁드려요!

minjongbaek

👍

hanyugeon

@WooDaeHyun

수고하셨습니다~! 👍

WooDaeHyun added 🐥 프론트 필수! ✨ feature New feature or request 🔥 v.1.0 New feature for releasing v.1.0 labels Oct 18, 2023

WooDaeHyun requested review from minjongbaek, gxxrxn and hanyugeon October 18, 2023 15:28

WooDaeHyun self-assigned this Oct 18, 2023

WooDaeHyun changed the title ~~[독서 모임][#407]~~ [독서모임][#407] 참여중인 모임 포스트 컴포넌트 Oct 18, 2023

minjongbaek requested changes Oct 18, 2023

View reviewed changes

src/v1/bookgroup/SimpleGroup.tsx Outdated Show resolved Hide resolved

src/v1/bookgroup/SimpleGroup.tsx Outdated Show resolved Hide resolved

src/v1/bookgroup/SimpleGroup.tsx Outdated Show resolved Hide resolved

minjongbaek reviewed Oct 18, 2023

View reviewed changes

src/v1/bookgroup/SimpleGroup.tsx Outdated Show resolved Hide resolved

hanyugeon reviewed Oct 19, 2023

View reviewed changes

vercel bot deployed to Preview October 19, 2023 14:17 View deployment

WooDaeHyun added 2 commits October 20, 2023 00:03

feat: 내가 가입한 모임 simple ui 구현

9cc11aa

fix: 스타일 관련 코드 수정 및 불필요한 요소 제거

013854a

WooDaeHyun force-pushed the feat/#407 branch from db8e6c5 to 013854a Compare October 19, 2023 15:11

vercel bot deployed to Preview October 19, 2023 15:12 View deployment

WooDaeHyun changed the title ~~[독서모임][#407] 참여중인 모임 포스트 컴포넌트~~ [#407][독서 모임] 참여중인 모임 포스트 컴포넌트 Oct 22, 2023

minjongbaek previously approved these changes Oct 25, 2023

View reviewed changes

fix: 컴포넌트명 변경

856454c

WooDaeHyun dismissed minjongbaek’s stale review via 856454c November 4, 2023 09:21

vercel bot deployed to Preview November 4, 2023 09:22 View deployment

WooDaeHyun requested review from hanyugeon and minjongbaek November 4, 2023 09:26

minjongbaek reviewed Nov 4, 2023

View reviewed changes

fix: 이벤트 핸들러 명칭 변경

bccc4ce

vercel bot deployed to Preview November 5, 2023 12:30 View deployment

WooDaeHyun requested a review from minjongbaek November 5, 2023 12:33

minjongbaek approved these changes Nov 5, 2023

View reviewed changes

hanyugeon approved these changes Nov 6, 2023

View reviewed changes

WooDaeHyun merged commit a63a012 into main Nov 6, 2023
3 checks passed

WooDaeHyun deleted the feat/#407 branch November 6, 2023 23:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#407][독서 모임] 참여중인 모임 포스트 컴포넌트 #422

[#407][독서 모임] 참여중인 모임 포스트 컴포넌트 #422

WooDaeHyun commented Oct 18, 2023 •

edited

Loading

vercel bot commented Oct 18, 2023 •

edited

Loading

WooDaeHyun commented Oct 18, 2023

github-actions bot commented Oct 18, 2023 •

edited

Loading

minjongbaek Oct 18, 2023

WooDaeHyun Oct 19, 2023

WooDaeHyun Oct 19, 2023

minjongbaek Oct 20, 2023

minjongbaek Oct 20, 2023

hanyugeon left a comment

hanyugeon Oct 19, 2023

WooDaeHyun Oct 19, 2023

hanyugeon Oct 19, 2023 •

edited

Loading

WooDaeHyun Oct 19, 2023

hanyugeon Oct 22, 2023

hanyugeon Oct 19, 2023 •

edited

Loading

WooDaeHyun Oct 19, 2023

WooDaeHyun Oct 19, 2023

WooDaeHyun commented Oct 19, 2023

WooDaeHyun commented Nov 4, 2023

minjongbaek Nov 4, 2023

minjongbaek Nov 4, 2023

gxxrxn Nov 4, 2023

minjongbaek Nov 4, 2023 •

edited

Loading

WooDaeHyun Nov 5, 2023

WooDaeHyun Nov 5, 2023

minjongbaek left a comment

hanyugeon left a comment •

edited

Loading

[#407][독서 모임] 참여중인 모임 포스트 컴포넌트 #422

[#407][독서 모임] 참여중인 모임 포스트 컴포넌트 #422

Conversation

WooDaeHyun commented Oct 18, 2023 • edited Loading

구현 내용

스크린샷

pr 포인트

Help

관련 이슈

vercel bot commented Oct 18, 2023 • edited Loading

WooDaeHyun commented Oct 18, 2023

github-actions bot commented Oct 18, 2023 • 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

hanyugeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanyugeon Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanyugeon Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WooDaeHyun commented Oct 19, 2023

WooDaeHyun commented Nov 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minjongbaek Nov 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minjongbaek left a comment

Choose a reason for hiding this comment

hanyugeon left a comment • edited Loading

Choose a reason for hiding this comment

WooDaeHyun commented Oct 18, 2023 •

edited

Loading

vercel bot commented Oct 18, 2023 •

edited

Loading

github-actions bot commented Oct 18, 2023 •

edited

Loading

hanyugeon Oct 19, 2023 •

edited

Loading

hanyugeon Oct 19, 2023 •

edited

Loading

minjongbaek Nov 4, 2023 •

edited

Loading

hanyugeon left a comment •

edited

Loading