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

[Fix] - 6차 피드백 반영(시모) #545

Merged
merged 16 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
20,769 changes: 20,769 additions & 0 deletions frontend/package-lock.json

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@
"webpack-bundle-analyzer": "^4.10.2",
"webpack-cli": "^5.1.4",
"webpack-dev-server": "^5.0.4",
"webpack-merge": "^6.0.1"
"webpack-merge": "^6.0.1",
"xml2js": "^0.6.2"
},
"contributors": [
{
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/assets/svg/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ export { default as Plus } from "./plus.svg";
export { default as tturiUrl } from "./tturi.svg?url";
export { default as MapIcon } from "./map-icon.svg";
export { default as HomeIcon } from "./home-icon.svg";

export { default as ResetIcon } from "./reset-icon.svg";
export { default as SortIcon } from "./sort-icon.svg";
3 changes: 3 additions & 0 deletions frontend/src/assets/svg/reset-icon.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions frontend/src/assets/svg/sort-icon.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
30 changes: 28 additions & 2 deletions frontend/src/components/common/Chip/Chip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,51 @@ import { ComponentPropsWithoutRef } from "react";

import { CYPRESS_DATA_MAP } from "@constants/cypress";

import theme from "@styles/theme";

import Icon from "../Icon/Icon";
import SVG_ICONS_MAP from "../Icon/svg-icons.json";
import Text from "../Text/Text";
import * as S from "./Chip.styled";

interface ChipProps extends ComponentPropsWithoutRef<"li"> {
label: string;
isSelected?: boolean;
index?: number;
iconPosition?: "none" | "left" | "right";
iconType?: keyof typeof SVG_ICONS_MAP;
}

const Chip = ({ label, isSelected = false, index, children, ...props }: ChipProps) => {
const Chip = ({
label,
isSelected = false,
index,
iconPosition = "none",
iconType = "down-arrow",
...props
}: ChipProps) => {
return (
<S.Layout
$isSelected={isSelected}
$index={index}
data-cy={isSelected ? `selected-${CYPRESS_DATA_MAP.chip}` : CYPRESS_DATA_MAP.chip}
{...props}
>
{iconPosition === "left" && (
<Icon
iconType={iconType}
size="8"
color={isSelected ? theme.colors.primary : theme.colors.text.secondary}
/>
)}
<Text textType={isSelected ? "detailBold" : "detail"}>{label}</Text>
{children}
{iconPosition === "right" && (
<Icon
iconType={iconType}
size="8"
color={isSelected ? theme.colors.primary : theme.colors.text.secondary}
/>
)}
</S.Layout>
);
};
Expand Down
18 changes: 18 additions & 0 deletions frontend/src/components/common/Icon/svg-icons.json
Original file line number Diff line number Diff line change
Expand Up @@ -286,5 +286,23 @@
"strokeWidth": "2",
"strokeLinecap": "round",
"strokeLinejoin": "round"
},
"reset-icon": {
"width": 1920,
"height": 1920,
"path": "M960 0v213.333c411.627 0 746.667 334.934 746.667 746.667S1371.627 1706.667 960 1706.667 213.333 1371.733 213.333 960c0-197.013 78.4-382.507 213.334-520.747v254.08H640V106.667H53.333V320h191.04C88.64 494.08 0 720.96 0 960c0 529.28 430.613 960 960 960s960-430.72 960-960S1489.387 0 960 0",
"stroke": "currentColor",
"strokeWidth": "1",
"strokeLinecap": "butt",
"strokeLinejoin": "miter"
},
"sort-icon": {
"width": 24,
"height": 24,
"path": "M6.293 4.293a1 1 0 0 1 1.414 0l4 4a1 1 0 0 1-1.414 1.414L8 7.414V19a1 1 0 1 1-2 0V7.414L3.707 9.707a1 1 0 0 1-1.414-1.414l4-4zM16 16.586V5a1 1 0 1 1 2 0v11.586l2.293-2.293a1 1 0 0 1 1.414 1.414l-4 4a1 1 0 0 1-1.414 0l-4-4a1 1 0 0 1 1.414-1.414L16 16.586z",
"stroke": "currentColor",
"strokeWidth": "1",
"strokeLinecap": "butt",
"strokeLinejoin": "miter"
}
}
72 changes: 52 additions & 20 deletions frontend/src/components/pages/main/MainPage.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { useEffect, useMemo } from "react";

import useInfiniteTravelogues from "@queries/useInfiniteTravelogues";

import {
Expand Down Expand Up @@ -32,13 +34,38 @@ import * as S from "./MainPage.styled";
import TravelogueCardSkeleton from "./TravelogueCard/skeleton/TravelogueCardSkeleton";

const MainPage = () => {
const { selectedTagIDs, handleClickTag, sortedTags, animationKey } = useMultiSelectionTag(
STORAGE_KEYS_MAP.mainPageSelectedTagIDs,
);
const { sorting, travelPeriod } = useSingleSelectionTag(
STORAGE_KEYS_MAP.mainPageSort,
STORAGE_KEYS_MAP.mainPageTravelPeriod,
);
const {
selectedTagIDs,
handleClickTag,
sortedTags,
multiSelectionTagAnimationKey,
resetMultiSelectionTag,
} = useMultiSelectionTag(STORAGE_KEYS_MAP.mainPageSelectedTagIDs);

const {
sorting,
travelPeriod,
resetSingleSelectionTags,
singleSelectionAnimationKey,
increaseSingleSelectionAnimationKey,
} = useSingleSelectionTag(STORAGE_KEYS_MAP.mainPageSort, STORAGE_KEYS_MAP.mainPageTravelPeriod);

const isTagsSelected = useMemo(() => {
return (
selectedTagIDs.length !== 0 ||
sorting.selectedOption !== "likeCount" ||
travelPeriod.selectedOption !== ""
);
}, [selectedTagIDs, sorting.selectedOption, travelPeriod.selectedOption]);

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

아마 useMemo로 감싸신 이유가 useEffect의 의존성 배열에 들어가서 참조 값을 고정하기 위해 사용한걸로 파악되는데, 의존성 배열에 isTagsSelected를 추가한 이유를 알 수 있을까요?!

image 제 린트에선 오히려 `increaseSingleSelectionAnimationKey`가 빠졌다고 메시지가 보여서요!

Copy link
Contributor

Choose a reason for hiding this comment

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

const isTagsSelected =
  selectedTagIDs.length !== 0 ||
  sorting.selectedOption !== "likeCount" ||
  travelPeriod.selectedOption !== "";

const handleClickResetButton = () => {
  resetMultiSelectionTag();
  resetSingleSelectionTags();
  increaseSingleSelectionAnimationKey();
};

자세한 코드는 모르지만, effect와 memo를 사용하지 않고, handleClickResetButton에 increaseSingleSelectionAnimationKey를 추가해도 동일하게 동작하니 참고 부탁드려요!

Copy link
Author

Choose a reason for hiding this comment

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

우선 의존성 부분은 useCallback에 대해서 잘 모르고 있었다 보니 저 부분에 린트 문제가 생긴 거 같아요 해당 부분은 수정하였습니다! 위의 코드의 의도는 초기화 버튼 생성 유무에 따라 위 버튼들에 에니메이션을 주고싶었어요. 그냥 띡 생기는 것이 버그 같다는 예전 피드백을 근거로 해서 해당 부분을 추가했습니다. 그래서 초기화 버튼을 누를 때도 animationKey를 증가시켜 에니메이션을 발생하기도 하지만 초기화 버튼이 사라질 때도 해당 부분이 있으면 좋겠다고 생각하였어요. 그 결과 tag의 선택 여부에 따라 즉, isTagsSelected 의 값이 바뀔 때 마다 에니메이션이 발생하게 했습니다!

increaseSingleSelectionAnimationKey();
}, [isTagsSelected, increaseSingleSelectionAnimationKey]);

const handleClickResetButton = () => {
resetMultiSelectionTag();
resetSingleSelectionTags();
};

const { travelogues, status, fetchNextPage, isPaused, error } = useInfiniteTravelogues({
selectedTagIDs,
Expand Down Expand Up @@ -74,30 +101,35 @@ const MainPage = () => {

<S.TagsContainer>
<S.SingleSelectionTagsContainer>
{isTagsSelected && (
Copy link
Contributor

Choose a reason for hiding this comment

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

태그가 선택되었을 때, 초기화가 활성화 되는데 그거보단 처음부터 초기화 영역이 있는게 자연스럽다고 생각해요! 시모의 의견은 어떠신가요?!

Copy link
Author

Choose a reason for hiding this comment

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

이 부분 말했듯 배민을 레퍼런스로 참고했습니다! 초기화 버튼이 생김에 따라 이목을 끌 수 있다는 점 이로 인하여 사용자가 초기화 버튼 존재 여부에 대해 인식할 수 있다는 점을 근거로 해당 방식을 채택하였습니다. 또한 초기화가 되어 있는 가운데 초기화 버튼이 활성화 되어 있다는 것이 오히려 불 필요한 이벤트를 발생시킬 수 있을 거 같아 해당 방법으로 구현하였습니다! 좋은 의견 고마워요

Copy link
Contributor

Choose a reason for hiding this comment

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

의도가 뚜렷해서 좋네요 :) 감사합니다 ㅎㅎ

<Chip
key={`reset-${singleSelectionAnimationKey}`}
label={`초기화`}
isSelected={false}
onClick={handleClickResetButton}
iconPosition="left"
iconType="reset-icon"
/>
)}
<Chip
key={`sorting-${singleSelectionAnimationKey}`}
label={SORTING_OPTIONS_MAP[sorting.selectedOption]}
isSelected={true}
onClick={sorting.handleOpenModal}
>
<Icon iconType="down-arrow" size="8" color={theme.colors.primary} />
</Chip>
iconPosition="left"
iconType="sort-icon"
/>
<Chip
key={`travelPeriod-${singleSelectionAnimationKey}`}
label={
travelPeriod.selectedOption
? TRAVEL_PERIOD_OPTIONS_MAP[travelPeriod.selectedOption]
: "여행 기간"
}
iconPosition="right"
isSelected={travelPeriod.selectedOption !== ""}
onClick={travelPeriod.handleOpenModal}
>
<Icon
iconType="down-arrow"
size="8"
color={
travelPeriod.selectedOption ? theme.colors.primary : theme.colors.text.secondary
}
/>
</Chip>
/>
</S.SingleSelectionTagsContainer>

<S.MultiSelectionTagsContainer
Expand All @@ -108,7 +140,7 @@ const MainPage = () => {
>
{sortedTags.map((tag, index) => (
<Chip
key={`${tag.id}-${animationKey}`}
key={`${tag.id}-${multiSelectionTagAnimationKey}`}
index={index}
label={tag.tag}
isSelected={selectedTagIDs.includes(tag.id)}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/components/pages/my/MyPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,12 @@ const MyPage = () => {
onClose={editModal.handleCloseEditModal}
>
<S.Button onClick={profileImage.handleClickProfileImageEditButton}>
<Text textType="detail">앨범에서 선택</Text>
<Text textType="detail">프로필 사진 올리기</Text>
</S.Button>
{profileImage.profileImageUrl && (
<S.Button onClick={profileImage.handleClickProfileImageDeleteButton}>
<Text textType="detailBold" css={S.deleteTextColor}>
프로필 사진 삭제
프로필 사진 삭제하기
</Text>
</S.Button>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ const TravelPlanDetailPage = () => {

<TransformFooter
onTransform={handleTransform}
buttonLabel="여행기로 전환"
buttonLabel="여행기로 남기기"
guideMessage="여행은 즐겁게 다녀오셨나요?"
/>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ const TravelogueDetailPage = () => {

<TransformFooter
guideMessage="이 여행기를 따라가고 싶으신가요?"
buttonLabel="여행 계획으로 전환"
buttonLabel="여행 계획으로 가져오기"
Copy link

Choose a reason for hiding this comment

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

확실히 순한국말로 바꾸니깐 더 편안하고 좋은거같네요 ㅎㅎ

onTransform={handleTransform}
>
<S.LikesContainer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ const TravelogueEditPage = () => {
const [isOpen, handleOpenBottomSheet, handleCloseBottomSheet] = useToggle();

const {
state: { title, thumbnail, travelogueDays, selectedTagIDs, sortedTags, animationKey },
state: {
title,
thumbnail,
travelogueDays,
selectedTagIDs,
sortedTags,
multiSelectionTagAnimationKey,
},
handler: {
handleChangeTitle,
handleInitializeThumbnail,
Expand Down Expand Up @@ -105,7 +112,7 @@ const TravelogueEditPage = () => {
>
{sortedTags.map((tag, index) => (
<Chip
key={`${tag.id}-${animationKey}`}
key={`${tag.id}-${multiSelectionTagAnimationKey}`}
index={index}
label={tag.tag}
isSelected={selectedTagIDs.includes(tag.id)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,14 @@ const TravelogueRegisterPage = () => {
const { transformDetail } = useTravelTransformDetailContext();

const {
state: { title, thumbnail, travelogueDays, selectedTagIDs, sortedTags, animationKey },
state: {
title,
thumbnail,
travelogueDays,
selectedTagIDs,
sortedTags,
multiSelectionTagAnimationKey,
},
handler: {
handleChangeTitle,
handleChangeThumbnail,
Expand Down Expand Up @@ -101,7 +108,7 @@ const TravelogueRegisterPage = () => {
>
{sortedTags.map((tag, index) => (
<Chip
key={`${tag.id}-${animationKey}`}
key={`${tag.id}-${multiSelectionTagAnimationKey}`}
index={index}
label={tag.tag}
isSelected={selectedTagIDs.includes(tag.id)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ const useTravelogueFormState = (transformDays: TravelTransformDays[]) => {
const { title, handleChangeTitle } = useTravelogueTitle(transformDays);
const { thumbnail, handleChangeThumbnail, handleResetThumbnail, handleInitializeThumbnail } =
useTravelogueThumbnail();
const { selectedTagIDs, sortedTags, animationKey, handleClickTag, handleChangeSelectedTagIDs } =
useMultiSelectionTag();
const {
selectedTagIDs,
sortedTags,
multiSelectionTagAnimationKey,
handleClickTag,
handleChangeSelectedTagIDs,
} = useMultiSelectionTag();
const {
travelogueDays,
handleAddDay,
Expand All @@ -30,7 +35,7 @@ const useTravelogueFormState = (transformDays: TravelTransformDays[]) => {
travelogueDays,
selectedTagIDs,
sortedTags,
animationKey,
multiSelectionTagAnimationKey,
},
handler: {
handleChangeTitle,
Expand Down
18 changes: 15 additions & 3 deletions frontend/src/hooks/useMultiSelectionTag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ const useMultiSelectionTag = (key?: string) => {
const [selectedTagIDs, setSelectedTagIDs] = useState<number[]>(
key ? JSON.parse(localStorage.getItem(key) ?? "[]") : [],
);
const [animationKey, setAnimationKey] = useState(0);
const [multiSelectionTagAnimationKey, setMultiSelectionTagAnimationKey] = useState(0);

const increaseMultiSelectionTagAnimationKey = () => {
setMultiSelectionTagAnimationKey((prev) => prev + 1);
};

const handleChangeSelectedTagIDs = useCallback((newSelectedTagIDs: number[]) => {
setSelectedTagIDs(newSelectedTagIDs);
Expand All @@ -36,7 +40,7 @@ const useMultiSelectionTag = (key?: string) => {
if (isTagIDsSelectedMax && key) localStorage.setItem(key, JSON.stringify(prevSelectedTagIDs));
if (isTagIDsSelectedMax) return prevSelectedTagIDs;

setAnimationKey((prev) => prev + 1);
increaseMultiSelectionTagAnimationKey();

if (key) {
localStorage.setItem(key, JSON.stringify(newSelectedTagIDs));
Expand All @@ -47,12 +51,20 @@ const useMultiSelectionTag = (key?: string) => {
});
};

const resetMultiSelectionTag = () => {
setSelectedTagIDs([]);
if (key) localStorage.setItem(key, JSON.stringify([]));

increaseMultiSelectionTagAnimationKey();
};

return {
selectedTagIDs,
handleChangeSelectedTagIDs,
sortedTags: createSortedTags(),
handleClickTag,
animationKey,
multiSelectionTagAnimationKey,
resetMultiSelectionTag,
};
};

Expand Down
Loading
Loading