Conversation
…ithub-actions [Fix] delete merged branch github action
…승-week2 [이현승] Week2
…현승-week3 Part1 이현승 week3
Part2 이현승 week8
|
|
||
| const GlobalStyle = createGlobalStyle` |
There was a problem hiding this comment.
초기화하는 스타일을 지정해주신 것 같아요.
css 파일을 불러오는 것보다 createGlobalStyle 를 이용해 전역 스타일을 설정했을 때의 이점이 있나요??
| [asyncFunction] | ||
| ); | ||
|
|
||
| return [data, getData] as [any, (value?: string | number) => Promise<void>]; |
There was a problem hiding this comment.
as 로 강제하기 보다 훅 함수의 타입을 지정해주면 어떨까요?
type UseData = (asyncFunction: (pram?: string | number) => Promise<any>) => [any, (value?: string | number) => Promise<void>]
export const useData:UseData = (asyncFunction) => {
// ...
}
추가로 제네릭 타입을 사용해 any 대신 제네릭 타입을 설정해 data 의 타입도 지정해보세요!
| const getData = useCallback(async () => { | ||
| await getFolder(); | ||
| }, [getFolder]); | ||
|
|
||
| useEffect(() => { | ||
| getData(); | ||
| }, [getData]); |
There was a problem hiding this comment.
- getData 는 불필요해보입니다. 별도 함수 선언 없이 useEffect 에서 호출해서 사용할 수 있을 것 같습니다.
- 폴더 정보 불러오는건 페이지 접근했을 때 최초에만 실행하면 될 것 같아서 deps 로 빈 배열을 넘기는게 적절할 것 같습니다.
- next 를 쓰는 만큼 서버 렌더링을 사용해보세요. 이 페이지가 딱 적용하기 쉬워보이네요!
There was a problem hiding this comment.
변경된 파일이 많아 리뷰할 때 깃허브가 버벅입니다..!
WeeklyMissionPart1 가 이전 파트 작업물이라면 ignore 에 추가하거나 별도 워크스페이스로 관리하면 좋을 것 같습니다~
| const Folder = ({ item }: { item: { [folder: string]: any } }) => { | ||
| const [clicked, setClicked] = useState(false); | ||
|
|
||
| const handleClick = () => { | ||
| setClicked(!clicked); | ||
| }; | ||
| return ( | ||
| <Folderr | ||
| tabIndex={0} | ||
| role="button" | ||
| $clicked={clicked} | ||
| onClick={handleClick} | ||
| > | ||
| <FolderName>{item?.name}</FolderName> | ||
| <FolderLinks>{item?.link.count}개 링크</FolderLinks> | ||
| </Folderr> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
컴포넌트 파일을 하나 생성해 Folder 를 옮기고 가져오도록 수정해주세요.
컴포넌트 파일 하나 당 컴포넌트가 여러개 있으면 코드 파악하기가 어렵게 됩니다.
| const items = linkData?.data; | ||
|
|
||
| const searchedItems = items?.filter((item: LinkItem) => { | ||
| if (searching === "" || searching === undefined) return true; |
There was a problem hiding this comment.
!folder 조건으로 줄일 수 있겠네요~
| const searchedItems = items?.filter((item: LinkItem) => { | ||
| if (searching === "" || searching === undefined) return true; | ||
| return ( | ||
| searching !== undefined && |
There was a problem hiding this comment.
위에서 undefined 일 때를 처리해서 불필요한 조건으로 보입니다.
| const NoLink = styled.div` | ||
| display: flex; | ||
| width: 100%; | ||
| height: 100px; | ||
| padding: 41px 0px 35px; | ||
| justify-content: center; | ||
| align-items: center; | ||
| margin-bottom: 200px; | ||
|
|
||
| @media (max-width: 767px) { | ||
| font-size: 14px; | ||
| line-height: normal; | ||
| } | ||
| `; | ||
|
|
||
| const Frame = styled.div` | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 24px; | ||
| `; |
There was a problem hiding this comment.
개인적으로 스타일드 컴포넌트는 별도 파일로 분리하거나 컴포넌트 함수 밑에 배치하길 선호합니다.
컴포넌트 파일에서 가장 중요한건 컴포넌트 함수라 위쪽에 바로 보이도록 하기 위함입니다.
| useEffect(() => { | ||
| document.body.style.cssText = ` | ||
| position: fixed; | ||
| top: -${window.scrollY}px; | ||
| overflow-y: scroll; | ||
| width: 100%;`; | ||
| return () => { | ||
| const scrollY = document.body.style.top; | ||
| document.body.style.cssText = ""; | ||
| window.scrollTo(0, parseInt(scrollY || "0", 10) * -1); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
스크롤을 내린 상태에서도 모달을 중앙에 배치하게 하기 위해 넣어주신 코드같은데 맞을까요..?
body 의 속성을 수정하지 않고 컴포넌트 만으로 해결할 수 있을 것 같은데 확인해주세요~
There was a problem hiding this comment.
모달 창이 열렸을 때 원래 페이지의 스크롤을 고정시켜서 움직이지 않도록 하는 코드입니다.
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게