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/#333 refactor custom hook #334

Open
wants to merge 17 commits into
base: release/2.1.1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
55 changes: 0 additions & 55 deletions src/@components/@common/hooks/useDraggableYContainer.ts

This file was deleted.

99 changes: 74 additions & 25 deletions src/@components/@common/hooks/useDraggingContainer.ts
Original file line number Diff line number Diff line change
@@ -1,60 +1,109 @@
import { useRef, useState } from "react";

export default function useDraggingContainer() {
type DragDirectionType = "X" | "Y";
Copy link
Member

Choose a reason for hiding this comment

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

'horizon'도 아닌, 'x'도 아닌, 'X'인 이유가 있을까요?
추상화 레벨 적절하게 열어진 것 다 좋은데, 이 인자값 인터페이스가 조금 어색해보여요
타이핑도 되어있고 해서 지금 상태도 괜찮지만, 가능하다면 좀 더 일반적으로 기대 가능한 인터페이스면 어떨까 싶습니다!!!!!

Copy link
Member Author

Choose a reason for hiding this comment

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

읽는 사람들(개발자들)에게 x/y는 'x축', 'y축'이라는 통용된 개념으로 읽히기 때문에 horizon/vertical으로 굳이 길게 늘릴 필요가 없다고 생각했어요!

x/y와 X/Y 중 고민했는데, 속성명 중에 pageX, pageY가 있기 때문에 통일성을 주기 위해 X, Y를 사용했습니다.
x/y가 좀 더 가독성이 낫다고 느끼신다면 고쳐보겠습니다!


type EventMapperType = {
[key in DragDirectionType]: "pageX" | "pageY";
};

const eventMapper: EventMapperType = {
X: "pageX",
Y: "pageY",
};

const FIRST_TOUCH = 0;
Copy link
Member

Choose a reason for hiding this comment

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

more specific naming

Copy link
Member Author

@ilmerry ilmerry Oct 2, 2023

Choose a reason for hiding this comment

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

터치이벤트가 여러개 발생할 수 있는데, 그 중 첫번째만 사용하고 있으므로 이부분을 인덱스로 따로 뺌
=> FIRST_TOUCH_EVENT_IDX로 변경하겠습니다!


export default function useDraggingContainer(dragDirection: DragDirectionType) {
const containerRef = useRef<HTMLElement | null>(null);

const [isStartDragging, setIsStartDragging] = useState(false);
const currentX = useRef(0);
const currentRef = useRef(0);
const standardRef = useRef(0);

const standardX = useRef(0);
const [draggedX, setDraggedX] = useState(0);
const [isStartDragging, setIsStartDragging] = useState<boolean>(false);
const [isArrivedEnd, setIsArrivedEnd] = useState<boolean>(false);
const [dragged, setDragged] = useState<number>(0);

function handleMouseDown(event: React.MouseEvent<HTMLElement, MouseEvent>) {
function handleTriggerDown(event: React.SyntheticEvent<HTMLElement>) {
Copy link
Member

Choose a reason for hiding this comment

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

X, Y를 한 훅에서 처리하는 건 적절해보이는데,
마우스 동작과 터치 동작을 한 함수로 묶는 건 좀 과해보여요
이후에 마우스, 터치 로직들이 추가되고 한다면 함수가 하나의 동작을 못하게 되고 다시 분리하게 되는 현상이 일어날 것 같아서,
차라리 Touch Hanlder 함수를 따로 파주는 게 나을 것 같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 고민했던 부분인데, MouseDown과 TouchDown 핸들러의 로직이 완전히 동일하기도 해서, 훅 내부가 장황해지는 것을 우려했던 것 같아요. 그래도 아래와 같이 분리하는 게 좋을까요?

function handleMouseDown(event: React.MouseEvent<HTMLElement>) {
  setIsStartDragging(true);

  const page = getPageByEventType(event);
  currentRef.current = page;
  initializeForDragged(page);
}

function handleTouchDown(event: React.TouchEvent<HTMLElement>) {
  setIsStartDragging(true);

  const page = getPageByEventType(event);
  currentRef.current = page;
  initializeForDragged(page);
}

Copy link
Member

@joohaem joohaem Oct 6, 2023

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.

넵 분리하겠습니다! 감사해요:)

setIsStartDragging(true);

currentX.current = event.pageX;
const page = getPageByEventType(event);
currentRef.current = page;
initializeForDragged(page);
}

initializeForDraggedX(event.pageX);
function getPageByEventType(event: React.SyntheticEvent<HTMLElement>): number {
const eventType = eventMapper[dragDirection];
if (event.nativeEvent instanceof TouchEvent) {
return event.nativeEvent.touches[FIRST_TOUCH][eventType];
}
if (event.nativeEvent instanceof MouseEvent) {
return event.nativeEvent[eventType];
}
return 0;
}

function initializeForDraggedX(standartX: number) {
setDraggedX(0);
standardX.current = standartX;
function initializeForDragged(standard: number) {
setDragged(0);
standardRef.current = standard;
}

function handleMouseMove(event: React.MouseEvent<HTMLElement, MouseEvent>) {
function handleTriggerMove(event: React.SyntheticEvent<HTMLElement>) {
const container = containerRef.current;

if (!container) return;
if (!isStartDragging) return;

moveContainerByCurrentX(container, event.pageX);
const page = getPageByEventType(event);
moveContainerByCurrent(container, page);
handleArrivedEnd(container);

setDragged(Math.abs(page - standardRef.current));
}

setDraggedX(Math.abs(event.pageX - standardX.current));
function moveContainerByCurrent(container: HTMLElement, movedTrigger: number) {
const delta = currentRef.current - movedTrigger;
if (dragDirection === "X") {
container.scrollLeft += delta;
}
if (dragDirection === "Y") {
container.scrollTop += delta;
}
currentRef.current = movedTrigger;
}

function moveContainerByCurrentX(container: HTMLElement, movedMouseX: number) {
container.scrollLeft += currentX.current - movedMouseX;
currentX.current = movedMouseX;
function handleArrivedEnd(container: HTMLElement) {
if (dragDirection === "X") {
setIsArrivedEnd(container.scrollWidth - container.scrollLeft === container.clientWidth);
}
if (dragDirection === "Y") {
setIsArrivedEnd(container.scrollHeight - container.scrollTop === container.clientHeight);
}
}

function handleMouseUpOrLeave() {
function handleTriggerEnd() {
reset();
}

function reset() {
setIsStartDragging(false);
currentX.current = 0;
standardX.current = 0;
currentRef.current = 0;
standardRef.current = 0;
}

return {
scrollableContainerProps: {
ref: containerRef,
onMouseDown: handleMouseDown,
onMouseMove: handleMouseMove,
onMouseUp: handleMouseUpOrLeave,
onMouseLeave: handleMouseUpOrLeave,
onMouseDown: handleTriggerDown,
onMouseMove: handleTriggerMove,
onMouseUp: handleTriggerEnd,
onMouseLeave: handleTriggerEnd,

onTouchStart: handleTriggerDown,
onTouchMove: handleTriggerMove,
onTouchEnd: handleTriggerEnd,
onTouchCancel: handleTriggerEnd,
},
isDragging: draggedX > 10,
isDragging: dragged > 10,
isArrivedEnd,
};
}
130 changes: 71 additions & 59 deletions src/@components/@common/hooks/useDrawer.ts
Original file line number Diff line number Diff line change
@@ -1,95 +1,107 @@
import { useCallback, useRef, useState } from "react";
import { SyntheticEvent, useRef, useState } from "react";

const thresholds = {
offset: 150, // 이 이상 내려야 drawer 닫힘
transitionTime: 0.2, // 애니메이션 지속 시간
OFFSET: 150,
ANIMATION_TRANSITION_TIME: 0.2,
};

const FIRST_TOUCH = 0;

export default function useDrawer(closeModal: () => void) {
const knobRef = useRef<HTMLDivElement | null>(null);
const containerRef = useRef<HTMLElement | null>(null);

const [isDragging, setIsDragging] = useState(false);
const [dragStart, setDragStart] = useState({ y: 0 });
const [yOffset, setYOffset] = useState(0);
const currentRef = useRef(0);
const standardRef = useRef(0);

const [isStartDragging, setIsStartDragging] = useState<boolean>(false);
Copy link
Member

Choose a reason for hiding this comment

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

isDragging?

const [dragged, setDragged] = useState<number>(0);
Copy link
Member

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.

어떤 부분이 중복이죠?!

Copy link
Member

Choose a reason for hiding this comment

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

false, 0으로 초기값 설정하면 타입추론이 될텐데, boolean, number로 한 겹 더 타입을 지정하는 건 되려 휴먼에러를 일으키지 않을까 해서🤔 저는 추론 될 때는 타입 지정을 안 하는 편이에요

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.

more specific naming

Copy link
Member Author

Choose a reason for hiding this comment

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

draggedDistance로 변경하겠습니다!


function handleMouseDown(event: React.SyntheticEvent<HTMLElement>) {
const knob = knobRef.current;
if (!knob) return;
if (isNode(event.target) && !knob.contains(event.target)) return;
setIsStartDragging(true);

const page = getPageByEventType(event);
currentRef.current = page;
initializeForDragged(page);
}

const handleMouseDown = useCallback((e: React.MouseEvent<HTMLElement>) => {
if (knobRef.current && knobRef.current.contains(e.target as Node)) {
setIsDragging(true);
setDragStart({
y: e.clientY,
});
function isNode(target: EventTarget): target is Node {
return (target as Node) !== undefined;
}

function getPageByEventType(event: React.SyntheticEvent<HTMLElement>): number {
if (event.nativeEvent instanceof TouchEvent) {
return event.nativeEvent.touches[FIRST_TOUCH].pageY;
}
if (event.nativeEvent instanceof MouseEvent) {
return event.nativeEvent.pageY;
}
}, []);
return 0;
}

const handleMouseMove = useCallback(
(e: React.MouseEvent<HTMLElement>) => {
if (!isDragging) return;
function initializeForDragged(standard: number) {
setDragged(0);
standardRef.current = standard;
}

const offset = Math.max(0, e.clientY - dragStart.y);
function handleMouseMove(event: SyntheticEvent<HTMLElement>) {
const container = containerRef.current;

// 모달을 드래그한 만큼 이동
const container = containerRef.current;
if (!container) return;
container.style.transform = `translateY(${offset}px)`;
setYOffset(offset);
},
[dragStart.y, isDragging],
);
if (!container) return;
if (!isStartDragging) return;

const page = getPageByEventType(event);
currentRef.current = page;
moveDrawerByCurrent(container, page);

setDragged(Math.abs(page - standardRef.current));
}

function moveDrawerByCurrent(container: HTMLElement, movedTrigger: number) {
const offset = Math.max(0, movedTrigger - standardRef.current);
container.style.transform = `translateY(${offset}px)`;
currentRef.current = movedTrigger;
}

const handleMouseUp = useCallback(() => {
function handleMouseUp() {
const container = containerRef.current;
if (!container) return;

if (yOffset > 150) {
container.style.transition = `transform ${thresholds.transitionTime}s ease-in-out`;
if (dragged > thresholds.OFFSET) {
container.style.transition = `transform ${thresholds.ANIMATION_TRANSITION_TIME}s ease-in-out`;
container.style.transform = "translateY(100%)";
setTimeout(() => {
closeModal();
}, thresholds.transitionTime * 1000 + 50);
}, thresholds.ANIMATION_TRANSITION_TIME * 1000 + 50);
} else {
container.style.transition = `transform ${thresholds.transitionTime / 2}s ease-out`;
container.style.transition = `transform ${thresholds.ANIMATION_TRANSITION_TIME / 2}s ease-out`;
container.style.transform = "translateY(0)";
}
setIsDragging(false);
}, [yOffset, closeModal]);

/**
* For Mobile
* */

const handleTouchStart = useCallback((e: React.TouchEvent<HTMLElement>) => {
if (knobRef.current && knobRef.current.contains(e.target as Node)) {
setIsDragging(true);
setDragStart({
y: e.touches[0].clientY,
});
}
}, []);

const handleTouchMove = useCallback(
(e: React.TouchEvent<HTMLElement>) => {
if (!isDragging) return;

const offset = Math.max(0, e.touches[0].clientY - dragStart.y);
reset();
}

const container = containerRef.current;
if (!container) return;
container.style.transform = `translateY(${offset}px)`;
setYOffset(offset);
},
[dragStart.y, isDragging],
);
function reset() {
setIsStartDragging(false);
currentRef.current = 0;
standardRef.current = 0;
}

return {
drawerProps: {
ref: containerRef,
onMouseDown: handleMouseDown,
onMouseMove: handleMouseMove,
onMouseUp: handleMouseUp,
onTouchStart: handleTouchStart,
onTouchMove: handleTouchMove,
onMouseLeave: handleMouseUp,

onTouchStart: handleMouseDown,
onTouchMove: handleMouseMove,
onTouchEnd: handleMouseUp,
onTouchCancel: handleMouseUp,
},
knobRef,
};
Expand Down
17 changes: 0 additions & 17 deletions src/@components/@common/hooks/useShowByQuery.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const BEST_PIICKLE_TOTAL_COUNT = 4;

export default function RecommendItem(props: RecommendProps) {
const { recommendList } = props;
const { scrollableContainerProps, isDragging } = useDraggingContainer();
const { scrollableContainerProps, isDragging } = useDraggingContainer("X");

return (
<St.RecommemdItemContainer>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { CardList, LocationType } from "../../../types/cardCollection";
import { HeadingTitle } from "../../../util/main/headingTitles";
import HeadingTitleContainer from "../../@common/HeadingTitleContainer";
import { useRecentlyBookmarked } from "../../@common/hooks/useRecentlyBookmarked";
import { useCardsByGender } from "./hooks/useCardsByGender";
import { useCardsByGender } from "../hooks/useCardsByGender";
import { useRecentlyBookmarked } from "../hooks/useRecentlyBookmarked";
import RecommendItem from "./RecommendItem";
import * as St from "./style";

Expand Down
Loading