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

feat: 전역 suspense 개선 #427

Merged
merged 26 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5954faf
feat: 페이지간 이동 로딩바 컴포넌트 구현
bassyu Oct 9, 2023
6b6b962
feat: 마지막 페이지와 함께 로딩 바를 제어하는 suspense fallback 컴포넌트 구현
bassyu Oct 9, 2023
82f5a9a
feat: 마지막 페이지를 기록하는 Wrapper 컴포넌트 추가
bassyu Oct 9, 2023
9cdebe4
feat: Context 대신 Recoil 사용
bassyu Oct 9, 2023
24c3fef
chore: MSW delay 3초로 설정
bassyu Oct 9, 2023
e608b96
refactor: 모든 페이지에 PageLogger 적용
bassyu Oct 9, 2023
b11d4d4
design: backdrop 제거
bassyu Oct 9, 2023
e040fef
chore: PageLogger 적용
bassyu Oct 9, 2023
dd1c42b
fix: Page effect에 로딩바 숨기기 추가
bassyu Oct 9, 2023
c4f28a0
style: lastPageValue 네이밍 변경
bassyu Oct 9, 2023
cf91a54
design: 로딩바 속도 2배 증가
bassyu Oct 9, 2023
236b4a8
chore: MSW 지연 500으로 변경
bassyu Oct 9, 2023
c6fb2ab
design: 왼쪽 border 제거
bassyu Oct 9, 2023
a4209de
design: 프로그래스 바 속도 증가
bassyu Oct 9, 2023
594bd4a
refactor: style 파일 분리
bassyu Oct 9, 2023
3130327
refactor: isShow 대신 isShowPageLoading 재활용
bassyu Oct 9, 2023
19356c1
fix: 로딩바 깜빡거리는 현상 제거
bassyu Oct 9, 2023
37b8f87
chore: MSW delay 200으로 변경
bassyu Oct 9, 2023
18772b6
chore: 초기 로딩 속도 증가
bassyu Oct 9, 2023
911b4af
fix: 애니메이션 겹치는 현상 해결
bassyu Oct 13, 2023
17d4a20
design: 두께 3px로 변경
bassyu Oct 13, 2023
8e8bd16
chore: useEffect 의존성 배열에 사용되는 값 추가
bassyu Oct 13, 2023
b43776c
chore: || 대신 ?? 사용
bassyu Oct 13, 2023
23630ae
Merge branch 'develop' into feature/426-전역_suspense_개선
bassyu Oct 13, 2023
e19cf6b
refactor: 페이지에 Main 태그 적용 및 hooks 폴더 이동
bassyu Oct 13, 2023
eb0e819
chore: 폴더 이동 import 적용
bassyu Oct 13, 2023
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
2 changes: 2 additions & 0 deletions frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { RouterProvider } from 'react-router-dom';
import { RecoilRoot } from 'recoil';
import { ThemeProvider } from 'styled-components';
import Confirm from 'components/@common/Confirm';
import PageLoadingBar from 'components/@common/PageLoadingBar';
import SvgSpriteMap from 'components/@common/SvgIcons/SvgSpriteMap';
import ToastList from 'components/@common/Toast/ToastList';
import { GlobalStyle } from 'style/Global.style';
Expand All @@ -22,6 +23,7 @@ const App = () => {
<RouterProvider router={router} />
<Confirm />
<ToastList />
<PageLoadingBar />
</RecoilRoot>
</QueryClientProvider>
</ThemeProvider>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/@common/Calendar/index.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { useState } from 'react';
import SvgIcons from 'components/@common/SvgIcons/SvgFill';
import { AlertSpan, Button, CalendarBox, DaysBox, HeaderBox, Wrapper } from './Calendar.style';
import useCalendar from 'hooks/@common/useCalendar';
import { convertDateKorYear, getDateToString, getDayInfo, getDaysBetween } from 'utils/date';
import { DateValidate } from 'utils/validate';
import { DAYS_OF_THE_WEEK } from 'constants/index';
import theme from 'style/theme.style';
import DaySmallBox from './DaySmallBox';
import useCalendar from './hooks/useCalendar';

interface CalendarProps {
selectedDate: Date | null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { keyframes, styled } from 'styled-components';

export const ProgressBar = styled.div`
position: fixed;
z-index: ${(props) => props.theme.zIndex.tooltip};
top: 0;
left: 0;

width: 100%;
height: 3px;

background-color: ${(props) => props.theme.color.primary};
border-radius: 0 4px 4px 0;
`;

export const progressing = keyframes`
0% { transform: translateX(-100%); }
50% { transform: translateX(-20%); }
100% { transform: translateX(0); }
`;

export const Progressing = styled(ProgressBar)`
animation: ${progressing} 6s ease-out;
`;

export const fillOut = keyframes`
0% {
opacity: 1;
transform: translateX(-100%);
}
50% {
opacity: 1;
transform: translateX(0);
}
100% {
opacity: 0;
transform: translateX(0);
}
`;

export const Finish = styled(ProgressBar)<{ $animationTime: number }>`
animation: ${fillOut} ${(props) => props.$animationTime}ms;
`;
44 changes: 44 additions & 0 deletions frontend/src/components/@common/PageLoadingBar/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { useEffect, useMemo } from 'react';
import { createPortal } from 'react-dom';
import { useRecoilValue } from 'recoil';
import { Finish, Progressing } from './PageLoadingBar.style';
import { isShowPageLoadingState } from 'store/atoms/@common';
import useToggle from 'hooks/@common/useToggle';

export const FINISH_ANIMATION_TIME = 600;

const PageLoadingBar = () => {
const isShowPageLoading = useRecoilValue(isShowPageLoadingState);

const { isOn: isShow, on: show, off: hide } = useToggle();
const { isOn: isShowFinish, on: showFinish, off: hideFinish } = useToggle();

const root = useMemo(() => document.getElementById('root')!, []);
Copy link
Collaborator

Choose a reason for hiding this comment

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

C

root엘리먼트가 없을 리가 없으니까 굳이 !를 사용하지 않고 단언을 사용해도 될 것 같은데 어떻게 생각하시나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

사실 없을리가 없으니 지금은 둘다 비슷할 것 같긴 하네요..
타입 자체를 바꿀 수 있는 단언보다, 타입은 같지만 있다는걸 확신해주는 !를 사용하는게 조금 더 안전한 코드라고 생각했습니다~

Copy link
Member

Choose a reason for hiding this comment

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

document.body가 아니라 react root element에 넣는 이유가 궁금해요

Copy link
Member Author

Choose a reason for hiding this comment

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

특별한 이유는 없었습니다! 혹시 body에 넣으면 장단점이 있을까요?

Copy link
Member

@WaiNaat WaiNaat Oct 14, 2023

Choose a reason for hiding this comment

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

  1. 불필요한 non-null assertion이 없어집니다
  2. nextjs처럼 root가 없는 환경으로 이사갈 때 편합니다
  3. position이 fixed라서 body에 삽입되더라도 다른 body의 요소들과 꼬이지 않습니다
  4. useMemo를 사용할 이유가 없어집니다


useEffect(() => {
if (isShowPageLoading) {
show();
hideFinish();
return;
}

showFinish();
const hideId = setTimeout(hide, FINISH_ANIMATION_TIME / 2);
const hideFinishId = setTimeout(hideFinish, FINISH_ANIMATION_TIME);

return () => {
clearTimeout(hideId);
clearTimeout(hideFinishId);
};
}, [isShowPageLoading, show, hide, showFinish, hideFinish]);

return createPortal(
<>
{isShow && <Progressing />}
{isShowFinish && <Finish $animationTime={FINISH_ANIMATION_TIME} />}
</>,
root
);
};

export default PageLoadingBar;
18 changes: 18 additions & 0 deletions frontend/src/components/@common/PageLogger/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { PropsWithChildren, useEffect } from 'react';
import { useSetRecoilState } from 'recoil';
import { isShowPageLoadingState, lastPageState } from 'store/atoms/@common';

const PageLogger = (props: PropsWithChildren) => {
const { children } = props;
const setLastPage = useSetRecoilState(lastPageState);
const setIsShowPageLoadingState = useSetRecoilState(isShowPageLoadingState);

useEffect(() => {
setLastPage(children);
setIsShowPageLoadingState(false);
}, [children, setLastPage, setIsShowPageLoadingState]);

return children;
};

export default PageLogger;
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Fragment } from 'react';
import useShowState from 'hooks/@common/useShowState';
import { ContentBox, SeeMoreButton, SeeMoreButtonArea, Wrapper } from './SeeMoreContentBox.styles';
import useShowState from './hooks/useShowState';

interface SeeMoreContentBoxProps {
children: string;
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/mocks/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import gardenHandlers from './handlers/gardenHandlers';
import historyHandlers from './handlers/historyHandlers';

export const worker = setupWorker(
...makeHandler(0, 0),
...makeHandler(200, 0),
...historyHandlers,
...dictionaryPlantRegistrationHandlers,
...gardenHandlers
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { keyframes, styled } from 'styled-components';

export const Wrapper = styled.div`
export const Main = styled.main`
display: flex;
flex-direction: column;
align-items: center;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
import { useState } from 'react';
import InstallPrompt from 'components/@common/InstallPrompt';
import Navbar from 'components/@common/Navbar';
import PageLogger from 'components/@common/PageLogger';
import SearchBox from 'components/search/SearchBox';
import { LogoMessage, SearchBoxArea, SearchMessage, Wrapper, Image, ImageArea } from './Main.style';
import { LogoMessage, SearchBoxArea, SearchMessage, Main, Image, ImageArea } from './Home.style';
import useDictionaryNavigate from 'hooks/dictionaryPlant/useDictionaryPlantNavigate';
import LogoSvg from 'assets/logo.svg';
import LogoWebp from 'assets/logo.webp';

const Main = () => {
const Home = () => {
const { goToProperDictionaryPlantPage, goToDictionaryPlantDetailPage } = useDictionaryNavigate();
const [searchValue, setSearchValue] = useState('');

return (
<>
<PageLogger>
<InstallPrompt />
<Wrapper>
<Main>
<LogoMessage>식물을 쉽게</LogoMessage>
<ImageArea>
<picture>
Expand All @@ -32,10 +33,10 @@ const Main = () => {
/>
</SearchBoxArea>
<SearchMessage>피움에 등록된 식물을 검색해 보세요!</SearchMessage>
</Wrapper>
</Main>
<Navbar />
</>
</PageLogger>
);
};

export default Main;
export default Home;
21 changes: 21 additions & 0 deletions frontend/src/pages/@common/LastPageLoading/index.tsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

R

해당 파일은 페이지 보다는 fallback을 나타내는 Loading component같은데 component로 옮기는건 어떤가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

어렵네요.. RootTemplate의 Outlet이 전부 페이지면서, Outlet 대신 들어가는 fallback이라 페이지라고 생각했어요! (원래의 Loading이 페이지에 있기도 했구요)
Outlet을 감싸는 Suspense는 페이지로 보고, 대신 그 안에서 감싸는 Suspense는 컴포넌트로 하는 건 어떤가요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

전 일반적인 페이지라고 한다면 고유한 url이 있어야 한다고 생각하는데 쵸파가 생각하는 페이지의 기준은 무엇인가요? (확실히 전역을 감싸는 에러 페이지나 전역 Suspense는 좀 애매하긴 하네요)

Copy link
Member Author

Choose a reason for hiding this comment

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

Outlet에 들어가는 내용이 페이지라고 생각했는데 url 기준도 좋네요!

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { useEffect } from 'react';
import { useRecoilValue, useSetRecoilState } from 'recoil';
import Loading from 'pages/@common/Loading';
import { isShowPageLoadingState, lastPageState } from 'store/atoms/@common';

const LastPageLoading = () => {
const lastPage = useRecoilValue(lastPageState);
const setIsShowPageLoading = useSetRecoilState(isShowPageLoadingState);

useEffect(() => {
setIsShowPageLoading(true);

return () => {
setIsShowPageLoading(false);
};
}, [setIsShowPageLoading]);

return lastPage ?? <Loading />;
};

export default LastPageLoading;
4 changes: 2 additions & 2 deletions frontend/src/pages/@common/RootTemplate/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Suspense } from 'react';
import { Outlet } from 'react-router-dom';
import NotFound from 'pages/@common/Error/NotFound';
import Loading from 'pages/@common/Loading';
import LastPageLoading from 'pages/@common/LastPageLoading';
import ErrorBoundary from 'components/@common/ErrorBoundary';
import Redirect from 'components/@common/Redirect';
import { PageArea, Wrapper } from './RootTemplate.style';
Expand All @@ -23,7 +23,7 @@ const RootTemplate = () => {
}
statusCode={401}
>
<Suspense fallback={<Loading />}>
<Suspense fallback={<LastPageLoading />}>
<Outlet />
</Suspense>
</ErrorBoundary>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/pages/auth/MyPage/MyPage.style.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Link } from 'react-router-dom';
import styled from 'styled-components';

export const Wrapper = styled.main`
export const Main = styled.main`
Copy link
Collaborator

Choose a reason for hiding this comment

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

C

컴포넌트 이름을 바꾼 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

다만, 최상단 레이아웃을 Wrapper로 통일하기로 했던 것 같은데 어디는 Main으로 바뀌는게 보이는데 그 이유를 물어봐도 될까요?

페이지마다 헤더와 푸터가 다르기 때문에 헤더와 푸터도 페이지의 구성요소라고 생각했습니다.
그래서 현재 MyPage에서 최상단 레이아웃 또는 최상단 엘리먼트는 사실 PageLogger라고 생각했습니다!
형재요소와 함께 Wrapper가 존재하면 오히려 컨벤션에 맞지 않다고 생각했어요~

position: relative;

display: flex;
Expand Down
11 changes: 6 additions & 5 deletions frontend/src/pages/auth/MyPage/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { FixedButtonArea } from 'pages/garden/GardenPostList/GardenPostList.style';
import ContentHeader from 'components/@common/ContentHeader';
import Navbar from 'components/@common/Navbar';
import PageLogger from 'components/@common/PageLogger';
import SvgFill from 'components/@common/SvgIcons/SvgFill';
import Toggle from 'components/@common/Toggle';
import VerticalDivider from 'components/@common/VerticalDivider/VerticalDivider.style';
Expand All @@ -11,7 +12,7 @@ import {
PushAlertContent,
PushAlertWrapper,
WarnParagraph,
Wrapper,
Main,
} from './MyPage.style';
import useConfirm from 'hooks/@common/useConfirm';
import usePushAlert from 'hooks/@common/usePushAlert';
Expand Down Expand Up @@ -43,9 +44,9 @@ const MyPage = () => {
};

return (
<>
<PageLogger>
<ContentHeader title="마이페이지" />
<Wrapper>
<Main>
<PushAlertWrapper>
<PushAlertContent>
<p>리마인더 알림 받기</p>
Expand Down Expand Up @@ -74,15 +75,15 @@ const MyPage = () => {
회원 탈퇴
</Button>
</ButtonBox>
</Wrapper>
</Main>
<Navbar />
<FixedButtonArea>
<BottomSheet to="https://forms.gle/rQUAi9GbVwrr7oG2A" target="blank">
<SvgFill icon="survey" color={theme.color.background} size={16} />
문의하기
</BottomSheet>
</FixedButtonArea>
</>
</PageLogger>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { generatePath, useNavigate, useParams } from 'react-router-dom';
import { Header } from 'pages/petPlant/PetPlantRegister/Form/Form.style';
import Image from 'components/@common/Image';
import PageLogger from 'components/@common/PageLogger';
import SvgFill from 'components/@common/SvgIcons/SvgFill';
import DictionaryPlantContent from 'components/dictionaryPlant/DictionaryPlantContent';
import { BackButton, BottomSheet, Main, PrimaryButton } from './DictionaryPlantDetail.style';
Expand Down Expand Up @@ -46,7 +47,7 @@ const DictionaryPlantDetail = () => {
};

return (
<>
<PageLogger>
<Header>
<BackButton onClick={goBack}>
<SvgFill icon="line-arrow-left" aria-label="뒤로 가기" color={theme.color.sub} />
Expand All @@ -61,7 +62,7 @@ const DictionaryPlantDetail = () => {
반려 식물로 등록하기
</PrimaryButton>
</BottomSheet>
</>
</PageLogger>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useState } from 'react';
import { useSearchParams } from 'react-router-dom';
import Navbar from 'components/@common/Navbar';
import PageLogger from 'components/@common/PageLogger';
import SearchBox from 'components/search/SearchBox';
import SearchResults from 'components/search/SearchResults';
import { Title, Wrapper } from './DictionaryPlantSearch.style';
Expand All @@ -14,7 +15,7 @@ const DictionarySearch = () => {
const [searchValue, setSearchValue] = useState('');

return (
<>
<PageLogger>
<Wrapper>
<SearchBox
value={searchValue}
Expand All @@ -27,7 +28,7 @@ const DictionarySearch = () => {
<SearchResults plantName={search} />
</Wrapper>
<Navbar />
</>
</PageLogger>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useLocation } from 'react-router-dom';
import ContentHeader from 'components/@common/ContentHeader';
import Navbar from 'components/@common/Navbar';
import PageLogger from 'components/@common/PageLogger';
import { Description, Main } from './NewDictionaryPlantRequest.style';
import { NUMBER } from 'constants/index';
import Form from './Form';
Expand All @@ -11,14 +12,14 @@ const NewDictionaryPlantRequest = () => {
const initialName = typeof state === 'string' ? state.slice(0, NUMBER.maxNicknameLength) : '';

return (
<>
<PageLogger>
<ContentHeader title="새로운 식물 추가 요청하기" />
<Main>
<Description>요청하신 식물은 저희가 검토 후 추가할게요!</Description>
<Form initialName={initialName} />
</Main>
<Navbar />
</>
</PageLogger>
);
};

Expand Down
Loading