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] BottomSheet 컴포넌트, 스토리북 #41

Merged
merged 21 commits into from
Nov 17, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5af13cf
[FEAT] BottomSheet 컴포넌트, 스토리북
houony Nov 8, 2024
3d3cf25
[REFACTOR] BottomSheet 컴포넌트
houony Nov 10, 2024
7bf1a90
Merge branch 'main' of https://github.com/Findy-org/FINDY_FE into fea…
houony Nov 10, 2024
366e0cb
[CHORE] BottomSheet 컴포넌트, 스토리북
houony Nov 10, 2024
70ffaf2
refactor : add error handling, reduce redundancy
houony Nov 11, 2024
eb7d761
refactor: add MouseEvent and TouchEvent
houony Nov 11, 2024
beeb54a
Merge branch 'main' of https://github.com/Findy-org/FINDY_FE into fea…
houony Nov 11, 2024
a92c437
chore : remove try-catch
houony Nov 11, 2024
c1a3f12
refactor : remove calculation of drag velocity
houony Nov 11, 2024
c63dfac
Merge branch 'main' of https://github.com/Findy-org/FINDY_FE into fea…
houony Nov 11, 2024
2279cf6
refactor : improve BottomSheet animation and positioning
houony Nov 12, 2024
631810a
refactor : optimize drag handler with requestAnimationFrame
houony Nov 12, 2024
90d4e6a
refactor : remove unnecessary code, change handleDragEnd logic
houony Nov 12, 2024
f273b49
refactor : remove onClose
houony Nov 12, 2024
f8f8c1f
refactor : add early return at handleDragEnd
houony Nov 12, 2024
390eb14
refactor : change style to tailwind, change dragElastic
houony Nov 12, 2024
5bf5e10
refactor : change drag amount
houony Nov 13, 2024
489baf2
refactor : move BottomSheet framer setting to motions.ts
houony Nov 14, 2024
b2f0822
refactor : remove onClose, add BottomSheetHeader, remove backdrop, up…
houony Nov 14, 2024
f3ff5ab
chore : remove react-use-measure, debounce
houony Nov 15, 2024
a4d2f8b
refactor : remove AnimatePresence, setTimeout
houony Nov 15, 2024
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
38 changes: 38 additions & 0 deletions .pnp.cjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 38 additions & 0 deletions src/components/common/BottomSheet/BottomSheet.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/* eslint-disable no-restricted-exports */
import { useState } from 'react';
import type { Meta, StoryObj } from '@storybook/react';

import { BottomSheet } from '.';
import { Button } from '../Button';

const meta = {
title: 'components/common/BottomSheet',
component: BottomSheet,
tags: ['autodocs'],
} satisfies Meta<typeof BottomSheet>;

export default meta;
type Story = StoryObj<typeof BottomSheet>;

export const Basic: Story = {
render: () => {
const [isOpen, setIsOpen] = useState(false);

const handleClickBottomSheet = () => setIsOpen(!isOpen);

return (
<>
<Button variant="primary" size="medium" onClick={handleClickBottomSheet}>
Open BottomSheet
</Button>
{isOpen && (
<BottomSheet isOpen={isOpen}>
<div className="flex flex-col gap-6 items-center">
<div>Contents ...</div>
</div>
</BottomSheet>
)}
</>
);
},
};
21 changes: 21 additions & 0 deletions src/components/common/BottomSheet/BottomSheet.types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { ComponentPropsWithoutRef, ReactNode } from 'react';

export type Props = ComponentPropsWithoutRef<'div'> & {
/**
* Indicates whether the BottomSheet is currently open.
* @default false
*/
isOpen: boolean;
/**
* Callback function that is called when the BottomSheet should close.
*/
onClose?: VoidFunction;
Copy link
Member

Choose a reason for hiding this comment

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

isOpen 으로 close도 관리하면 될 것 같은데 onClose함수가 필요한 이유가 따로 있나요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

혹시 나중에 바텀시트가 닫히는 시점에 부모 컴포넌트에서 액션이 필요한 부분이 있을까봐 부모 컴포넌트로 바텀시트 상태를 전달하려고 따로 작성했는데, 생각해보니 바텀시트가 닫혀도 다른 상태는 바뀔게 없을 것 같아서 isOpen으로 통합하겠습니다!

/**
* The content to be displayed inside the BottomSheet.
*/
children: ReactNode;
};

export type DragInfo = {
delta: { x: number; y: number };
};
50 changes: 50 additions & 0 deletions src/components/common/BottomSheet/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { memo } from 'react';
import { motion, useDragControls, AnimatePresence } from 'framer-motion';

import { BOTTOM_SHEET_ANIMATION } from '@/constants/motions';
import { useBottomSheet } from '@/hooks/common/useBottomSheet';

import { Props } from './BottomSheet.types';

import { Portal } from '../Portal';

const Content = ({ children }: React.PropsWithChildren) => <div>{children}</div>;

export const BottomSheet = memo(({ children, isOpen }: Props) => {
const dragControls = useDragControls();
keemsebin marked this conversation as resolved.
Show resolved Hide resolved

const { sheetHeight, isHidden, isInteractionDisabled, handleDrag, handleDragEnd, handleClose } =
useBottomSheet(isOpen);

return (
<Portal isOpen={!isHidden}>
<div
className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${
isHidden ? 'pointer-events-none' : 'pointer-events-auto'
}`}
onClick={handleClose}
/>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

백드롭 구현 개선 필요

현재 백드롭에 다음과 같은 개선이 필요합니다:

  1. 배경색이 지정되어 있지 않습니다
  2. 불투명도 전환만 있고 실제 불투명도 값이 설정되어 있지 않습니다
-        className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${
-          isHidden ? 'pointer-events-none' : 'pointer-events-auto'
-        }`}
+        className={`absolute top-0 left-0 w-full h-full bg-black transition-opacity duration-300 ${
+          isHidden ? 'opacity-0 pointer-events-none' : 'opacity-70 pointer-events-auto'
+        }`}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div
className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${
isHidden ? 'pointer-events-none' : 'pointer-events-auto'
}`}
onClick={handleClose}
/>
<div
className={`absolute top-0 left-0 w-full h-full bg-black transition-opacity duration-300 ${
isHidden ? 'opacity-0 pointer-events-none' : 'opacity-70 pointer-events-auto'
}`}
onClick={handleClose}
/>


<AnimatePresence>
keemsebin marked this conversation as resolved.
Show resolved Hide resolved
{!isHidden && (
<motion.div
className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden z-[1000] h-${sheetHeight}"
drag="y"
dragControls={dragControls}
dragElastic={0}
dragConstraints={{ top: 0, bottom: 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
Collaborator Author

Choose a reason for hiding this comment

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

네 이 속성들은 다 적용되고 있습니당

onDrag={handleDrag}
onDragEnd={handleDragEnd}
animate={BOTTOM_SHEET_ANIMATION.animate(sheetHeight, isHidden)}
transition={BOTTOM_SHEET_ANIMATION.transition}
onPointerDown={(e) => !isInteractionDisabled && dragControls.start(e)}
exit={BOTTOM_SHEET_ANIMATION.exit}
>
<div className="w-20 h-1.5 bg-primary mx-auto rounded-full" />
<Content>{children}</Content>
</motion.div>
)}
</AnimatePresence>
</Portal>
);
});
3 changes: 3 additions & 0 deletions src/constants/bottomSheetOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const INITIAL_HEIGHT = 150;
export const MIN_VISIBLE_HEIGHT = 60;
export const MAX_HEIGHT = window.innerHeight * 0.9;
17 changes: 17 additions & 0 deletions src/constants/motions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,20 @@ export const SLIDER_ANIMATION = {
},
},
};

export const BOTTOM_SHEET_ANIMATION = {
animate: (sheetHeight: number, isHidden: boolean) => ({
height: sheetHeight,
opacity: isHidden ? 0 : 1,
}),
exit: {
height: 0,
opacity: 0,
},
transition: {
type: 'spring',
stiffness: 170,
damping: 30,
duration: 0.3,
},
};
76 changes: 76 additions & 0 deletions src/hooks/common/useBottomSheet.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { useState, useCallback, useRef, useEffect } from 'react';

import { DragInfo } from '@/components/common/BottomSheet/BottomSheet.types';
import { INITIAL_HEIGHT, MIN_VISIBLE_HEIGHT, MAX_HEIGHT } from '@/constants/bottomSheetOptions';

export const useBottomSheet = (isOpen: boolean) => {
const [sheetHeight, setSheetHeight] = useState(INITIAL_HEIGHT);
const [isHidden, setIsHidden] = useState(false);
const [isInteractionDisabled, setIsInteractionDisabled] = useState(false);
const dragOffsetRef = useRef(0);
const initialPositionRef = useRef(INITIAL_HEIGHT);

useEffect(() => {
if (isOpen) {
setSheetHeight(INITIAL_HEIGHT);
setIsHidden(false);
setIsInteractionDisabled(false);
dragOffsetRef.current = 0;
initialPositionRef.current = INITIAL_HEIGHT;
}
}, [isOpen]);

const handleDrag = useCallback(
(_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => {
if (isInteractionDisabled) return;

requestAnimationFrame(() => {
const dragAmount = -info.delta.y;
dragOffsetRef.current += dragAmount * 5;

const newHeight = Math.min(
Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT),
MAX_HEIGHT
);
setSheetHeight(newHeight);
});
},
[isInteractionDisabled, setSheetHeight]
);
Comment on lines +42 to +58
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

드래그 동작 개선이 필요합니다.

현재 구현에서 발견된 문제점들:

  1. dragAmount에 임의의 값(5)을 곱하는 것은 예측하기 어려운 동작을 야기할 수 있습니다.
  2. 드래그 저항이 없어 사용자 경험이 부자연스러울 수 있습니다.
 const handleDrag = useCallback(
   (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => {
     if (isInteractionDisabled) return;

+    // 수직 방향으로만 드래그 허용
+    if (Math.abs(info.delta.x) > Math.abs(info.delta.y)) return;
+
     requestAnimationFrame(() => {
       const dragAmount = -info.delta.y;
-      dragOffsetRef.current += dragAmount * 5;
+      // 드래그 저항 추가
+      const resistance = 0.5;
+      dragOffsetRef.current += dragAmount * resistance;

       const newHeight = Math.min(
         Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT),
         MAX_HEIGHT
       );
       setSheetHeight(newHeight);
     });
   },
   [isInteractionDisabled, setSheetHeight]
 );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleDrag = useCallback(
(_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => {
if (isInteractionDisabled) return;
requestAnimationFrame(() => {
const dragAmount = -info.delta.y;
dragOffsetRef.current += dragAmount * 5;
const newHeight = Math.min(
Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT),
MAX_HEIGHT
);
setSheetHeight(newHeight);
});
},
[isInteractionDisabled, setSheetHeight]
);
const handleDrag = useCallback(
(_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => {
if (isInteractionDisabled) return;
// 수직 방향으로만 드래그 허용
if (Math.abs(info.delta.x) > Math.abs(info.delta.y)) return;
requestAnimationFrame(() => {
const dragAmount = -info.delta.y;
// 드래그 저항 추가
const resistance = 0.5;
dragOffsetRef.current += dragAmount * resistance;
const newHeight = Math.min(
Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT),
MAX_HEIGHT
);
setSheetHeight(newHeight);
});
},
[isInteractionDisabled, setSheetHeight]
);


const handleDragEnd = useCallback(() => {
if (sheetHeight <= MIN_VISIBLE_HEIGHT) {
setIsHidden(true);
setIsInteractionDisabled(true);
return;
}

if (sheetHeight > MAX_HEIGHT) {
setSheetHeight(MAX_HEIGHT);
return;
}
}, [sheetHeight]);

const resetSheet = useCallback(() => {
setSheetHeight(INITIAL_HEIGHT);
setIsHidden(false);
setIsInteractionDisabled(false);
dragOffsetRef.current = 0;
initialPositionRef.current = INITIAL_HEIGHT;
}, []);

const handleClose = useCallback(() => {
if (!isInteractionDisabled) {
resetSheet();
}
}, [isInteractionDisabled, resetSheet]);

return {
sheetHeight,
isHidden,
isInteractionDisabled,
handleDrag,
handleDragEnd,
handleClose,
};
};
Loading