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

[FE] 페이지 상단 이동 버튼 구현 #153

Merged
merged 6 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions frontend/src/assets/upperArrow.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
18 changes: 18 additions & 0 deletions frontend/src/components/common/TopButton/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import UpperArrowIcon from '@/assets/upperArrow.svg';
import useTopButton from '@/hooks/useTopButton';

import * as S from './style';

const TopButton = () => {
const { showTopButton, scrollToTop } = useTopButton();
Copy link
Contributor

Choose a reason for hiding this comment

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

버튼 로직을 훅으로 뺀 것 굳굳입니다


if (!showTopButton) return null;

return (
<S.TopButton onClick={scrollToTop} type="button">
<S.ArrowImage src={UpperArrowIcon} alt="위 화살표"></S.ArrowImage>
</S.TopButton>
);
};

export default TopButton;
26 changes: 26 additions & 0 deletions frontend/src/components/common/TopButton/style.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import styled from '@emotion/styled';

export const TopButton = styled.button`
position: fixed;
right: 5rem;
bottom: 5rem;

display: flex;
align-items: center;
justify-content: center;

width: 5rem;
height: 5rem;

color: ${({ theme }) => theme.colors.white};

background-color: ${({ theme }) => theme.colors.primary};
filter: drop-shadow(0 0 0.2rem ${({ theme }) => theme.colors.primary});
border: 0.2rem solid ${({ theme }) => theme.colors.primary};
border-radius: 100%;
`;

export const ArrowImage = styled.img`
width: 3rem;
height: 3rem;
`;
Comment on lines +3 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

마지막으로 push 하기 전에 yarn run lint:styles 로 css 순서를 정리해보는 건 어떨까요!

1 change: 1 addition & 0 deletions frontend/src/components/common/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ export { default as ProjectImg } from './ProjectImg';
export { default as ReviewDate } from './ReviewDate';
export { default as ReviewComment } from './ReviewComment';
export { default as MultilineTextViewer } from './MultilineTextViewer';
export { default as TopButton } from './TopButton';
export * from './modals';
3 changes: 3 additions & 0 deletions frontend/src/components/layouts/PageLayout/index.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { PropsWithChildren } from 'react';

import { TopButton } from '@/components/common';

import * as S from './styles';

const PageLayout = ({ children }: PropsWithChildren) => {
return (
<S.Layout>
<S.Wrapper>{children}</S.Wrapper>
<TopButton />
</S.Layout>
);
};
Expand Down
39 changes: 39 additions & 0 deletions frontend/src/hooks/useTopButton.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { useState, useEffect } from 'react';

const TOP_BUTTON_DISPLAY_THRESHOLD = 500;
Copy link
Contributor

Choose a reason for hiding this comment

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

500이라는 특정 기준을 잡는거 좋아요


interface UseTopButtonResult {
showTopButton: boolean;
scrollToTop: () => void;
}

const useTopButton = (): UseTopButtonResult => {
Copy link
Contributor

Choose a reason for hiding this comment

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

버튼에 대한 이벤트들을 훅으로 분리한 것 좋아요.

const [showTopButton, setShowTopButton] = useState(false);

const handleShowTopButton = () => {
if (window.scrollY > TOP_BUTTON_DISPLAY_THRESHOLD) {
Copy link
Contributor

Choose a reason for hiding this comment

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

요기서 if문의 결과를 별도의 변수로 받고, 그 변수를 바로 set 함수에 전달하는 건 어떤가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저는 2개 이상의 조건문이 AND 또는 OR로 결합되어 있는 경우에 별도의 변수로 분리하는 편이에요. 상수의 이름이 조금 길어서 if문이 길어지게 된 것 같은데, 혹시 지금 코드에서 가독성에 문제가 있다고 보시나요??

setShowTopButton(true);
} else {
setShowTopButton(false);
}
};

useEffect(() => {
window.addEventListener('scroll', handleShowTopButton);

return () => {
window.removeEventListener('scroll', handleShowTopButton);
};
}, []);

const scrollToTop = () => {
window.scrollTo({
top: 0,
behavior: 'smooth',
});
};

return { showTopButton, scrollToTop };
};

export default useTopButton;