-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Feature/BAR-149] 참고하는 페이지 UI 구성 #49
Conversation
f31a309
to
a5a59d0
Compare
현재 2가지 문제가 있습니다.
이 두 문제도 이어서 확인해보겠습니다. |
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.
굿굿 수고하셨습니다~
스크롤을 내린 상태에서 토글을 하는 경우 애니메이션이 깨진다는점
탭에 대한 내용인 것 같은데 position: fixed 때문에 그런 것 같네요... 저도 알아볼게요!
src/components/Card/style.css.ts
Outdated
{ | ||
padding: '28px 32px 20px', | ||
}, |
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.
@dmswl98
적용하지 않으면 제시된 패딩과 값이 다르지 않나요??
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.
추가적으로 footer에 적용된 margin-bottom: 8px
은 제거하는게 어떨까요? devtools 켜고 호버했을 때 영역이 깨지기도 하고, 굳이 음수값을 쓰지 않아도 괜찮을 것 같아서요!
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.
592ccaa
footer 문구 반영하였습니다!
}, | ||
]; | ||
|
||
const ReferTab = () => { |
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.
ReferTab
대신 ReferTabContent
또는 참고하는TabContent
로 수정하면 어떨까요?
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.
@dmswl98
좋네요!
cfb5c87
해당 커밋에서 반영해보았습니다. 폴더명도 한글을 쓰는게 훨씬 직관적일 것 같아서 반영해보았습니다.
@wonjin-dev
괜찮다면 이후 작업에서 domain/write
도 domain/끄적이는
으로 바꿔보면 어떨까요?
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.
전 좋아요~!
src/domain/Refer/models/index.ts
Outdated
export interface Refer { | ||
templateId: number; | ||
category: | ||
| 'ask' | ||
| 'report' | ||
| 'celebrate' | ||
| 'thank' | ||
| 'comfort' | ||
| 'regard' | ||
| 'etc'; | ||
subCategory: string; | ||
content: string; | ||
savedCount: number; | ||
copiedCount: number; | ||
} | ||
|
||
export enum Category { | ||
ask = '부탁하기', | ||
report = '보고하기', | ||
celebrate = '축하하기', | ||
thank = '감사 전하기', | ||
comfort = '위로하기', | ||
regard = '안부 전하기', | ||
etc = '기타', | ||
} |
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.
export interface Refer { | |
templateId: number; | |
category: | |
| 'ask' | |
| 'report' | |
| 'celebrate' | |
| 'thank' | |
| 'comfort' | |
| 'regard' | |
| 'etc'; | |
subCategory: string; | |
content: string; | |
savedCount: number; | |
copiedCount: number; | |
} | |
export enum Category { | |
ask = '부탁하기', | |
report = '보고하기', | |
celebrate = '축하하기', | |
thank = '감사 전하기', | |
comfort = '위로하기', | |
regard = '안부 전하기', | |
etc = '기타', | |
} | |
export interface Refer { | |
templateId: number; | |
category: keyof typeof CATEGORY; | |
subCategory: string; | |
content: string; | |
savedCount: number; | |
copiedCount: number; | |
} | |
const CATEGORY = { | |
ask: '부탁하기', | |
report: '보고하기', | |
celebrate: '축하하기', | |
thank: '감사 전하기', | |
comfort: '위로하기', | |
regard: '안부 전하기', | |
etc: '기타', | |
} as const; |
이렇게 작성하면 중복 코드가 줄어들 것 같아요~
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.
const categoryNameKr = Category[category]; | ||
|
||
return ( | ||
<Card type="refer"> |
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.
type 대신 className을 받아서 도메인과 관련된 내용들의 스타일은 외부에서 주입하는건 어떨까요?
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.
className을 외부 주입 하는 형태로 반영하였습니다!
data: Refer; | ||
} | ||
|
||
const ReferCard = ({ data }: ReferCardProps) => { |
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.
TemplateCard or ReferTemplateCard or 참고하는TemplateCard는 어떨까요?
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.
src/domain/Refer/utils/index.ts
Outdated
export const getNumToK = (num: number) => { | ||
if (num < 1000) return num; | ||
return `${num / 1000}k`; | ||
}; |
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.
const formatter = new Intl.NumberFormat('en', { notation: 'compact' })
const view1 = 1200;
formatter.format(view1); // '1.2K'
const view2 = 1200000;
formatter.format(view); // '1.2M'
Intl.NumberFormat
을 사용하면 후에 여러 기준(1000, ...)에 따라 자동으로 포맷팅이 되서 대신 사용해보면 좋을 것 같아요.
export const formatNumber = (number: number) => {
const formatter = new Intl.NumberFormat('en', { notation: 'compact' })
return formatter.format(number);
}
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.
const ReferCard = ({ data }: ReferCardProps) => { | ||
const { category, subCategory, content, copiedCount, savedCount } = data; | ||
|
||
const categoryNameKr = Category[category]; |
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.
CATEGORY_COLOR
의 키값이 한국어로 되어 있어서 그렇군요!
아래 값 참고해서 CATEGORY_COLOR
의 키값을 영어로 맞추면 좋을 것 같네요
const CATEGORY = {
ask: '부탁하기',
report: '보고하기',
celebrate: '축하하기',
thank: '감사 전하기',
comfort: '위로하기',
regard: '안부 전하기',
etc: '기타',
} as const;
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.
@dmswl98
어짜피 한국어로의 전환이 한번 필요해서 현재 형태로 유지하는게 어떨까요?
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.
수고하셨습니다~~!
스크롤을 내린 상태에서 토글을 하는 경우 애니메이션이 깨진다는점
이 부분은 나중에 고쳐보도록 합시다!
891d6ce
to
4556cc6
Compare
Summary
To Reviewers
aea4206
hover시에 svg 아이콘 색상을 바꾸고 싶은데... className을 전달하기 않고 처리하는 좋은 방법이 있을까요?
How To Test