feat: 카테고리 리스트 컴포넌트 추가 #21
Conversation
|
|
||
| interface CategoryType { | ||
| type: 'row' | 'column'; | ||
| size: 'small' | 'medium'; |
There was a problem hiding this comment.
다른 쪽 컴포넌트는 sm, md 단위 사이즈로 작성해주셨는데 컨벤션 통일 해주면 좋겠네요 ㅎㅎ
There was a problem hiding this comment.
sm,md 단위 사이즈로 컨벤션을 통일을 위해 다시 변경 하였습니다.
| size: 'small' | 'medium'; | ||
| } | ||
|
|
||
| const CategoryContainer = styled.ul<CategoryType>` |
There was a problem hiding this comment.
css in js 코드도 상단 또는 하단 한쪽에 작성해주면 좋을 것 같습니다. 최근 오픈소스들의 추세를 보면 하단에 작성하긴 하더라구요 ㅎㅎ
There was a problem hiding this comment.
전체 머지가 된 후 광민님과 커뮤니케이션을 통해 수정 해보겠습니다 !
| opacity: 0.7; | ||
| transition: all 350ms cubic-bezier(0.87, 1, 0.07, 1); | ||
|
|
||
| input:checked + & { |
There was a problem hiding this comment.
css in js 는 보통 셀렉터를 지양해서 작성해야합니다. 컴포넌트보다 셀렉터의 돔 렌더링비용이 비싸기 떄문입니다. 참고부탁드릴게용
| font-size: ${({theme}) => theme.typography.fontSizes.size300}; | ||
| font-weight: ${({theme}) => theme.typography.fontWeights.semiBold}; | ||
| text-transform: capitalize; | ||
| letter-spacing: ${({theme}) => theme.typography.letterSpacings.spacing300}; |
There was a problem hiding this comment.
font-size, font-weight, letter-spacing 를 하나로 묶어서 타이포그라피 컴포넌트를 만들어보면 좋을 것 같습니다.
| const [checkedItems, setCheckedItems] = useState<{ [category: string]: boolean }>({}); | ||
| const categoryList = ['Beach', 'Camping', 'Fishing', 'Forest', 'Mountain', 'Ocean']; | ||
|
|
||
| const handleCheckboxChange = (category) => { |
There was a problem hiding this comment.
category 카테고리 타입추론 문제 있어 보이는데 확인 부탁드릴게요~
There was a problem hiding this comment.
깃허브액션으로 eslint 검사하는 CI 하나 만들어보면 좋겠네요. 지금 처럼 실수로 올라간 PR 이 프로덕션에 반영 되는 허들을 하나 추가해서 코드를 보호하면 좋을 것 같습니다.
| width: 315px; | ||
| `; | ||
|
|
||
| const CheckboxWrapper = styled.label` |
There was a problem hiding this comment.
� CheckboxWrapper 이름에서 label 태그를 추론하기 어려워 보입니다. 네이밍 한번 검토 부탁드려요~
| <CategoryContainer type={type}> | ||
| {categoryList.map((category) => ( | ||
| <li key={category}> | ||
| <CheckboxWrapper> |
| border: 0; | ||
| `; | ||
|
|
||
| const ToggleButton = styled.span<CategoryType>` |
| width: ${props => (props.size === 'small' ? '44px' : '70px')}; | ||
| height: ${props => (props.size === 'small' ? '44px' : '70px')}; | ||
| padding: 10px; | ||
| background: #fafafa; |
There was a problem hiding this comment.
| background: #fafafa; | |
| background-color: #fafafa; |
| height={iconSize} | ||
| quality={100} | ||
| placeholder="blur" | ||
| onError={(e) => console.log(e.target.name)} |
There was a problem hiding this comment.
아래 처럼 에러를 바로 던저 개발중에 개발자가 에러를 바로 인지할 수 있도록 하면 좋을 것 같습니다.
| onError={(e) => console.log(e.target.name)} | |
| onError={(e) => throw new Error("~~~")} |
close #20