Skip to content
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

[오동혁] Week12 #377

Merged

Conversation

ohdong9795
Copy link
Collaborator

@ohdong9795 ohdong9795 commented May 5, 2024

요구사항

기본

  • TypeScript를 활용해 프로젝트의 필요한 곳에 타입을 명시해 주세요.
  • ‘/shared’, ‘/folder’ 페이지에 현재 폴더에서 검색어가 포함된 링크를 찾을 수 있는 링크 검색 기능을 만들어 주세요.

심화

  • 폴더 페이지에 상단에 있는 링크 추가하기 input이 화면에서 가려질 때 최하단에 고정해 주세요.

주요 변경사항

  • typescript 적용
  • 검색 기능 적용
  • 링크 추가하기 input 하단 고정

스크린샷

1
2
3

멘토에게

  • 처음부터 typescript로 만든게 아니라 js -> ts로 적용하려하니 정신없이 오류만 고쳐가면서 처리해서 interface extends나 재활용면에서 많이 부족한 것 같지만 일단 돌아는 가게 했습니다.
  • 모든 파일이 수정되었고 commit이 한개라서 리뷰하시기 힘들 것 같아서 궁금했던 점을 적어두겠습니다.
  • /src/pages/folder/Header.jsx에서 심화 요구사항을 구현할 때
    input을 하단에 fixed 했을 때 fixed는 공간을 차지하지 않아 footer가 가려져서 임시방편으로 빈 div에 height를 주고 body 최하단에 넣는 방식으로 처리했습니다.
    스크롤을 끝까지 내렸을 때 footer가 가려지진 않지만 footer아래에 input이 오게되는데 input이 최하단에 있다가 푸터를 만날때 푸터 위에 고정되도록 하려면 어떻게 처리해야할까요?

@ohdong9795 ohdong9795 requested a review from Ahseo May 5, 2024 07:49
Copy link
Collaborator

@Ahseo Ahseo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

js -> ts로 바꾸는 작업을 잘 해주신 것 같아요! any type을 사용한 부분도 없고(kakao 제외) generic type을 사용하셔서 잘 구현해주신 부분도 쉽지 않을 수도 있는데 잘 해주신 것 같습니다 :)

질문에 대한 답변도 해당 코드 부분에 달아두었으니 참고해주세요~!

고생하셨습니다 ㅎㅎ

error: Error | null;
}

const useFetch = <T>({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generic type에 대한 이해도를 가지고 바로 사용하시는 부분이 좋네요! ㅎㅎ


interface UseFetchParams {
url?: string;
method?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

method의 경우에는 경우의 수가 한정적이라 GET | POST ,,, 이렇게 정의해두고 쓰면 더 좋을 것 같아요!

return (
<>
<StyledHeader ref={DivRef}>
<PositionedDiv $isView={isView}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 말씀이시군요!! 제 생각에는, 헤더 안에서 이를 다 처리하는 것은 어려울 것 같고, 위계상으로도 조금 모호할 것 같아요. 페이지단에서 처리하는 것이 더 좋을 것 같아요.
헤더 컴포넌트 안에서 처리하지만 페이지내에서 같이 스크롤되면서 footer 위에 보여지는 부분이기 때문에 구현은 가능하나, 가독성이 좋은 방식은 아닐 것 같습니다!

제안을 드린다면 우선 footer에서 보이는 컴포넌트와 header에서 보이는 컴포넌트를 분리하는게 좋을 것 같아요.
컴포넌트는 두개를 만들고, 화면에서 보이는지 안보이는지에 대한 처리를(inView로 처리한 부분) 페이지단에서 하면 어떨까요~?

기본적으로 footer에는 input이 페이지에서 붙어있고, 헤더에도 붙어있지만
둘다 화면에 보이지 않을 때 화면 바닥에 붙어있는 input이 호출되도록 하는거죠 ㅎㅎ


function Content({ folderData }: ContentProps) {
const [selectedId, setSelectedId] = useState(0);
const [searchValue, setSearchValue] = useState<string>('');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 경우에는 이 없어도 됩니다 ㅎㅎ 기본적으로 초기 값을 세팅하는 과정에서 string이라고 자동으로 인식될거에요!

Comment on lines +31 to +39
const handleLoad = () => setLoading(false);
const handleError = (e: ErrorEvent) => {
setError(e.error);
};

script.addEventListener('load', handleLoad);
script.addEventListener('error', handleError);

document.body.appendChild(script);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 작성해도 되지만, 만약 이렇게되면 useEffect의 dependency인 src와 option이 바뀌어서 다시 호출될 때 함수가 계속 재정의될거에요. 이를 방지하려면 바깥이 useCallback으로 작성할 수도 있으니 참고해주세요~!

@Ahseo Ahseo merged commit a0f4176 into codeit-bootcamp-frontend:part2-오동혁 May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants