implement List molecule component/#18 #27

gahyuun · 2024-02-16T16:47:31Z

📌 작업 내용

구현 내용 및 작업 했던 내역

list molecule component 구현
추가,삭제 버튼이 list row에 존재할 수 있도록 수정이 필요합니다!
이 부분은 기이수 과목 삭제 기능을 구현하면서 수정 할 예정입니다

🤔 고민 했던 부분

list row 컴포넌트에서 column이 일정하게 정해진 크기를 가져야합니다. 처음에는 table 태그를 사용해서 table-layout: fixed;
속성을 사용하려 했지만, 아래 두 가지 이유로 기존 졸부 코드랑 비슷하게 width 19%, width 33%로 가져갔습니다
더 좋은 방법이 있을까요?

table 사용시 rounded 적용 문제
list row만 사용하는 컴포넌트 존재

🔊 도움이 필요한 부분

select component와 아래 두 아티클을 참고해서 compound component pattern을 적용해보았습니다!
object.assign 사용하지 않는게 더 한 눈에 들어오는 것 같아서 사용하지는 않았는데, select component랑 비슷한 구현 방식을 가져가는게 통일성에 더 좋을까요??
https://fe-developers.kakaoent.com/2022/220731-composition-component/
https://im-developer.tistory.com/238

github-actions · 2024-02-16T16:49:10Z

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

seonghunYang · 2024-02-17T07:45:48Z

app/ui/view/molecule/list/index.tsx

+export function List({ children }: ListProps) {
+  const getChildren = (type: typeof ListHeader | typeof ListRow) => {
+    const childrenArray = Children.toArray(children);
+    return childrenArray.filter((child) => isValidElement(child) && child.type === type);
+  };


[request change]

이렇게 함수형 컴포넌트 내부에 함수를 선언하면, 리렌더링할 때마다 함수가 재생성됩니다.

해당 함수는 List 함수 바깥으로 빼도 문제가 없어 보여서 빼는게 좋아 보입니다.

디테일을 신경쓰지 못했네요 수정했습니다!
91f3565

seonghunYang · 2024-02-17T07:46:52Z

app/ui/view/molecule/list/list-header.tsx

+          key={index}
+          className={twMerge(
+            'text-[#7590ff] leading-4 text-lg font-bold w-[19%] text-center',
+            columns.length === 5 ? 'w-[19%]' : 'w-[33%]',


[request change]

해당 상황에는 grid를 사용하면 좋을 것 같네요! width로 설정하는건 제가 했겠지만 별로인 것 같습니다.

seonghunYang · 2024-02-17T07:51:25Z

app/ui/view/molecule/list/index.tsx

+  children: ReactNode;
+};
+
+export function List({ children }: ListProps) {


[comment]

리스트에서는 컴파운드 패턴과 비슷한 방식으로 렌더 프롭스 패턴을 자주 사용합니다.

이 패턴을 사용하면, 컴파운드 패턴처럼 의존성을 역전시키고 인터페이스를 분리하는 장점이 있습니다.

또한, list와 같은 자료구조와 UI에 적합한 패턴입니다.

링크를 첨부하니, 한번 확인하시고 이 패턴이 좋아보이면 적용해도 좋을 듯 하네요!

https://www.patterns.dev/react/render-props-pattern

seonghunYang · 2024-02-17T07:54:15Z

app/ui/view/molecule/list/index.tsx

+List.Header = ListHeader;
+List.Row = ListRow;


[comment]

저는 가독성 측면에서 assign을 사용하는게 더 좋다고 생각했어요!

각각 컴포넌트가 따로 사용되는게 아니라 컴파운트 패턴이 적용된 서브 컴포넌트임이 더 명확하게 드러나기 때문입니다.

아래 링크에서 영감을 얻었어서 첨부할게요
https://fe-developers.kakaoent.com/2022/220731-composition-component/#3-3-%EB%A9%94%EC%9D%B8--%EC%84%9C%EB%B8%8C-%EC%BB%B4%ED%8F%AC%EB%84%8C%ED%8A%B8%EB%A5%BC-%EB%AC%B6%EC%96%B4%EC%84%9C-export

(그리고 사실 이건 정확히 기억은 안나는데 assign을 사용해야하는 또 다른 기술적 이유도 있었던 것 같아요. 찾으면 이건 다시 말씀드리겠습니다)

seonghunYang · 2024-02-17T08:01:28Z

app/ui/view/molecule/list/list-header.tsx

+  columns: string[];
+};
+
+export function ListHeader({ columns }: ListHeaderProps) {


[requset change]

named export를 사용하여 컴포넌트를 내보내는 것을 지향하는 이유가 있나요?

저는 React와 Next에서 기본적으로 생성하는 템플릿이 export default이기 때문에, 이를 표준으로 사용하는 것을 제안합니다.

또한, 추가적으로 컴파운드 패턴을 사용할 때는, 하위 컴포넌트에 named export를 사용하고, 컴파운드 패턴의 루트 컴포넌트에는 default export를 적용하면 컴포넌트를 분리하는 것이 더 명확해질 것이라고 생각합니다.

추가로, default export를 사용하면 파일이 하나의 책임만을 가지도록 어느 정도 강제할 수 있어 좋다고 생각합니다.

만약 컴포넌트 파일에서 여러 함수나 타입 컴포넌트를 export하고 있다면, 관심사의 분리가 잘 이루어지지 않았다고 판단될 수 있습니다. 따라서 이를 방지하는 데에도 유용하다고 생각합니다.

List의 경우에는 default export를 적용했을 때 아래 문제가 있는 걸 확인했습니다

List는 List header와 List row라는 컴포넌트를 가지고 있는데 export default만 해줄 경우 List component만 export를 해주기에 story book에서 오류가 발생합니다

그래서 List의 경우에는 default export를 사용하지 않았습니다

추가적으로 궁금한 게 있는데, compound pattern에 있어서 하위 컴포넌트라는 named export를 사용하는 게 더 명확해질거라고 말씀하셨는데 하위 컴포넌트라도 파일에서는 하나의 책임만 가지므로 default export 를 적용하는 게 더 좋지않을까요??

저도 이와 비슷한 문제를 이전에 경험한 적이 있었던 것 같네요. 제가 사용한 select 컴포넌트처럼, index 파일에서 assign으로 묶어서 export default를 사용해 보시는 건 어떨까요?

Compound 패턴에서 하위 컴포넌트에 named export를 사용하는 것을 제안하는 이유는, 독립적으로 사용되는 컴포넌트는 모두 default export를 하고, 하위 컴포넌트는 named export를 사용하게 되면, 다른 곳에서 import 할 때 이 컴포넌트가 독립된 컴포넌트인지, 아니면 하위 컴포넌트인지 명확하게 구분할 수 있기 때문입니다.

import ListHeader from '../../..'; // 하위 컴포넌트인지 알 수 없음 import { ListHeader } from '../../'; // ListHeader는 하위 컴포넌트임을 알 수 있음 import List from '../../'; // List는 독립적인 컴포넌트임을 알 수 있음

이와 동일한 이유로, 독립적인 컴포넌트는 default export로 통일하면 구분이 더욱 명확해집니다. 따라서 List 컴포넌트를 export default로 변경하는 것이 좋아 보입니다!

yougyung · 2024-02-18T05:06:35Z

app/ui/view/molecule/list/index.tsx

+import { ListHeader } from './list-header';
+import { ListRow } from './list-row';
+
+type ListProps = {


type 및 interface 중 적용하시는 기준이 있으신가요?

interface나 type 어떤 걸 사용하던 상관이 없을 때는 통일성을 위해 팀원들과 맞춰 사용하는 편입니다
해당 코드에서는 select component와 통일성을 위해서 type을 사용했습니다!

yougyung · 2024-02-18T07:48:28Z

list molecule component 구현
추가,삭제 버튼이 list row에 존재할 수 있도록 수정이 필요합니다!
이 부분은 기이수 과목 삭제 기능을 구현하면서 수정 할 예정입니다

버튼 기능은 순수하게 list가 하는 역할이 아닌데, 기존 코드에서는 역할 이상을 list component가 해주고 있다고 느꼈습니다. 리스트내 버튼 기능을 추가하기 위해서 또 다른 컴포넌트를 만들어주어야하지만, list component에 button이 없는 현재 상태도 충분하다고 생각합니다.

github-actions · 2024-02-18T08:06:53Z

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

gahyuun · 2024-02-18T08:28:54Z

list molecule component 구현
추가,삭제 버튼이 list row에 존재할 수 있도록 수정이 필요합니다!
이 부분은 기이수 과목 삭제 기능을 구현하면서 수정 할 예정입니다

버튼 기능은 순수하게 list가 하는 역할이 아닌데, 기존 코드에서는 역할 이상을 list component가 해주고 있다고 느꼈습니다. 리스트내 버튼 기능을 추가하기 위해서 또 다른 컴포넌트를 만들어주어야하지만, list component에 button이 없는 현재 상태도 충분하다고 생각합니다.

저는 아래와 같이 코드를 구현하려 했습니다

    const rowColumns = ['HEC01302', '분산프로그래밍', '3'];
    return (
      <div>
        <List>
          <List.Row columns={rowColumns} invalid={true} actionButton ={<Button></Button>} />
        </List>
      </div>
    );
    
export function ListRow({ columns, invalid = false, actionButton}: ListRowProps) {
  return (
    <div className="flex border-solid border-gray-300 border-b-[1px] items-center last:border-b-0">
      {columns.map((column, index) => (
        <div
          key={index}
          className={twMerge(
            'text-center my-4 font-medium text-lg text-[#2f2f2f]',
            columns.length === 5 ? 'w-[19%]' : 'w-[33%]',
            invalid ? 'text-red-500' : 'text-[#2f2f2f]',
          )}
        >
          {column}
        </div>
      ))}
      {actionButton}
    </div>
  );
}

위 방식은 button을 위한 component를 구현한 게 아니고 단지 list가 trigger 역할을 한다고 생각합니다!
혹시 이 방식이 list component가 역할 이상을 해준다고 생각하신 게 맞을까요? 맞다면 button을 위해 list를 어떤 방식으로 수정하는 걸 선호하시는 지 좀 더 말씀해주실 수 있나요?

github-actions · 2024-02-18T11:18:58Z

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

seonghunYang · 2024-02-18T13:33:07Z

유경 씨의 의견과 비슷한데, actionbutton뿐만 아니라 ListRow 컴포넌트에 invalid 속성이 있는 것이 조금 어색하게 느껴집니다. 두 속성 모두 List와 ListRow보다는 기이수 과목 리스트를 위한 속성이라서 그런 것 같습니다. 제 생각에는 다음 두 가지 방안이 있을 것 같습니다:

List, ListRow 컴포넌트를 도메인 컴포넌트로 변경 - invalid, actionButton 속성은 범용적인 list 컴포넌트보다는 기이수 과목 리스트에 해당하는 내용이므로, 이를 도메인 컴포넌트로 변경
컴포넌트 계층화 - List, ListRow를 재사용 및 확장 가능한 뷰 컴포넌트로 리팩토링하여 본래의 List, ListRow 기능에 집중하고, 이를 기반으로 기이수 과목 리스트 도메인 컴포넌트를 구현

세 분 모두 관련 사안에 대한 의견을 주셨으니 채팅으로 진행하는 것도 좋지만, 시간이 지체될 수 있으므로 보이스로 한 번 이야기해 보는 것은 어떨까요?

github-actions · 2024-02-20T11:06:44Z

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

github-actions · 2024-02-21T09:03:19Z

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

gahyuun · 2024-02-21T09:14:33Z

@yougyung @seonghunYang list component 리팩토링을 해서 push 했습니다! 구현하면서 아래와 같은 점들을 느꼈습니다

grid component 만큼 다른 컴포넌트를 추상화할 수 있을까? grid component의 역할이 컴포넌트로 분리할 정도의 역할일까? 라는 의문이 생겼습니다.
하지만 여기서 큰 문제가 될 것 같지 않고 grid component가 주는 이점도 있을 것 같아서 구현했습니다
render props pattern을 굳이 이용해야 하는 이유를 잘 모르겠어서 사용하지 않았습니다! children으로 사용해도 되는데 굳이 render 함수를 만들어야 하는 이유를 모르겠습니다 또한 List component에만 render props 패턴을 적용하기에 일관성이 떨어진다고 생각했습니다!

github-actions · 2024-02-21T09:42:30Z

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

github-actions · 2024-02-21T16:01:57Z

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

github-actions · 2024-02-22T03:01:14Z

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

seonghunYang

확인했습니다. 고생하셨어요!

gahyuun added 4 commits February 16, 2024 21:33

feat: list row component 구현

1c369a1

feat: List component 구현

32ca1d3

feat: list header component 구현

035d7a1

feat: list stories 구현

8cddc7d

gahyuun self-assigned this Feb 16, 2024

gahyuun requested review from yougyung and seonghunYang as code owners February 16, 2024 16:47

seonghunYang requested changes Feb 17, 2024

View reviewed changes

yougyung previously approved these changes Feb 18, 2024

View reviewed changes

chore: conflict 해결

6cea9c5

refactor: getChildren 함수 분리

91f3565

gahyuun dismissed yougyung’s stale review via 91f3565 February 18, 2024 08:04

gahyuun added 3 commits February 18, 2024 17:34

chore: named export -> default export

60b333c

chore: Button stories import error 수정

848c73f

fix: List default export error 해결

478fe8e

chore: merge branch 'main'

0a9128c

gahyuun added 3 commits February 21, 2024 17:58

feat: implement grid component

0e94161

refactor: improve list component

f33ae84

feat: implement Table component

96a91f4

chore: change color naming

b0be474

refactor: Refactor table component and add new props

14cf989

gahyuun added 3 commits February 22, 2024 11:48

chore: change Button variant name

7b104f1

chore: headerinfo -> headerInfo

c37259a

style: change list button color

4ab6c6c

seonghunYang approved these changes Feb 22, 2024

View reviewed changes

yougyung approved these changes Feb 22, 2024

View reviewed changes

gahyuun merged commit a97f6e6 into main Feb 22, 2024
3 checks passed

gahyuun deleted the list-molecule-component/#18 branch February 22, 2024 05:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement List molecule component/#18 #27

implement List molecule component/#18 #27

gahyuun commented Feb 16, 2024

github-actions bot commented Feb 16, 2024

seonghunYang Feb 17, 2024

gahyuun Feb 18, 2024

seonghunYang Feb 17, 2024 •

edited

Loading

seonghunYang Feb 17, 2024

seonghunYang Feb 17, 2024 •

edited

Loading

seonghunYang Feb 17, 2024 •

edited

Loading

seonghunYang Feb 17, 2024

gahyuun Feb 18, 2024 •

edited

Loading

seonghunYang Feb 18, 2024

seonghunYang Feb 18, 2024

yougyung Feb 18, 2024

gahyuun Feb 18, 2024

yougyung commented Feb 18, 2024

github-actions bot commented Feb 18, 2024

gahyuun commented Feb 18, 2024

github-actions bot commented Feb 18, 2024

seonghunYang commented Feb 18, 2024

github-actions bot commented Feb 20, 2024

github-actions bot commented Feb 21, 2024

gahyuun commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 22, 2024

seonghunYang left a comment

		List.Header = ListHeader;
		List.Row = ListRow;

implement List molecule component/#18 #27

implement List molecule component/#18 #27

Conversation

gahyuun commented Feb 16, 2024

📌 작업 내용

🤔 고민 했던 부분

🔊 도움이 필요한 부분

github-actions bot commented Feb 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seonghunYang Feb 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seonghunYang Feb 17, 2024 • edited Loading

Choose a reason for hiding this comment

seonghunYang Feb 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gahyuun Feb 18, 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

yougyung commented Feb 18, 2024

github-actions bot commented Feb 18, 2024

gahyuun commented Feb 18, 2024

github-actions bot commented Feb 18, 2024

seonghunYang commented Feb 18, 2024

github-actions bot commented Feb 20, 2024

github-actions bot commented Feb 21, 2024

gahyuun commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 22, 2024

seonghunYang left a comment

Choose a reason for hiding this comment

seonghunYang Feb 17, 2024 •

edited

Loading

seonghunYang Feb 17, 2024 •

edited

Loading

seonghunYang Feb 17, 2024 •

edited

Loading

gahyuun Feb 18, 2024 •

edited

Loading