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.

Sign up for GitHub

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

Jump to bottom

[#439] [독서모임] 모임 목록 페이지 리팩토링 #440

Merged
merged 10 commits into from
Nov 23, 2023
Merged

[#439] [독서모임] 모임 목록 페이지 리팩토링 #440

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2f925c6
refactor: 모임 목록 페이지 리팩토링
WooDaeHyun Nov 11, 2023
a5465bc
fix: type error 수정
WooDaeHyun Nov 11, 2023
1b8b293
fix: build error 수정
WooDaeHyun Nov 11, 2023
ead74ea
Chore: 이벤트 핸들러 명칭 수정
WooDaeHyun Nov 14, 2023
c9c0cc1
Chore: storybook prop 수정
WooDaeHyun Nov 14, 2023
2798e04
Fix: Simple,DetailBookGroupCard Link 태그로 전환
WooDaeHyun Nov 18, 2023
0dd536c
Fix: owner.name type 정의 수정
WooDaeHyun Nov 18, 2023
6517b39
Merge branch 'main' of https://github.com/prgrms-web-devcourse/Team-G…
WooDaeHyun Nov 18, 2023
12e7e94
Fix: SimpleBookGroupCard href 수정
WooDaeHyun Nov 18, 2023
bf719dc
Fix: build 에러 수정
WooDaeHyun Nov 18, 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
12 changes: 12 additions & 0 deletions next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,18 @@ const nextConfig = {
port: '',
pathname: '/**',
},
{
protocol: 'http',
hostname: 'k.kakaocdn.net',
port: '',
pathname: '/**',
},
{
protocol: 'https',
hostname: 'blog.kakaocdn.net',
port: '',
pathname: '/**',
},
Comment on lines +37 to +48
Copy link
Member

Choose a reason for hiding this comment

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

ask;
기존에 있던 search1.kakaocdn.net 호스트 외에 두 호스트 에서 받아오는 이미지가 있나요? 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

@minjongbaek
네 말씀주신 호스트 외에 독서 모임 목록 페이지에서 위의 두 호스트에서 받아오는 이미지가 존재하여 에러가 발생해 추가로 작성하게 되었습니다! 👀

],
},
};
Expand Down
130 changes: 83 additions & 47 deletions src/app/group/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,57 +1,38 @@
'use client';

import TopHeader from '@/ui/Base/TopHeader';
import SearchGroup from '@/v1/bookGroup/SearchGroup';
import SimpleBookGroupCard from '@/v1/bookGroup/SimpleBookGroupCard';
import DetailBookGroupCard from '@/v1/bookGroup/DetailBookGroupCard';

import useEntireGroupsQuery from '@/queries/group/useEntireGroupsQuery';
import GroupHeader from '@/ui/Group/GroupHeader';
import GroupList from '@/ui/Group/GroupList';
import GroupSearch from '@/ui/Group/GroupSearch';
import { Box, Skeleton, VStack } from '@chakra-ui/react';
import { useState, useEffect } from 'react';
import useMyGroupsQuery from '@/queries/group/useMyGroupsQuery';
import Link from 'next/link';
import { Skeleton, VStack } from '@chakra-ui/react';
import { useEffect } from 'react';
import { useInView } from 'react-intersection-observer';

interface SearchValue {
[key: string]: string;
input: string;
select: string;
}

const GroupPage = () => {
const [searchValue, setSearchValue] = useState<SearchValue>({
input: '',
select: '모임',
});
const { ref, inView } = useInView();

const {
isSuccess,
data,
isSuccess: entireGroupsIsSuccess,
data: entireGroupsData,
isLoading,
fetchNextPage,
hasNextPage,
isFetchingNextPage,
} = useEntireGroupsQuery();

const { isSuccess: myGroupsIsSuccess, data: myGroupsData } =
useMyGroupsQuery();

useEffect(() => {
if (inView && hasNextPage) {
fetchNextPage();
}
}, [fetchNextPage, inView, hasNextPage]);

const handleSumbit = () => {
const { input } = searchValue;
if (input.trim().length === 0) {
/*공백만 입력한 경우 전체 데이터 렌더링 */
} else {
/*검색 API호출 및 setMeetingListData 업데이트 */
}
};

const handleChange = (name: string, value: string) => {
if (!(name in searchValue)) return;
const tempSearchValue = { ...searchValue };
tempSearchValue[name] = value;
setSearchValue(tempSearchValue);
};

if (isLoading)
return (
<VStack gap="0.5rem" align="stretch" w="100%">
Expand All @@ -63,22 +44,77 @@ const GroupPage = () => {
);

return (
<VStack align="center">
<Box w="100%">
<GroupHeader />
<GroupSearch
searchValue={searchValue}
handleChange={handleChange}
handleSumbit={handleSumbit}
<>
<TopHeader pathname={'/group'} />
<div className="mt-[2rem] flex w-full flex-col gap-[1.5rem]">
<SearchGroup
onClick={() => {
alert('추후 업데이트 될 예정입니다.');
}}
/>
{isSuccess &&
data.pages.map((groups, idx) => {
return <GroupList key={idx} bookGroups={groups.bookGroups} />;
})}
</Box>
<Box ref={ref} />
<div className="mt-[0.7rem] flex gap-[1rem] overflow-scroll">
{myGroupsIsSuccess &&
myGroupsData.bookGroups.map(group => {
const { title, book, bookGroupId } = group;
return (
//API isOwner 값이 존재하지 않아 비교하는 로직 추가 필요
<Link key={bookGroupId} href={`/book/${book.id}`}>
<SimpleBookGroupCard
title={title}
imageSource={book.imageUrl}
isOwner={false}
/>
</Link>
);
})}
</div>
<div className="flex flex-col gap-[1rem]">
{entireGroupsIsSuccess &&
entireGroupsData.pages.map(groups => {
return groups.bookGroups.map(group => {
Comment on lines +71 to +73
Copy link
Member Author

Choose a reason for hiding this comment

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

받아온 데이터를 두 번 풀어서 사용해야 하는 상황에서 반복문을 두 번 돌리는 것이 어색한 것 같아 별도의 방법을 찾아보던 중
Tanstack Query 예제에서 제가 작성한 코드처럼 map을 두 번 사용하는 경우도 있기에 구조적으로 큰 어색함은 없다는 생각이 들어 다음과 같은 방식으로 코드를 구현했습니다.

const {
title,
introduce,
book,
startDate,
endDate,
owner,
memberCount,
commentCount,
isPublic,
bookGroupId,
} = group;
return (
<Link
key={bookGroupId}
className="w-full"
href={`/group/${bookGroupId}`}
>
<DetailBookGroupCard
title={title}
description={introduce}
bookImageSrc={book.imageUrl}
date={{ start: startDate, end: endDate }}
owner={{
name: owner.nickname,
profileImageSrc: owner.profileUrl,
}}
memberCount={memberCount}
commentCount={commentCount}
isPublic={isPublic}
/>
</Link>
);
});
})}
</div>
</div>
<div ref={ref} />
{isFetchingNextPage && <Skeleton w="100%" height="28rem" />}
</VStack>
{/* <Link href={'/group/create'}>
<FloatingButton position="bottom-right" />
</Link> */}
</>
);
};

Expand Down
5 changes: 1 addition & 4 deletions src/stories/bookGroup/DetailBookGroupCard.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ export const Default: Story = {
title: '프롱이 리팩터링 스터디',
description:
'제1차 프롱이 기수연합 독서 스터디 입니다. 마틴 파울러의 저서 ‘리팩터링 2판’과 함께 진행합니다.',
book: {
title: '리팩터링 2판',
bookImageSrc: 'https://image.yes24.com/goods/89649360/XL',
},
bookImageSrc: 'https://image.yes24.com/goods/89649360/XL',
date: {
start: '2023-10-31',
end: '2023-11-27',
Expand Down
4 changes: 2 additions & 2 deletions src/stories/bookGroup/SearchGroup.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default meta;

type Story = StoryObj<typeof SearchGroup>;

const alertMessage = () => {
const handleClick = () => {
document.getElementById('groupSearching')?.blur();
alert(
`
Expand All @@ -22,5 +22,5 @@ const alertMessage = () => {
};

export const Default: Story = {
render: () => <SearchGroup handleClick={alertMessage} />,
render: () => <SearchGroup onClick={handleClick} />,
};
2 changes: 1 addition & 1 deletion src/ui/Base/FloatingButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const FloatingButton = ({ position, ...props }: FloatingButtonProps) => {

return createPortal(
<button
className={`absolute flex h-[5.1rem] w-[5.1rem] items-center justify-center rounded-full bg-main-900 ${positionClasses}`}
className={`${positionClasses} fixed left-[50%] top-[50%] flex h-[5.1rem] w-[5.1rem] -translate-x-1/2 -translate-y-1/2 items-center justify-center rounded-full bg-main-900`}
{...props}
>
<IconPlus />
Expand Down
4 changes: 2 additions & 2 deletions src/v1/book/BookCover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type BookCoverSize =
type BookCoverProps = Required<
Pick<ComponentPropsWithoutRef<typeof Image>, 'src'>
> & {
title: string;
title?: string;
size?: BookCoverSize;
};
Comment on lines 14 to 19
Copy link
Member

Choose a reason for hiding this comment

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

p5;
title prop 은 <img /> 요소에서 alt 속성을 위해 사용되는 것으로 보이네요. title은 책 이름을 화면에서 표시하기 위한 prop으로 보여질 수 있다고 생각해서 alt 로 변경해도 좋을 것 같다고 생각해요.

Copy link
Member Author

Choose a reason for hiding this comment

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

@minjongbaek
ask;
좋은 의견 감사합니다! 궁금한 점이 있는데 민종님은 보통 prop들을 정의할때 받아오는 prop의 상태(?) 혹은 prop의 내용(?)을 기준으로 prop의 이름을 정하는것이 아니라 해당 prop을 받는 컴포넌트를 기준으로 prop을 받는 컴포넌트에서 어떻게 활용되는지를 기준으로 prop이름들을 정하시는 것을 선호하시는 것 같은데? 저 또한 좋은 관점이라는 생각이 들었는데 어떤 이유에서 그런 생각을 하시게 된 건지 여쭤봐도 될까요?? 레퍼런스나 책, 혹은 조언을 들은 건가요?? 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

@minjongbaek
아 참고로 BookCover 컴포넌트의 경우 @gxxrxn 님이 구현을 해주셨는데 같이 디스코드를 통해서 작업을 하던 중에 제가 일부 옵셔널로 변경을 한 사항이라 규란님의 의견도 함께 들어보고 결정하면 좋을 것 같습니다.

@gxxrxn @minjongbaek

Copy link
Member

Choose a reason for hiding this comment

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

좋은 의견 감사합니다! 궁금한 점이 있는데 민종님은 보통 prop들을 정의할때 받아오는 prop의 상태(?) 혹은 prop의 내용(?)을 기준으로 prop의 이름을 정하는것이 아니라 해당 prop을 받는 컴포넌트를 기준으로 prop을 받는 컴포넌트에서 어떻게 활용되는지를 기준으로 prop이름들을 정하시는 것을 선호하시는 것 같은데? 저 또한 좋은 관점이라는 생각이 들었는데 어떤 이유에서 그런 생각을 하시게 된 건지 여쭤봐도 될까요?? 레퍼런스나 책, 혹은 조언을 들은 건가요?? 👀

@WooDaeHyun 따로 참고한 레퍼런스는 없습니다. prop이 HTML 요소에서 지원하는 속성을 위해 사용된다면 DX 측면에서 동일한 이름을 사용하는 것이 좋지 않을까? 라고 생각하는데 이런 생각이 코드에 반영되는 것 같아요. 이 부분은 주관적인 내용을 포함하고 있기에 '누구는 이런 기준으로 prop 명을 정하는구나.' 라고 봐주시면 좋을 것 같아요. 🙇

그렇기 때문에 코멘트에 p5를 적용했습니다. 만약 대현님께서 다른 생각을 가지고 있다면 공유해주시고, 제 피드백을 반영하지 않아도 좋아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

@minjongbaek
이 부분에서는 정말 코드 작성 스타일에 따라 의견이 나뉠 수 있을 것 같아서! 프로젝트에 참여하는 사람들의 의견을 모아 앞으로 작성되는 prop 명들을 어떤 부분을 기준으로 이름을 정할지 의견을 나눠보면 좋을 것 같아요 👀

Copy link
Member

Choose a reason for hiding this comment

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

고민해봤는데 저는 이미지를 보여줄 수 없을 때 alt 값을 보여주는만큼 BookCover 컴포넌트에서는 title이라는 정확한 값을 받는 것이 더 좋은 것 같다는 생각이 들어요!


Expand Down Expand Up @@ -48,7 +48,7 @@ const BookCover = ({ src, title, size = 'medium' }: BookCoverProps) => {
<span className={`relative ${sizeClasses}`}>
<Image
src={src}
alt={title}
alt={title || 'book-cover'}
placeholder="blur"
blurDataURL={DATA_URL['placeholder']}
className="object-fit rounded-[0.5rem] shadow-bookcover"
Expand Down
71 changes: 23 additions & 48 deletions src/v1/bookGroup/DetailBookGroupCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@ import Image from 'next/image';

import Badge from '@/ui/Base/Badge';
import { IconCalendar, IconMembers, IconComments } from '@public/icons';
import BookCover from '@/v1/book/BookCover';

interface DetailBookGroupCardProps {
title: string;
description: string;
book: { title: string; bookImageSrc: string };
bookImageSrc: string;
date: { start: string; end: string };
owner: { name: string; profileImageSrc: string };
owner: { name: string | null; profileImageSrc: string };
memberCount: number;
commentCount: number;
isPublic: boolean;
handleClick: () => void;
handleClick?: () => void;
Copy link
Member

Choose a reason for hiding this comment

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

p1;
지난 번 정리한 컨벤션을 따라 onClick 으로 작성하는건 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 생각입니다. 다른 컴포넌트 handleClick으로 작성된 부분을 일부 수정했었는데 여기도 있었네요! 매의 눈 감사합니다 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

이 부분 수정했었으나 Link 태그로 전환하면서 onClick 이벤트는 제거했습니다. 참고 부탁드립니다~

}

type BookGroupStatus = 'before' | 'dday' | 'ongoing' | 'end';
Expand Down Expand Up @@ -45,7 +46,7 @@ const toDayFromMillseconds = (value: number) => {
const DetailBookGroupCard = ({
title,
description,
book,
bookImageSrc,
date,
owner,
memberCount,
Expand All @@ -65,31 +66,26 @@ const DetailBookGroupCard = ({
return (
<div
onClick={handleClick}
className="h-[16.142rem] w-[35.5rem] rounded-[0.4rem] px-[1.6rem] py-[0.9rem] shadow-[0_0_0.6rem_rgba(180,180,180,0.25)]"
className="min-h-[16.142rem] w-full rounded-[0.4rem] px-[1.6rem] py-[0.9rem] shadow-[0_0_0.6rem_rgba(180,180,180,0.25)]"
>
<div className="mb-[1rem] flex gap-[0.5rem]">
<Dday {...ddayProps}></Dday>
<Public isPublic={isPublic}></Public>
<Dday {...ddayProps} />
<Public isPublic={isPublic} />
</div>
<div className="flex gap-[1.4rem]">
<div className="flex flex-col gap-[0.63rem]">
<Title title={title}></Title>
<Description description={description}></Description>
<Duration start={date.start} end={date.end}></Duration>
<div className="flex w-[22.5rem] justify-between">
<Owner
name={owner.name}
profileImageSrc={owner.profileImageSrc}
></Owner>
<div className="flex justify-between gap-[1.5rem]">
<div className="flex flex-grow flex-col gap-[0.63rem]">
<Title title={title} />
<Description description={description} />
<Duration start={date.start} end={date.end} />
<div className="flex justify-between">
<Owner name={owner.name} profileImageSrc={owner.profileImageSrc} />
<div className="flex gap-[0.5rem]">
<MemberCount memberCount={memberCount}></MemberCount>
<CommentCount commentCount={commentCount}></CommentCount>
<MemberCount memberCount={memberCount} />
<CommentCount commentCount={commentCount} />
</div>
</div>
</div>
<div>
<Book title={book.title} bookImageSrc={book.bookImageSrc}></Book>
</div>
<BookCover src={bookImageSrc} size="medium" />
</div>
</div>
);
Expand Down Expand Up @@ -158,7 +154,7 @@ const Title = ({ title }: { title: string }) => {

const Description = ({ description }: { description: string }) => {
return (
<div className="w-[22.5rem] truncate text-sm">
<div className="w-[22rem] truncate text-sm">
<span>{description}</span>
</div>
);
Expand All @@ -183,14 +179,16 @@ const Owner = ({
name,
profileImageSrc,
}: {
name: string;
name: string | null;
Copy link
Member

@minjongbaek minjongbaek Nov 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

ask;
name 이 모임장의 이름을 나타내는 것으로 보이는데 null인 경우가 있나요? 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

@minjongbaek
해당 부분에서 type을 정의하는 과정에서 name prop만 유일하게 string | null type의 경우 string 타입을 할당할 수 없다는 에러메세지를 지속적으로 받아 답답함에 그만.. null을 넣어버렸습니다.. 이 부분은 수정이 필요해요... 도와줘..😭

Copy link
Member

@minjongbaek minjongbaek Nov 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@WooDaeHyun name prop 의 타입을 string 으로 지정하게 되면 string | null 은 string 타입에 할당할 수 없다. 는 에러메시지가 출력된다는 말씀이신가요?

제가 이해한 내용이 맞다면, 7번 라인에서 선언한 DetailBookGroupCardProps 인터페이스를 보면 owner.name 의 타입을 nullable 하지 않도록 수정해야 할 것 같아요 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

@minjongbaek
네 맞아요! 말씀 주신 내용까지는 접근을 했었는데 동일한 데이터를 받아오고 있는데 owner.name만 null인 경우 있을 수 있다라는 에러가 발생해서 거기서 막혔습니다. 🥹 처음부터 자세하게 설명을 드렸어야 했는데 죄송합니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

@minjongbaek

type APIGroupOwner = {
  id: APIUser['userId'];
  profileUrl: APIUser['profileImage'];
  // FIXME nickname: APIUser['nickname'] nullable 하지 않게 수정 후 다시 반영
  nickname: string;
};

현재 APIGroupOwner에서 정의된 owner.name 값이 nullable 한 상태여서 발생한 Error임을 인지하였고 해당 부분을 명시적으로 string 타입으로 지정하여 해당 부분을 수정하기는 했습니다. 확인 부탁드려요~

Copy link
Member

Choose a reason for hiding this comment

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

types/group.ts 파일 안에 APIGroupOwner.nickname이 APIUser['nickname'] 타입으로 정의되어있는데, 해당 타입이 nullable해요! 하지만 api 명세를 보면 nickname 프로퍼티는 nullable하지 않은 값이기 때문에 제 pr에서도 APIUser 타입을 전체적으로 수정하는 작업을 했어요! 우선은 APIGroupOwner.nickname 타입을 string 으로 수정하고, 추후 제 pr이 머지된 후에 다시 APIUser['nickname'] 타입으로 수정하면 해결할 수 있을 것 같아요!

profileImageSrc: string;
}) => {
return (
<div className="flex h-[2rem] gap-[0.5rem]">
{/* 아바타 컴포넌트로 변경 예정 */}
<div className={'relative h-[2rem] w-[2rem] rounded-full bg-black-400'}>
{profileImageSrc && <Image alt={name} src={profileImageSrc} fill />}
{profileImageSrc && (
<Image alt={name ? name : ''} src={profileImageSrc} fill />
)}
</div>
<div className="flex items-center text-xs">
<span>{name}</span>
Expand Down Expand Up @@ -224,26 +222,3 @@ const CommentCount = ({ commentCount }: { commentCount: number }) => {
</div>
);
};

const Book = ({
bookImageSrc,
title,
}: {
bookImageSrc: string;
title: string;
}) => {
return (
<div>
<div className="relative h-[10.442rem] w-[8rem]">
{bookImageSrc && (
<Image
src={bookImageSrc}
alt={title}
fill
className="object-fit rounded-[0.4rem] shadow-[0.1rem_0.2rem_0.4rem_0_rgba(0,0,0,0.25)]"
/>
)}
</div>
</div>
);
};
8 changes: 4 additions & 4 deletions src/v1/bookGroup/SearchGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import { IconSearch } from '@public/icons';

interface SearchGroup {
handleClick: () => void;
onClick: () => void;
}

const SearchGroup = ({ handleClick }: SearchGroup) => {
const SearchGroup = ({ onClick }: SearchGroup) => {
return (
<div className="flex">
<div className="rounded-bl-[0.4rem] rounded-tl-[0.4rem] border-[0.1rem] border-r-[0rem] border-solid border-black-100 bg-[#fffff] pl-[1rem] pt-[0.8rem]">
<IconSearch fill="#AFAFAF" />
</div>
<input
id="groupSearching"
className="placeholder:text-Placeholder h-[3.7rem] w-full rounded-br-[0.4rem] rounded-tr-[0.4rem] border-[0.1rem] border-l-[0rem] border-black-100 pl-[2rem] text-[1.4rem] leading-[1.6rem] focus:outline-0"
className="h-[3.7rem] w-full rounded-br-[0.4rem] rounded-tr-[0.4rem] border-[0.1rem] border-l-[0rem] border-black-100 pl-[2rem] text-[1.4rem] leading-[1.6rem] placeholder:text-placeholder focus:outline-0"
placeholder="모임을 검색해보세요"
type="text"
onClick={handleClick}
onClick={onClick}
/>
</div>
);
Expand Down
Loading