[feature/#1/header] - 헤더 컴포넌트 구현 #18
Conversation
src/components/Header/Header.tsx
Outdated
| <Center | ||
| w="100%" | ||
| position="fixed" | ||
| bg="#fff" |
There was a problem hiding this comment.
white 색상 사용하실거면 bg='white'(or theme.ts에 지정해놓은 색상 변수명)으로 설정하시면 나중에 다크모드 할때 바꾸기 편할것 같습니다
There was a problem hiding this comment.
아 다크모드를 생각해야겠군요!
최대한 theme.ts에 있는 변수를 사용하겠습니다~
|
|
||
| const NotificationPopover = ({ | ||
| NotificationIconButton, | ||
| }: INotificationPopover) => { |
There was a problem hiding this comment.
컴포넌트 props 지정할때 컨벤션처럼 NotificationPopover+ Props 이런식으로 수정해주시면 감사하겠습니다!
There was a problem hiding this comment.
넵... 제가 I랑 T 넣는다고 착각하고 있었는데 그걸 수정 안하고 바로 올려버렸네요;;
수정하겠습니다!
| <Box | ||
| pl="1.5rem" | ||
| alignSelf="end" | ||
| pb="2rem"> | ||
| <InputGroup> | ||
| <InputRightElement> | ||
| <Icon | ||
| as={IoSearch} | ||
| w="2rem" | ||
| h="2rem" | ||
| /> | ||
| </InputRightElement> | ||
| <Input | ||
| size="lg" | ||
| variant="flushed" | ||
| placeholder="검색어를 입력하세요" | ||
| /> | ||
| </InputGroup> | ||
| </Box> |
There was a problem hiding this comment.
이런건 하나의 컴포넌트로 묶어도 될 것 같은데, 어떻게 생각하시나요..?
There was a problem hiding this comment.
아 이건 일단 나중에 검색창 컴포넌트로 대체하려고 대충 만든거긴한데
헤더의 검색창이 뭔가 디자인이 살짝 다르다보니까
종혁님 말씀대로 따로 컴포넌트로 빼도 될 것 같네요!
일단은 검색창 나오는데로 수정하겠습니다~
| const ProfilePopover = ({ ProfileButton }: IProfilePopover) => { | ||
| return ( | ||
| <Popover> | ||
| <PopoverTrigger>{ProfileButton}</PopoverTrigger> | ||
| <PopoverContent> | ||
| <PopoverArrow /> | ||
| <PopoverCloseButton /> | ||
| <PopoverHeader> | ||
| <Heading size="md">프로필</Heading> | ||
| </PopoverHeader> | ||
| <Divider /> | ||
| <PopoverBody>이쪽에 프로필 팝오버 목록을 넣을 예정입니다</PopoverBody> | ||
| </PopoverContent> | ||
| </Popover> | ||
| ) | ||
| } |
There was a problem hiding this comment.
NotificationPopover 와 의미 차이때문에 나눠놓으신 걸까요? Popover 자체를 하나의 컴포넌트로 만들어서 children같은걸로 body를 받는 방법은 어떠신지 궁금합니다
There was a problem hiding this comment.
요기는 미완성이긴 합니다!
알림과 프로필 popover가 서로 다른 로직을 사용하기도 하고
이후 어떤식으로 구현될지 아직 감을 못잡았다보니
일단 나눠놓고 구현한 다음 겹치는게 보이면 그걸 따로 컴포넌트로 묶어보겠습니다~
|
|
||
| import Menu from "./components/Menu" | ||
|
|
||
| const Header = () => { |
There was a problem hiding this comment.
저는 뭔가 Header의 의미 상 들어가는 컴포넌트들을 (대부분 차크라에서 제공하는 컴포넌트를 사용했어도) 하나의 커스텀 컴포넌트로 만들어도 좋을 것 같습니다
예를 들어
<Link
as={ReactRouterLink}
to="/">
<Image
src={SidePeekLogoSVG}
alt="side peek logo"
/>
</Link>
이런걸 Logo 컴포넌트로 만든다던지.. 해서 Header에서는 최대한 깔끔한? return문을 유지하면 나중에 요소 추가하기도 쉽지 않을까..싶습니다
There was a problem hiding this comment.
오... 맞는 말씀이에요
의미상으로 중요한 UI들이 딱히 특별한 로직이 없더라도
가독성 차원에서 컴포넌트로 구분해 놓으면
다른 분들이 읽을 때 훨씬 수월하겠네요!
너무 좋은 조언 감사합니다~! 👍👍
| import NotificationPopover from "./NotificationPopover" | ||
| import ProfilePopover from "./ProfilePopover" | ||
|
|
||
| const Menu = () => { |
There was a problem hiding this comment.
P3 :
Chakra UI에 Menu라는 컴포넌트가 존재해서(select랑 기능 비슷) 자세히는 안 봤지만 나중에 import할 때 조금 꼬일 수 있을 것 같습니다
There was a problem hiding this comment.
Chakra UI에 이미 있는거군요;;
근데 이 Menu는 헤더에서만 쓰이는 거기도 하고
혹시 Chakra Menu하고 같이 써야되면 Menu as ChakraMenu로 이름 바꿔서 사용해도 되니까
일단 이대로 머지하겠습니다!
#️⃣연관된 이슈
close #1
💡 핵심적으로 구현된 사항
➕ 그 외에 추가적으로 구현된 사항
react-icons를package.json에 추가하였습니다.theme에 헤더의 높이 변수headerHeight를 추가하였습니다.🤔 테스트,검증 && 고민 사항
npm i이 아니라npm ci를 해야 안전하게 재설치될 것 같습니다!