-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Handy Semantic 토큰 추가 #132
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
figma에 있는 모든 semantic 토큰을 정의하였습니다. (chip, pagination, checkbox, button 전부 포함)
아싸 잘 갖다 쓰겠습니다
따라서 src/utils/variant.ts 에 있는 MergeVariants 타입을 만들어 토큰을 구성하기 그나마 쉽게 만들었습니다.
👍(⊙o⊙)👍
템플릿 리터럴 타입 활용의 완전 좋은 예라고 생각해요!!!
특히 InteractiveVariant
가 없었다면 중복이 심해졌을 수 있을 거 같은데 아주 기강을 잡아두셨군요
타입스크립트 이제 좀 안다 생각했는데,,, 아직 멀었군요 많이 배워갑니당
준이 작업하던 브랜치(feat/#124-handy-semantic-token
)가 남아있는데
이 브랜치는 리뷰 제출하는 대로 제가 멸종시켜버리겠습니다
고생하셨어요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
채널 확인을 늦게 했는데 Spcing(Radius)
로 되어있던 이름이 Number
로 바뀌었네요.....🤔
저는 지금처럼 Spacing
들어간 게 의도를 드러내기 좋다고 생각하는데
피그마 이름과 달라지는 부분이라 조심스럽네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 약간 바뀐게 좋다고 생각하는게
padding
, margin
만 쓰이는거면 spacing
이 맥락상 맞아보이는데
그 값들과 연계되는 border-radius
까지 설정되어 버리니까 spacing
보다는 number
또는 sizing
이 더 좋아보이긴하네요
- 이 부분도 여기서 수정해서 커밋 넣겠습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿 같이 수정하겠습니다
Spacing -> number / radius 로 네이밍 수정된 것을 반영했습니다. 따라서 export type YDSTheme = {
primitive: {
color: PrimitiveColorPalette;
number: PrimitiveNumber;
};
semantic: {
color: SemanticColorPalette;
radius: SemanticRadius;
};
typo: Typos;
}; semantic/radius 같은 경우는,
만약
등과 같은 값이 추가로 생긴다면, 동일한 방법으로
경로에 구현을 진행해주시면 됩니다.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🤸♀️👍
1️⃣ 어떤 작업을 했나요? (Summary)
기존 코드에 영향을 미치지 않는 변경사항
기존 코드에 영향을 미치는 변경사항
@2wndrhs 이 구현했던 primitive color의 색상 타입을
MergeVariants
타입을 이용하여 리팩토링을 진행했습니다.2️⃣ 알아두시면 좋아요!
1. 토큰 추가하기 관련
figma의 Token Handoff에 보면 정말 많은 토큰들이 있습니다.
수작업으로 모든 토큰을 작성하기에는 귀찮고, 가독성도 안좋고, 휴먼 에러가 발생할 가능성이 높습니다.
따라서
src/utils/variant.ts
에 있는MergeVariants
타입을 만들어 토큰을 구성하기 그나마 쉽게 만들었습니다.기존 (하드코딩):
MergeVariants
사용 시:이때, 생성된 결과는 동일합니다.
2. Color 문서에 색상 팔레트 추가하기
완전 자동화는 아닙니다.
src/style/foundation/color/color.stories.tsx
파일의AllThemeColors
컴포넌트 속semanticColorCategories
변수에 색상 값에 대한 정보를 입력해야 합니다.3. Spacing 문서에 값 추가하기
이 또한 완전 자동화는 아닙니다.
src/style/foundation/spacing/spacing.stories.tsx
파일의AllBorderSpacings
컴포넌트 속에StyledRow
를 추가하여 예시를 작성해야합니다.3️⃣ 추후 작업
논의할거 1개
현재 해당 토큰에 대한 접근이
theme.semantic.spacing.spacingRadiusXL
처럼 해야되는데,spacing이 두번 들어가는게 굳이굳이 싶습니다
theme.semantic.spacing.radiusXL
이런 방식이어도 괜찮지 않을까요?4️⃣ 체크리스트 (Checklist)
main
브랜치의 최신 코드를pull
받았나요?