Feat(cds): tag 공통 컴포넌트 리디자인 적용 - #256
Conversation
🎨 Storybook 배포 완료PR 작성자: @twossu |
jogpfls
left a comment
There was a problem hiding this comment.
tag 컴포넌트 수고 많으셨어요,, 서버분들과 소통 많이 하시느라 힘드셨을텐데,, 이번에 데이터 내려오는 형식 또 바뀌니까,,, 화이팅,,,,,,,,,,,,,,,,,,,,,,,,,
코멘트 남겼으니까 확인부탁드려요 !
| aria-hidden="true" | ||
| /> | ||
| <p>{text}</p> | ||
| {!isAi && onRemove && ( |
There was a problem hiding this comment.
ai결과물일 때에는 remove를 사용하지 못하도록 한 것 같아요
여기서 제한하기보다는 타입 쪽에서 먼저 제한하는 방향은 어떨까요 ?
There was a problem hiding this comment.
타입쪽에서 막는 방향을 어떻게 설계해야 할지 감이 잘 안 잡히옵니다....
| {/* TODO: Label 리디자인 반영 후 수정 (현재는 임시 Label로 구현)*/} | ||
| {tagList.length > 0 ? ( | ||
| tagList.map((tag) => ( | ||
| <Label | ||
| key={tag.tagId} | ||
| labelSize="sm" | ||
| labelColor="blue" | ||
| labelText={tag.name ?? ''} | ||
| /> | ||
| )) | ||
| ) : ( | ||
| <Label labelSize="sm" labelColor="grey" labelText="라벨없음" /> | ||
| )} | ||
| {isAiGenerated && <Tag size="lg" variant="ai" text="AI 결과물" />} | ||
| {tagList.map((tag) => ( | ||
| <Tag | ||
| key={tag.tagId} | ||
| size="lg" | ||
| backgroundColor={tag.backgroundColorHex ?? ''} | ||
| textColor={tag.textColorHex ?? ''} | ||
| text={tag.name ?? ''} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
Label-select도 tag-select로 네이밍 변경하면서 + 코드 삭제된 게 많아서 그냥 새로 구현된 것처럼 깃허브에 떠요. 그냥 처분해도 괜찮을까요?…… 어차피 제가 이 컴포넌트 구현 담당이어서 새로운 시작도 하고 싶은 겸……
이게 뭔가 했네요. 네. 처분하세요.
There was a problem hiding this comment.
전반적으로 이 파일에서는 ai 관련된 도메인은 모두 삭제하는게 좋을 것 같아요 ! css에서는 ai의 색상때문에 ai가 들어가도 상관없을 것 같지만(도메인 개념보다는 ai의 스타일 자체를 의미한다면) 여기 파일에서는 없앨 수 있을 것 같아요 !
There was a problem hiding this comment.
반영했습니다! 디자인 시스템은 도메인 지식이 있으면 안 된다는 것을 다시 되짚어보고 깨달아갑니다!
근데 이렇게 되니 ai 태그일 때 removable을 막을 수 있는 방법이 헤린님이 제안해주신 타입으로 분리하는 것뿐이더라고요..... 근데 분리해보려는데 도저히 감이 안잡히오네요....
그래서 cds-ui에서는 제한 안 하고 ai 태그엔 onRemove를 안 넘긴다는 걸 호출부 책임으로 남겨두는 게 낫지않을까 싶습니다...
너무 어려워요!!!!!!1
There was a problem hiding this comment.
디자인 시스템은 컴포넌트의 모양, 피그마 프로퍼티를 보면서 인터페이스를 설계하면 좋을 것 같아요 !! 그렇게 그렇게 되면 도메인이 들어가지 않을 수 있을 것 같아요 !!
그래서 cds-ui에서는 제한 안 하고 ai 태그엔 onRemove를 안 넘긴다는 걸 호출부 책임으로 남겨두는 게 낫지않을까 싶습니다...
아쉽긴 하지만,, 나중에 윤하님께서 리팩토링 진행해보셔도 좋을 것 같아요 !!
그리고 저희 디자인 시스템이 실제 배포된 것도 아니고 배포할 예정도 아직은 없기 때문에 문제가 되는 건 아니니까 천천히 공부해가면서 구현해도 좋을 것 같아요 !! 너무너무 잘하시고 계십니다 !!!
여기다가 작성하긴 길어서 제가 개인 메시지로 방향성 보냈는데 그것도 확인 해주시면 감사할 것 같아요 !!
There was a problem hiding this comment.
이런 새로운 방식을 저에게 하사해주셔서 감사합니다. 히하 헤린님의 메시지를 받고 처음엔 와닿지 않았었는데 그것에 대해 조금 파먹기를 해보니깐 이해가 되고 흐흐 조으네요 새로운걸 얻고가서 기분이 갑자기 좋아졌어요
근데 고민이 타입 정의 줄이 길어지다 보니깐 이것을 파일분리해야할지 참말로 고민이네요.
- 분리 안 하면: 타입 블록이 길어서 파일의 목적을 한눈에 파악하기 어려울 것만 같음
- 분리하면: 전체 코드량 자체는 많지 않아서 분리 기준이 애매하고 로직과 타입을 오가며 봐야 하는 불편함이 생길 것 같음
이런 경우 보통 어떤 기준으로 나누는게 좋을지 궁금하옵니다! 폴더구조는 매번 자꾸 헷갈리고 저를 괴롭히네요 ㅎㅎ
refactor: 태그 컴포넌트 fill/outlined variant 기반으로 재설계
refactor: 태그 타입에서 중복된 TagBaseProps 교차를 상위로 통합
| card: CardInfoType; | ||
| isSelected?: boolean; | ||
| isDragging?: boolean; | ||
| isNewAi?: boolean; |
There was a problem hiding this comment.
헉 이거는 isNew일 때 스타일이 따로 있어요 !!
isNew랑 isAiGenerated랑 따로 봐야해요 !!
제가 위치를 잘못 놓은 것도 있어서 혹시 type CardInfoType에
isNew: boolean으로 추가해주실 수 있나요 ??! (그리고 네이밍도 isNewAi -> isNew로 해주시면 감사하겟습니당,,,,,,ㅎㅎㅎㅎㅎ)
There was a problem hiding this comment.
카드 컴포넌트 헷갈렸는데 딱 정리해주셔서 아하! 완벽이해 완료했듭니당
반영했사옵니다~ 훨훨
| className={styles.cardContainer({ | ||
| isSelected, | ||
| isDragging, | ||
| isAiGenerated, | ||
| })} |
There was a problem hiding this comment.
| className={styles.cardContainer({ | |
| isSelected, | |
| isDragging, | |
| isAiGenerated, | |
| })} | |
| className={styles.cardContainer({ | |
| isSelected, | |
| isDragging, | |
| isNew, | |
| })} |
jogpfls
left a comment
There was a problem hiding this comment.
수고하셨어용 !! 코멘트 궁금한거 하나만 확인해주세요 !!!
굳!!!!!!!!!!!!!!!!!!
| const onRemove = rest.action === 'remove' ? rest.onRemove : undefined; | ||
| const removable = !!onRemove; |
There was a problem hiding this comment.
원래 onRemove는 삭제 함수,
removable은 그 함수의 존재 여부를 스타일에 boolean으로 넘기기 위해 따로 뽑아둔 값이었습니다.
그런데 헤린님의 리뷰로 다시 관찰해보니 굳이 복잡하게 빼지않고 인라인으로 사용하는 것이 더 좋을 것 같아 수정했습니다!
fix: 태그 컴포넌트의 불필요한 removable 상태 제거
++ onRemove도 조건부 렌더링과 클릭 핸들러 호출에서만 쓰이고 rest.onRemove로 직접 읽어도 충분히 명확해서 별도 상수로 뺄 필요가 없다고 생각해 인라인으로 변경했습니다!
refactor: onRemove 핸들러를 rest.onRemove로 직접 참조하도록 변경
Tag 컴포넌트로 완전히 대체되어 더 이상 쓰이지 않는 Label, LabelList 컴포넌트와 label-color-map 상수를 제거합니다.
하드코딩된 5개 라벨 대신 useGetTag()로 가져온 실제 태그 목록을 드롭다운에 표시하고, 선택된 태그를 colorHex 기반 Tag 컴포넌트로 렌더링하도록 수정합니다. 메모 저장 요청도 실제 태그명을 tagNames로 전달하도록 변경했습니다.
탭 색상이 더 이상 태그를 따라가지 않도록 관련 로직을 제거하고, 곧 삭제될 디테일모달에서도 태그 렌더링을 걷어냅니다.
71e0c7c to
1f8e202
Compare
jm8468
left a comment
There was a problem hiding this comment.
바로 머지 가시져!
이 PR이 머지되어야 제 작업이 가능해서요👍
굿굿굿



📌 Summary
기존
Label컴포넌트는졸업 프로젝트,교양,SOPT,레퍼런스같이 종류가 고정되어 있어 사용자가 그중에서 선택하는 방식이었습니다. 그런데 이번 스프린트에서 백엔드가 태그마다 색상 이름(color, 예:pink/red/orange등)을 내려주는 것으로 바뀌면서, 프론트는 고정 팔레트 없이 서버가 준 색상 이름을TAG_COLOR_MATCH로 매칭해 실제 배경색/글자색 hex 값을 계산해서 사용하도록 변경했습니다. 이에 맞춰 앱 전역에 남아있던Label시스템(LabelTextType/LabelColorType,LabelSelect등)도 걷어내고 실제 백엔드 태그(tagId,name,color)와 "태그"라는 네이밍으로 통일했습니다.📚 Tasks
Tag공통 컴포넌트:colorprop(고정 enum)을 없애고backgroundColor/textColor두 개의 hex 값을 받아 그대로 렌더링하도록 변경card.tsx: 서버가 내려주는 태그 색상 이름(color)을TAG_COLOR_MATCH로 매칭해 계산한backgroundColor/textColorhex 값을 사용card.tsx: 카드 그라데이션 색상 기준을isNew→isAiGenerated로 변경🔍 Describe
Label → Tag 전면 전환
packages/cds-ui의Label,LabelList컴포넌트 및label-color-map.ts삭제label-select.tsx: 하드코딩된 5개 라벨 대신useGetTag()로 가져온 실제 태그 목록을 드롭다운에 표시하고 선택된 태그는 서버가 내려주는 색상 이름(color)을TAG_COLOR_MATCH로 매칭한Tag로 렌더링detail-modal.tsx: 곧 삭제 예정인 컴포넌트라 태그 렌더링만 걷어내고 나머지는 유지tag/tag.tsx, tag/tag.css.ts
colorprop 하나로 배경색을 받던 방식 →backgroundColor/textColor두 개의 prop으로 분리해 전달받은 hex 값을 각각 배경색/글자색·인디케이터 색상에 인라인style로 적용 (이 hex 값은card.tsx에서 서버가 내려주는 색상 이름(color)을TAG_COLOR_MATCH로 매칭한 결과)onRemoveprop 추가 → 삭제 버튼(ic_delete아이콘) 조건부 렌더링 지원removablevariant 추가 →onRemove가 전달된(=삭제 가능한) 태그에만1px solid grey300보더를 표시해 일반 태그와 시각적으로 구분삭제 버튼에서
stopPropagation을 호출하는 이유:Tag는 나중에? 생길 tag-popover컴포넌트 안에 중첩되어 렌더링됩니다. 이게 없으면 삭제 버튼 클릭이 상위 트리거로 버블링돼서 "태그 삭제"와 "드롭다운 토글"이 동시에 발생해요. 부모는onRemove: () => void콜백만 받고 버튼 DOM에 직접 접근할 수 없기 때문에 이벤트를 끊는 책임을Tag내부에 선언했습니다.variant="ai"일 때 삭제 버튼을 막은 이유: AI 결과물 태그는 시스템이 붙이는 라벨이라 사용자가 지울 수 없어야해서 막앗습니다.AI 결과물 태그/카드 스타일
isAiGeneratedboolean 하나만 내려주고 색상/보더 같은 프레젠테이션 정보는 태그 데이터(tagList)에 얹지 않기로 했습니다. 그 이유는AI 결과물 태그는 항상 고정된 색 하나만 쓰기 때문에 이걸 서버 스키마에 태우면 서버가 클라이언트 디자인까지 알아야 하는 불필요한 커플링이 생긴다는 게 이유였습니다. 그래서 서버는 "AI 생성 여부"라는 의미만 책임지고 색상은 프론트가 소유하는 쪽으로 정리했습니다!card.tsx: 기존에isNewAi(=memo.isNew기준으로 그라데이션 보더를 주던 카드)를isAiGenerated(=memo.isAiGenerated기준)로 교체하고 카드 상단에Tag size="lg" variant="ai" text="AI 결과물"을 렌더링하도록 수정태그 타입 재설계 (fill/outlined variant 기반)
기존엔
TagProps가{ size, color, text, onRemove }형태의 단일 인터페이스였고 컴포넌트 내부에서color === 'ai'를 런타임에 체크해서 AI 태그일 때 색상/삭제 버튼 로직을 분기했습니다. 리뷰에서 지적된 대로 이 방식은 cds-ui가 "ai"라는 도메인 개념을 알고 있어야 동작한다는 문제가 있었습니다.다만 도메인 체크를 걷어내더라도 "outlined 스타일 태그는 색상을 가질 수 없고 삭제 액션도 가질 수 없다"는 UI 제약 자체는 지켜야 했습니다. 그래서 런타임 체크 대신 variant(
fill|outlined), size(sm|lg), action(remove 유무) 조합별로 허용되는 prop만 남도록 discriminated union으로 타입을 재설계했습니다.SmallFillTagProps/LargeFillTagProps/OutlinedTagProps세 갈래로 분리OutlinedTagProps는color?: never,onRemove?: never로 타입 레벨에서 차단action: 'remove'일 때만onRemove를 요구하도록NoneActionProps | RemoveActionProps로 분리TagBaseProps를 유니온 전체를 감싸는 형태(TagBaseProps & (A | B | C))로 통합해 중복 제거결과적으로 cds-ui는 "ai"를 몰라도 되고 "AI 결과물 태그"라는 의미 부여는 사용처(client)가
variant="outlined"를 선택하는 것만으로 이루어집니다. outlined 태그에color나onRemove를 넘기려 하면 컴파일 타임에 에러가 나서 기존 런타임 체크가 하던 방어를 타입으로 대체합니다.👀 To Reviewer
LabelTextType/LabelColorType(label-type.ts,label-match.ts)에 의존하고 있고 태그 색상도 예전 고정 팔레트 방식 그대로입니다. 이 두 파일은 트리뷰가 참조하고 있어 완전히 삭제하지 못하고 남겨뒀고, Refactor(client): 트리뷰 파일 구조 리팩토링 #257 머지 이후에 트리뷰의 Label → Tag 전환을 추가로 이곳에서 진행하고 머지하겠습니다!!Caution
Label-select도 tag-select로 네이밍 변경하면서 + 코드 삭제된 게 많아서 그냥 새로 구현된 것처럼 깃허브에 떠요. 그냥 처분해도 괜찮을까요?…… 어차피 제가 이 컴포넌트 구현 담당이어서 새로운 시작도 하고 싶은 겸……
📸 Screenshot