-
Notifications
You must be signed in to change notification settings - Fork 2
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: 카테고리 리스트 컴포넌트 추가 #21
base: main
Are you sure you want to change the base?
Changes from 5 commits
966f5ba
f144300
6f58330
68c7115
58d8039
1a31494
2159597
c0462f5
44b932f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
import React, {useState} from 'react'; | ||
import styled from "@emotion/styled"; | ||
import CategoryIcon from "@/ui-library/components/iconCategory/categoryIcon"; | ||
|
||
interface CategoryType { | ||
type: 'row' | 'column'; | ||
size: 'small' | 'medium'; | ||
} | ||
|
||
const CategoryContainer = styled.ul<CategoryType>` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. css in js 코드도 상단 또는 하단 한쪽에 작성해주면 좋을 것 같습니다. 최근 오픈소스들의 추세를 보면 하단에 작성하긴 하더라구요 ㅎㅎ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 전체 머지가 된 후 광민님과 커뮤니케이션을 통해 수정 해보겠습니다 ! |
||
display: grid; | ||
grid-template-columns: ${props => (props.type === 'row' ? 'repeat(6, 1fr)' : 'repeat(2, 1fr)')}; | ||
grid-template-rows: ${props => (props.type === 'row' ? 'repeat(1, 54px)' : 'repeat(3, 143px)')}; | ||
column-gap: ${props => (props.type === 'row' ? '10px' : '35px')}; | ||
row-gap: ${props => (props.type === 'row' ? 'unset' : '30px')}; | ||
width: 315px; | ||
`; | ||
|
||
const CheckboxWrapper = styled.label` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. � CheckboxWrapper 이름에서 label 태그를 추론하기 어려워 보입니다. 네이밍 한번 검토 부탁드려요~ |
||
display: flex; | ||
align-items: center; | ||
height: 100%; | ||
`; | ||
|
||
const HiddenCheckbox = styled.input` | ||
position: absolute; | ||
overflow: hidden; | ||
clip: rect(0 0 0 0); | ||
width: 1px; | ||
height: 1px; | ||
padding: 0; | ||
margin: -1px; | ||
border: 0; | ||
`; | ||
|
||
const ToggleButton = styled.span<CategoryType>` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 버튼은 button 태그 이용해주세요~ |
||
display: flex; | ||
flex-direction: ${props => (props.type === 'row' ? 'row' : 'column')}; | ||
justify-content: center; | ||
align-items: center; | ||
gap: 10px; | ||
width: 100%; | ||
height: 100%; | ||
color: ${({theme}) => theme.color.black.black900}; | ||
padding: ${props => (props.type === 'row' ? '5px 20px 5px 5px' : 'unset')}; | ||
border: 2px solid ${({theme}) => theme.color.black.black100}; | ||
border-radius: 20px; | ||
cursor: pointer; | ||
opacity: 0.7; | ||
transition: all 350ms cubic-bezier(0.87, 1, 0.07, 1); | ||
|
||
input:checked + & { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. css in js 는 보통 셀렉터를 지양해서 작성해야합니다. 컴포넌트보다 셀렉터의 돔 렌더링비용이 비싸기 떄문입니다. 참고부탁드릴게용 |
||
background-color: ${({theme}) => theme.color.black.black0}; | ||
border-color: ${({theme}) => theme.color.success.success600}; | ||
opacity: 1; | ||
} | ||
`; | ||
|
||
const CheckBoxTitle = styled.span` | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. font-size, font-weight, letter-spacing 를 하나로 묶어서 타이포그라피 컴포넌트를 만들어보면 좋을 것 같습니다. |
||
`; | ||
|
||
const CategoryList = ({type, size}: CategoryType) => { | ||
const [checkedItems, setCheckedItems] = useState<{ [category: string]: boolean }>({}); | ||
const categoryList = ['Beach', 'Camping', 'Fishing', 'Forest', 'Mountain', 'Ocean']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 특정 버티컬에 종속적인 값은 외부에서 넘겨 받아 컴포넌트를 순수하게 관리해 재사용성을 높여주면 좋을 것 같습니다 |
||
|
||
const handleCheckboxChange = (category) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. category 카테고리 타입추론 문제 있어 보이는데 확인 부탁드릴게요~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 깃허브액션으로 eslint 검사하는 CI 하나 만들어보면 좋겠네요. 지금 처럼 실수로 올라간 PR 이 프로덕션에 반영 되는 허들을 하나 추가해서 코드를 보호하면 좋을 것 같습니다. |
||
setCheckedItems((prevCheckedItems) => ({ | ||
...prevCheckedItems, | ||
[category]: !prevCheckedItems[category] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이벤트타입이 키 값으로 사용 되는거 맞을까요? |
||
})); | ||
}; | ||
|
||
return ( | ||
<> | ||
<CategoryContainer type={type}> | ||
{categoryList.map((category) => ( | ||
<li key={category}> | ||
<CheckboxWrapper> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. htmlfor 어트리뷰트 추가 부탁드릴게용~ |
||
<HiddenCheckbox | ||
type="checkbox" | ||
checked={checkedItems[category]} | ||
onChange={handleCheckboxChange} | ||
/> | ||
<ToggleButton type={type}> | ||
<CategoryIcon name={category} size={size}/> | ||
<CheckBoxTitle>{category}</CheckBoxTitle> | ||
</ToggleButton> | ||
</CheckboxWrapper> | ||
</li> | ||
))} | ||
</CategoryContainer> | ||
</> | ||
); | ||
}; | ||
|
||
export default CategoryList; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,56 @@ | ||||||
import Image from "next/image"; | ||||||
import * as category from "./categoryImage"; | ||||||
import styled from "@emotion/styled"; | ||||||
|
||||||
interface CategoryIconProps { | ||||||
name: keyof typeof category; | ||||||
size: 'small' | 'medium'; | ||||||
} | ||||||
|
||||||
const IconBackground = styled.div<CategoryIconProps>` | ||||||
display: flex; | ||||||
justify-content: center; | ||||||
align-items: center; | ||||||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
border-radius: ${props => (props.size === 'small' ? '15px' : '25px')}; | ||||||
`; | ||||||
|
||||||
const StyledImage = styled(Image)` | ||||||
filter: drop-shadow(0px 4px 10px rgba(59, 125, 21, 0.3)); | ||||||
`; | ||||||
|
||||||
const sizeMap: { [key in CategoryIconProps['size']]: number } = { | ||||||
small: 24, | ||||||
medium: 40 | ||||||
}; | ||||||
|
||||||
const CategoryIcon = ({name, size = 'small'}: CategoryIconProps) => { | ||||||
if (!name in category) { | ||||||
throw new Error(`이름이 잘못 되었습니다.`); | ||||||
} | ||||||
|
||||||
if (size !== 'small' && size !== 'medium') { | ||||||
throw new Error('입력하신 사이즈가 없습니다.'); | ||||||
} | ||||||
|
||||||
const iconSize = sizeMap[size] || sizeMap['small']; | ||||||
|
||||||
return ( | ||||||
<IconBackground size={size}> | ||||||
<StyledImage | ||||||
src={category[name]} | ||||||
alt={name} | ||||||
width={iconSize} | ||||||
height={iconSize} | ||||||
quality={100} | ||||||
placeholder="blur" | ||||||
onError={(e) => console.log(e.target.name)} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아래 처럼 에러를 바로 던저 개발중에 개발자가 에러를 바로 인지할 수 있도록 하면 좋을 것 같습니다.
Suggested change
|
||||||
/> | ||||||
</IconBackground> | ||||||
); | ||||||
}; | ||||||
|
||||||
export default CategoryIcon; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
export {default as Beach} from '@/public/images/categorys/beach.png'; | ||
export {default as Mountain} from '@/public/images/categorys/mountain.png'; | ||
export {default as Forest} from '@/public/images/categorys/forest.png'; | ||
export {default as Ocean} from '@/public/images/categorys/ocean.png'; | ||
export {default as Camping} from '@/public/images/categorys/camping.png'; | ||
export {default as Fishing} from '@/public/images/categorys/fishing.png'; |
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.
다른 쪽 컴포넌트는 sm, md 단위 사이즈로 작성해주셨는데 컨벤션 통일 해주면 좋겠네요 ㅎㅎ
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.
sm,md 단위 사이즈로 컨벤션을 통일을 위해 다시 변경 하였습니다.