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/#315 card blacklist #321

Merged
merged 16 commits into from
Aug 17, 2023
Merged

Feat/#315 card blacklist #321

merged 16 commits into from
Aug 17, 2023

Conversation

ilmerry
Copy link
Member

@ilmerry ilmerry commented Aug 15, 2023

  • 브랜치명, 브랜치 알맞게 설정
  • Reviewer, Assignees, Label, Milestone, Issue(PR 작성 후에) 붙이기
  • PR이 승인된 경우 해당 브랜치는 삭제하기

📌 내용

  • useBlacklist를 사용해 카드 블락 기능을 수행합니다.
    • blacklist 원래 로직대로라면, 블랙리스트 추가/삭제를 수행한 후 카드를 다시 mutate시켜, 블락이 적용된 카드를 다시 받아와야하지만, 기존 로직이 리패치하면 랜덤으로 새로운 카드 리스트 30장을 받아오게 구현되어있기 때문에, mutate하지 못하는 상황입니다. 따라서 blacklist state에 블락된 카드 id를 추가해, 블락된 카드이면(blacklist.includes(_id)이면) 블락용 UI를 보여주어 optimistic update 하고 있습니다.
    • handleClickAddBlacklist 카드를 블랙리스트에 추가, 다음 카드로 자동넘김, blacklist에 카드 id 추가, showToast 등을 수행합니다.
    • handleClickCancelBlacklist 카드를 블랙리스트에서 제외, 취소 카드로 자동넘김, blacklist에서 id 제외, blackoutToast 등을 수행합니다.
  • useCardSwiperautoSlide를 추가합니다. 이전 카드로 되돌아가는 기능을 수행합니다.
  • ToastProviderblackoutToast를 추가합니다. 블랙리스트 취소를 누르는 즉시 토스트 메시지가 사라지는 역할을 합니다.

⚠️ 업데이트

  • blacklist 를 export하던 기존방식에서, getIsBlacklist 함수를 export하도록 변경하였습니다. card id를 반환하면, blacklist 여부를 판단해 반환합니다.

📸 스크린샷

image

@ilmerry ilmerry requested review from joohaem and NYeonK August 15, 2023 05:45
@ilmerry ilmerry self-assigned this Aug 15, 2023
@ilmerry ilmerry marked this pull request as draft August 15, 2023 12:16
@ilmerry ilmerry marked this pull request as ready for review August 15, 2023 12:35
Copy link
Contributor

@NYeonK NYeonK left a comment

Choose a reason for hiding this comment

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

수고하셨습니다💘✨

저는 코드도 이해 잘되고 좋았아요! 블랙리스트 기능 구현할 때, 타 파트와 소통이 많았을 텐데 넘넘 고생하셨어요! 👊

Comment on lines +33 to +36
<St.ToastMessage>
{toast.message}
{toast.handleClickCancel && <St.CancelButton onClick={toast.handleClickCancel}>취소</St.CancelButton>}
</St.ToastMessage>
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
Member Author

Choose a reason for hiding this comment

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

네! 정확히는 메시지 오른쪽의 취소버튼입니다! toast.handleClickCancel을 달아둔 상태예요

Copy link
Contributor

Choose a reason for hiding this comment

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

엇 아니요! 토스트 메세지가 생겼을 때, 2.5초가 안지나더라도
메세지를 클릭하면 사라지게 만들어야 한다는 말이었어용!

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
Contributor

Choose a reason for hiding this comment

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

image
기능명세서에는 이렇게 나와있어서 이야기 했어요! 변경이 안됐나보네요🤣

@@ -10,7 +15,7 @@ const useBlacklist = (handleClickBeforeLogin: () => void) => {
// onSuccess: onSuccessAdd,
// });

const handleClickAddBlacklist = (_id: string, onSuccessAdd: () => void) => {
const handleClickAddBlacklist = ({ _id, onSuccess: onSuccessAdd }: handleClickParams) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

객체화 구웃👍👍

Comment on lines +30 to +33
const autoSlide: autoSlideType = {
slideDown: () => swiperRef.current?.swiper.slideTo(sliderIdx + 1),
slideUp: () => swiperRef.current?.swiper.slideTo(sliderIdx),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

슬라이드 자동으로 이렇게 넘겼구나! 라이브러리 사용 너무 잘해😆😆

Comment on lines +26 to +37
const onSuccessAddBlacklist = () => {
closeHandler();
showToast({
message: "🚫 해당 대화주제가 더 이상 추천되지 않아요",
duration: 3.5,
handleClickCancel: () => {
handleClickCancelBlacklist({ _id: currentCardId, onSuccess: blackoutToast });
autoSlide.slideUp();
},
});
autoSlide.slideDown();
};
Copy link
Contributor

Choose a reason for hiding this comment

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

ModalItems이 너무 길어져서 볼 때 힘들었는데, 따로 빼내니까 좋네요👏

Copy link
Member Author

Choose a reason for hiding this comment

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

다행이네요! 이 함수도 보기 힘들까 걱정됐는데, 괜찮다니 다행이에요!

Comment on lines 74 to 78
useEffect(() => {
if (location.search.includes(LocationType.BEST)) setIsBlockShow(false);
else if (location.search.includes(LocationType.MEDLEY)) setIsBlockShow(false);
else setIsBlockShow(true);
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

if랑 else if문 합치는 거 어떻게 생각하세용?

Copy link
Contributor

Choose a reason for hiding this comment

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

// 파라미터로 보이지 않아야할 locationType 전달
export default function useShowByCardType(locationTypes: LocationType[]) {
    const [isShow, setIsShow] = useState<boolean>(false);

    useEffect(() => {
       const cardType = new URLSearchParams(window.location.search.split("?")[1]).get("type");
       setIsShow(!locationTypes.includes(cardType || ""));
    }, []);

    return { isShow };
}

Last Card PR에서 리뷰했었던 요 부분을 현재 코드로 리팩토링한건가요!?

Copy link
Member Author

Choose a reason for hiding this comment

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

아직 리팩토링 안한 상태입니다! Last Card 합쳐지면 같이 리팩토링 하려고 했어요

Copy link
Contributor

Choose a reason for hiding this comment

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

ㅎㅎ 저도 blacklist 합쳐지면, 리팩토링할라구 기다리는 중이었어요😋😋

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
Contributor

Choose a reason for hiding this comment

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

넵~! 머지되면 호딱 반영할게요:)

@NYeonK NYeonK linked an issue Aug 16, 2023 that may be closed by this pull request
4 tasks
@ilmerry ilmerry merged commit fb58837 into release/2.1.0 Aug 17, 2023
@ilmerry ilmerry deleted the feat/#315-card_blacklist branch August 17, 2023 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ CardCollection - Blacklist ] 주제 다시 안보기 기능 구현
2 participants