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

Refactor/#551 SongDetailListPage의 스와이프 추가 fetch 옵져버 로직에 callbackRef를 적용하여 로직 흐름을 개선한다. #552

Merged
merged 6 commits into from
Nov 7, 2023
70 changes: 70 additions & 0 deletions frontend/src/features/songs/hooks/useExtraSongDetail.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { useCallback, useRef } from 'react';
import useExtraFetch from '@/shared/hooks/useExtraFetch';
import useValidParams from '@/shared/hooks/useValidParams';
import createObserver from '@/shared/utils/createObserver';
import { getExtraNextSongDetails, getExtraPrevSongDetails } from '../remotes/songs';
import type { Genre } from '../types/Song.type';

const useExtraSongDetail = () => {
const { genre: genreParams } = useValidParams();

const { data: extraPrevSongDetails, fetchData: fetchExtraPrevSongDetails } = useExtraFetch(
getExtraPrevSongDetails,
'prev'
);

const { data: extraNextSongDetails, fetchData: fetchExtraNextSongDetails } = useExtraFetch(
getExtraNextSongDetails,
'next'
);

const prevObserverRef = useRef<IntersectionObserver | null>(null);
const nextObserverRef = useRef<IntersectionObserver | null>(null);

const getExtraPrevSongDetailsOnObserve: React.RefCallback<HTMLDivElement> = useCallback((dom) => {
if (dom !== null) {
prevObserverRef.current = createObserver(() =>
fetchExtraPrevSongDetails(getFirstSongId(dom), genreParams as Genre)
);

prevObserverRef.current.observe(dom);
return;
}

prevObserverRef.current?.disconnect();
}, []);

Check warning on line 35 in frontend/src/features/songs/hooks/useExtraSongDetail.ts

View workflow job for this annotation

GitHub Actions / test

React Hook useCallback has missing dependencies: 'fetchExtraPrevSongDetails' and 'genreParams'. Either include them or remove the dependency array

const getExtraNextSongDetailsOnObserve: React.RefCallback<HTMLDivElement> = useCallback((dom) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

useCallback 으로 참조 동일성 지켜주신 부분 👍 👍 👍 덕분에 많이 배웠어요.

if (dom !== null) {
nextObserverRef.current = createObserver(() =>
fetchExtraNextSongDetails(getLastSongId(dom), genreParams as Genre)
);

nextObserverRef.current.observe(dom);
return;
}

nextObserverRef.current?.disconnect();
}, []);

Check warning on line 48 in frontend/src/features/songs/hooks/useExtraSongDetail.ts

View workflow job for this annotation

GitHub Actions / test

React Hook useCallback has missing dependencies: 'fetchExtraNextSongDetails' and 'genreParams'. Either include them or remove the dependency array

const getFirstSongId = (dom: HTMLDivElement) => {
const firstSongId = dom.nextElementSibling?.getAttribute('data-song-id') as string;

return Number(firstSongId);
};
Comment on lines +50 to +54
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 저희 data-attribute로 처리하고 있는 것들 개선하는 것이 좋을 것 같아요.
사용하다보니 너무 비직관적이라 상태관리로 하는 것이 더 좋다고 생각이 듭니다.


const getLastSongId = (dom: HTMLDivElement) => {
const lastSongId = dom.previousElementSibling?.getAttribute('data-song-id') as string;

return Number(lastSongId);
};

return {
extraPrevSongDetails,
extraNextSongDetails,
getExtraPrevSongDetailsOnObserve,
getExtraNextSongDetailsOnObserve,
};
};

export default useExtraSongDetail;
21 changes: 21 additions & 0 deletions frontend/src/features/songs/hooks/useSongDetailEntries.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { useCallback } from 'react';
import useFetch from '@/shared/hooks/useFetch';
import useValidParams from '@/shared/hooks/useValidParams';
import { getSongDetailEntries } from '../remotes/songs';
import type { Genre } from '../types/Song.type';

const useSongDetailEntries = () => {
const { id: songIdParams, genre: genreParams } = useValidParams();

const { data: songDetailEntries } = useFetch(() =>
getSongDetailEntries(Number(songIdParams), genreParams as Genre)
);

const scrollIntoCurrentSong: React.RefCallback<HTMLDivElement> = useCallback((dom) => {
if (dom !== null) dom.scrollIntoView({ behavior: 'instant', block: 'start' });
}, []);

return { songDetailEntries, scrollIntoCurrentSong };
};

export default useSongDetailEntries;
23 changes: 14 additions & 9 deletions frontend/src/mocks/handlers/songsHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,13 @@ import { rest } from 'msw';
import comments from '../fixtures/comments.json';
import extraNextSongDetails from '../fixtures/extraNextSongDetails.json';
import extraPrevSongDetails from '../fixtures/extraPrevSongDetails.json';
import popularSongs from '../fixtures/popularSongs.json';
import recentSongs from '../fixtures/recentSongs.json';
import songEntries from '../fixtures/songEntries.json';
import type { KillingPartPostRequest } from '@/shared/types/killingPart';

const { BASE_URL } = process.env;

const songsHandlers = [
rest.get(`${BASE_URL}/songs/high-liked`, (req, res, ctx) => {
// const genre = req.url.searchParams.get('genre')
return res(ctx.status(200), ctx.json(popularSongs));
}),

rest.get(`${BASE_URL}/songs/:songId/parts/:partId/comments`, (req, res, ctx) => {
return res(ctx.status(200), ctx.json(comments));
}),
Expand Down Expand Up @@ -45,13 +39,24 @@ const songsHandlers = [
return res(ctx.status(200), ctx.json(songEntries));
}),
rest.get(`${BASE_URL}/songs/high-liked/:songId/prev`, (req, res, ctx) => {
// const genre = req.url.searchParams.get('genre')
return res(ctx.status(200), ctx.json(extraPrevSongDetails));
// const genre = req.url.searchParams.get('genre');
const { songId } = req.params;

const targetIdx = extraPrevSongDetails.findIndex((song) => song.id === Number(songId));
const sliced = extraPrevSongDetails.slice(0, targetIdx);

return res(ctx.status(200), ctx.json(sliced));
}),

rest.get(`${BASE_URL}/songs/high-liked/:songId/next`, (req, res, ctx) => {
// const genre = req.url.searchParams.get('genre')
return res(ctx.status(200), ctx.json(extraNextSongDetails));

const { songId } = req.params;

const targetIdx = extraNextSongDetails.findIndex((song) => song.id === Number(songId));
const sliced = extraNextSongDetails.slice(targetIdx);

return res(ctx.status(200), ctx.json(sliced));
}),

rest.get(`${BASE_URL}/songs/recent`, (req, res, ctx) => {
Expand Down
90 changes: 14 additions & 76 deletions frontend/src/pages/SongDetailListPage.tsx
Original file line number Diff line number Diff line change
@@ -1,97 +1,35 @@
import { useEffect, useLayoutEffect, useRef } from 'react';
import { styled } from 'styled-components';
import swipeUpDown from '@/assets/icon/swipe-up-down.svg';
import SongDetailItem from '@/features/songs/components/SongDetailItem';
import {
getExtraNextSongDetails,
getExtraPrevSongDetails,
getSongDetailEntries,
} from '@/features/songs/remotes/songs';
import useExtraSongDetail from '@/features/songs/hooks/useExtraSongDetail';
import useSongDetailEntries from '@/features/songs/hooks/useSongDetailEntries';
import useModal from '@/shared/components/Modal/hooks/useModal';
import Modal from '@/shared/components/Modal/Modal';
import Spacing from '@/shared/components/Spacing';
import useExtraFetch from '@/shared/hooks/useExtraFetch';
import useFetch from '@/shared/hooks/useFetch';
import useLocalStorage from '@/shared/hooks/useLocalStorage';
import useValidParams from '@/shared/hooks/useValidParams';
import createObserver from '@/shared/utils/createObserver';
import type { Genre } from '@/features/songs/types/Song.type';

const SongDetailListPage = () => {
const { isOpen, closeModal } = useModal(true);
const [onboarding, setOnboarding] = useLocalStorage<boolean>('onboarding', true);

const { id: songIdParams, genre: genreParams } = useValidParams();
const { data: songDetailEntries } = useFetch(() =>
getSongDetailEntries(Number(songIdParams), genreParams as Genre)
);

const { data: extraPrevSongDetails, fetchData: fetchExtraPrevSongDetails } = useExtraFetch(
getExtraPrevSongDetails,
'prev'
);

const { data: extraNextSongDetails, fetchData: fetchExtraNextSongDetails } = useExtraFetch(
getExtraNextSongDetails,
'next'
);

const itemRef = useRef<HTMLDivElement>(null);
const { songDetailEntries, scrollIntoCurrentSong } = useSongDetailEntries();

const prevTargetRef = useRef<HTMLDivElement | null>(null);
const nextTargetRef = useRef<HTMLDivElement | null>(null);
const {
extraPrevSongDetails,
extraNextSongDetails,
getExtraPrevSongDetailsOnObserve,
getExtraNextSongDetailsOnObserve,
} = useExtraSongDetail();

const getFirstSongId = () => {
const firstSongId = prevTargetRef.current?.nextElementSibling?.getAttribute(
'data-song-id'
) as string;
if (!songDetailEntries) return null;

return Number(firstSongId);
};

const getLastSongId = () => {
const lastSongId = nextTargetRef.current?.previousElementSibling?.getAttribute(
'data-song-id'
) as string;

return Number(lastSongId);
};
const { prevSongs, currentSong, nextSongs } = songDetailEntries;

const closeCoachMark = () => {
setOnboarding(false);
closeModal();
};

useEffect(() => {
if (!prevTargetRef.current) return;

const prevObserver = createObserver(() =>
fetchExtraPrevSongDetails(getFirstSongId(), genreParams as Genre)
);
prevObserver.observe(prevTargetRef.current);

return () => prevObserver.disconnect();
}, [fetchExtraPrevSongDetails, songDetailEntries, genreParams]);

useEffect(() => {
if (!nextTargetRef.current) return;

const nextObserver = createObserver(() =>
fetchExtraNextSongDetails(getLastSongId(), genreParams as Genre)
);
nextObserver.observe(nextTargetRef.current);

return () => nextObserver.disconnect();
}, [fetchExtraNextSongDetails, songDetailEntries, genreParams]);

useLayoutEffect(() => {
itemRef.current?.scrollIntoView({ behavior: 'instant', block: 'start' });
}, [songDetailEntries]);

if (!songDetailEntries) return;

const { prevSongs, currentSong, nextSongs } = songDetailEntries;

return (
<>
{onboarding && (
Expand All @@ -113,7 +51,7 @@ const SongDetailListPage = () => {
)}

<ItemContainer>
<ObservingTrigger ref={prevTargetRef} aria-hidden="true" />
<ObservingTrigger ref={getExtraPrevSongDetailsOnObserve} aria-hidden="true" />

{extraPrevSongDetails?.map((extraPrevSongDetail) => (
<SongDetailItem key={extraPrevSongDetail.id} {...extraPrevSongDetail} />
Expand All @@ -122,7 +60,7 @@ const SongDetailListPage = () => {
<SongDetailItem key={prevSongDetail.id} {...prevSongDetail} />
))}

<SongDetailItem ref={itemRef} key={currentSong.id} {...currentSong} />
<SongDetailItem ref={scrollIntoCurrentSong} key={currentSong.id} {...currentSong} />

{nextSongs.map((nextSongDetail) => (
<SongDetailItem key={nextSongDetail.id} {...nextSongDetail} />
Expand All @@ -131,7 +69,7 @@ const SongDetailListPage = () => {
<SongDetailItem key={extraNextSongDetail.id} {...extraNextSongDetail} />
))}

<ObservingTrigger ref={nextTargetRef} aria-hidden="true" />
<ObservingTrigger ref={getExtraNextSongDetailsOnObserve} aria-hidden="true" />
</ItemContainer>
</>
);
Expand Down
Loading