Skip to content
This repository has been archived by the owner on Jul 29, 2024. It is now read-only.

[FE] feat: Ripple 컴포넌트 추가 #505

Closed
wants to merge 2 commits into from

Conversation

yogjin
Copy link
Collaborator

@yogjin yogjin commented Oct 1, 2023

🛠️ Issue

✅ Tasks

  • Ripple 컴포넌트 추가
  • 스토리 추가

⏰ Time Difference

  • 2 + 0

📝 Note

  • 자신을 감싸는 컴포넌트에 물결이 퍼지는 효과를 부여하는 컴포넌트 입니다.
  • 클릭으로 효과가 트리거됩니다.
2023-10-01.10.05.18.mov

@yogjin yogjin added 🎨 UI 디자인, UI 🖥️ frontend 프론트엔드 작업 labels Oct 1, 2023
@yogjin yogjin self-assigned this Oct 1, 2023
@nangkyeonglim nangkyeonglim changed the title [FE] Ripple 컴포넌트 추가 [FE] feat: Ripple 컴포넌트 추가 Oct 2, 2023
Copy link
Member

@jeonjeunghoon jeonjeunghoon left a comment

Choose a reason for hiding this comment

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

스토리북에서 리플을 열심히 사용해봤는데 너무 좋더라구요!
디바운스도 걸려있고 여러 번 클릭해도 잘 동작하고 너무 좋아요

궁금한 부분은 코멘트에 추가해놨어요!
고생하셨어요~

import React, { useState, useLayoutEffect, CSSProperties } from 'react';
import PropTypes from 'prop-types';

const useDebouncedRippleCleanUp = (
Copy link
Member

Choose a reason for hiding this comment

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

해당 훅은 파일로 분리하지 않은 이유가 궁금합니다

Comment on lines +10 to +20
let bounce: any = null;
if (rippleCount > 0) {
clearTimeout(bounce);

bounce = setTimeout(() => {
cleanUpFunction();
clearTimeout(bounce);
}, duration * 4);
}

return () => clearTimeout(bounce);
Copy link
Member

Choose a reason for hiding this comment

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

스택오버플로우

이 부분에 대해 찾아봤는데,
에디터에서 setTimeout이 반환하는 타입이 NodeJS.Timeout 이더라구요!
제 생각에는 동글은 node 환경이 필요없다고 생각합니당.
따라서 window.setTimeout() 으로 수정해 브라우저 환경에 맞는 타입을 받는게 좋을 것같아요!

Suggested change
let bounce: any = null;
if (rippleCount > 0) {
clearTimeout(bounce);
bounce = setTimeout(() => {
cleanUpFunction();
clearTimeout(bounce);
}, duration * 4);
}
return () => clearTimeout(bounce);
let bounce = 0; // 혹은 null 또한 받을 수 있도록 수정
if (rippleCount > 0) {
clearTimeout(bounce);
bounce = window.setTimeout(() => {

혹은 node 환경도 필요하다면 ReturnType<Type> 과 같은 유틸리티 타입을 사용하는 것도 좋을 것같아요.

Suggested change
let bounce: any = null;
if (rippleCount > 0) {
clearTimeout(bounce);
bounce = setTimeout(() => {
cleanUpFunction();
clearTimeout(bounce);
}, duration * 4);
}
return () => clearTimeout(bounce);
let bounce: ReturnType<typeof setTimeout> | null = null;
if (rippleCount > 0 && bounce) {
clearTimeout(bounce);
bounce = setTimeout(() => {
cleanUpFunction();
if (bounce) clearTimeout(bounce);
}, duration * 4);
}
return () => {
if (bounce) clearTimeout(bounce);
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://stackoverflow.com/questions/45802988/typescript-use-correct-version-of-settimeout-node-vs-window/56239226#56239226

저는 이글을 봤는데 그냥 아래처럼 ReturnType<Type>을 활용하면 node든 브라우저 환경이든 둘다 지원가능할 것 같아요!

그리고 밑에 아커코드에 &&bounce 조건문 있으면 영원히 조건문안으로 못들어오지않나요?

Copy link
Member

Choose a reason for hiding this comment

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

오 같은 글을 봤네요! 해당 글에서도 다양한 의견들이 나온 것같아요

저는 동글이 node 환경을 지원해야 하는 이유를 모르겠어요...! 어떤 경우에 필요한가요??

그리고 다시 보니 && bounce 있으면 조건문 안에 못들어가네요...ㅠ

Copy link
Collaborator

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.

아앗..... 그렇다면 필요하겠네요

Comment on lines +1 to +2
import React, { useState, useLayoutEffect, CSSProperties } from 'react';
import PropTypes from 'prop-types';
Copy link
Member

Choose a reason for hiding this comment

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

ReactPropTypes 현재 사용되지 않고 있는 것같아요

Comment on lines +37 to +53
* @Ripple
*
* 물결이 퍼지는 효과를 추가하는 컴포넌트
*
* 사용방법: 효과를 적용하고 싶은 컴포넌트에 `<Ripple/>`을 children으로 선언한다.
*
* `<Component>
* ...
* <Ripple/>
* </Component>`
*
* `Component` style에
* `{
* position: relative;
* overflow: hidden;
* }` 을 추가해야한다.
*/
Copy link
Member

Choose a reason for hiding this comment

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

자세한 사용법 감사합니다~

const useDebouncedRippleCleanUp = (
rippleCount: number,
duration: number,
cleanUpFunction: Function,
Copy link
Member

Choose a reason for hiding this comment

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

Function 을 사용한 이유는 어떤 형태의 함수가 들어올 지 정의가 안되어 그런걸까요?

duration: number,
cleanUpFunction: Function,
) => {
useLayoutEffect(() => {
Copy link
Member

Choose a reason for hiding this comment

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

useEffect 대신 useLayoutEffect 를 사용한 이유가 궁금합니다.
사용자의 마우스 위치 때문인가요?

Comment on lines +63 to +68
const size =
rippleContainer.width > rippleContainer.height
? rippleContainer.width
: rippleContainer.height;
const x = event.pageX - rippleContainer.x - size / 2;
const y = event.pageY - rippleContainer.y - size / 2;
Copy link
Member

Choose a reason for hiding this comment

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

이 친구들의 역할이 궁금해요...
제 추측엔
xy 는 사용자의 마우스 위치인 것같은데 맞나요?
size는 어떤 역할인지 모르겠어요. 또한 size / 2 의 역할은 무엇인가요?

Comment on lines +80 to +93
{rippleArray.length > 0 &&
rippleArray.map((ripple, index) => {
return (
<span
key={'span' + index}
style={{
top: ripple.y,
left: ripple.x,
width: ripple.size,
height: ripple.size,
}}
/>
);
})}
Copy link
Member

Choose a reason for hiding this comment

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

rippleArray가 배열인 이유가 사용자가 빠르게 여러 번 클릭했을 때, 모든 효과들이 끊기지 않고 정상 동작하고 사라지도록 하기 위함인가요?

export default Ripple;

import styled from 'styled-components';
import { Color } from 'styles/theme';
Copy link
Member

Choose a reason for hiding this comment

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

해당 객체는 사용되지 않고 있어요!

import { Color } from 'styles/theme';

const S = {
RippleContainer: styled.div<{ duration: number }>`
Copy link
Member

Choose a reason for hiding this comment

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

아마 duration 에 $ prefix를 붙이지 않으면 콘솔창에 에러 메시지가 발생할 것같아요

export default Ripple;

import styled from 'styled-components';
import { Color } from 'styles/theme';
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 친구는 안쓰고 있는 것 같아요 위에도 안쓰고 있는 import문들이 있어요
그리고 여기 import문 위로 올릴 수 있을까요?

};

return (
<S.RippleContainer duration={duration} color={color} onMouseDown={addRipple}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

onMouseDown를 사용한 이유가 궁금합니다!

Comment on lines +10 to +20
let bounce: any = null;
if (rippleCount > 0) {
clearTimeout(bounce);

bounce = setTimeout(() => {
cleanUpFunction();
clearTimeout(bounce);
}, duration * 4);
}

return () => clearTimeout(bounce);
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://stackoverflow.com/questions/45802988/typescript-use-correct-version-of-settimeout-node-vs-window/56239226#56239226

저는 이글을 봤는데 그냥 아래처럼 ReturnType<Type>을 활용하면 node든 브라우저 환경이든 둘다 지원가능할 것 같아요!

그리고 밑에 아커코드에 &&bounce 조건문 있으면 영원히 조건문안으로 못들어오지않나요?

@yogjin yogjin closed this Oct 10, 2023
@ezzanzzan ezzanzzan deleted the feature/ripple-component-503 branch October 10, 2023 05:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🖥️ frontend 프론트엔드 작업 🎨 UI 디자인, UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

물결이 퍼지는 효과를 주는 Ripple 컴포넌트 추가
3 participants