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

[Refactor]: useFocusInput 다른 input에서도 쓸 수 있도록 변경 #310

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

dlsxjzld
Copy link
Contributor

@dlsxjzld dlsxjzld commented Mar 28, 2024

📋 Issue Number

close #302

💻 구현 내용

  • useFocusInput 훅에 인자로 조건 값(isFocus)을 넘겨서 다른 input에서도 쓸 수 있게 만들었습니다.
    • 문장, 코드, 단어 게임에서 input에 focus 하기 위한 조건 값 : isRoundWaiting

📷 Screenshots

🤔 고민사항

  • 다른 div나 button 엘리먼트 등에서도 사용할 수 있게 만들어보려고 했는데 타입 문제 때문에 실패했습니다.
  • 참고한 자료

👀 문제점

1. 현재 상황

스크린샷 2024-03-28 오후 11 55 27
  • type이 mutableRef 여야함 (focusElement.current에 값을 넣기 위해서)
  • 스크린샷 2024-03-29 오전 12 03 19

2. 겪고 있는 문제

  • button 이나 div 엘리먼트는 type이 LegacyRef라 �mutableRef를 할당할 수 없음
스크린샷 2024-03-29 오전 12 19 42

3. 해결 방안

??????

Copy link

vercel bot commented Mar 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
team-e2-i4-ti-ki-taza-fe ✅ Ready (Inspect) Visit Preview Mar 28, 2024 3:56pm

@loevray
Copy link
Contributor

loevray commented Mar 29, 2024

만약 다른 엘리먼트도 focus를 주신다면, useFocus라는 네이밍이 조금 더 적절해보입니다!
또한 useEffect 리턴문에서 useRef값을 초기화 해주시는데, 안해주셔도 될 것 같아요! 이렇게 하면 null문제는 해결될 것 같습니다
그리고 제네릭으로 타입을 받아오면 타입을 조금이라도 더 좁힐 수 있을 것 같아요😃

import { useEffect, useRef } from 'react';

const useFocus = <T = null>(isFocus: boolean) => {
  const focusEl = useRef<T>(null);

  useEffect(() => {
    if (isFocus || !(focusEl.current instanceof HTMLElement)) {
      return;
    }
    focusEl.current.focus();
  }, [isFocus]);

  return { focusEl };
};

export default useFocus;

//---------- 사용할 때 --------------//
const { focusEl } = useFocusInput<HTMLButtonElement>(true);

<button ref={focusEl}>ㅎㅇ</button>

Copy link
Member

@yejinleee yejinleee left a comment

Choose a reason for hiding this comment

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

div 등에 focus를 주는건 어떤 효과일까요? 필요한 이유가 있나요??


+ @loevray

type이 mutableRef 여야함 (focusElement.current에 값을 넣기 위해서)

             ref={(e) => {
                ref(e);
                focusInput.current = e;
              }}

종욱님이 남기신거처럼 react-hook-form사용할때 ref에 할당하려면, 지금처럼 const focusInput = useRef<HTMLInputElement | null>(null); 로 해야 mutableRef 타입이 되는데,
영현님이 코멘트 주신것 처럼 제네릭을 쓰면 이부분이 문제가 되는것 같아요!
잘못이해한걸까요

@loevray
Copy link
Contributor

loevray commented Mar 29, 2024

저는 useRef를 dom제어용으로 사용하시는 줄 알았는데, 값 할당도 필요햇던걸까요?! 👀

@dlsxjzld
Copy link
Contributor Author

dlsxjzld commented Mar 30, 2024

저는 useRef를 dom제어용으로 사용하시는 줄 알았는데, 값 할당도 필요햇던걸까요?! 👀
예진님이 설명을 잘 해주셨네요!
react-hook-form과 같이 사용하므로 값 할당이 필요했습니다!

div 등에 focus를 주는건 어떤 효과일까요? 필요한 이유가 있나요??

  • 예전에 영현님과 얘기를 할 때 focus가 다른 엘리먼트에서도 가능하므로 범용적으로 써볼 수 있게 만들면 어떨까? 라는 얘기에서 시도하게 된 건데 필요한 이유는 크진 않은 것 같습니다!

-> 지금으로서는 input에만 사용하는 훅으로 남겨두는 것이 좋을 것 같은데 어떻게 생각하시나요?! 👀 @loevray @yejinleee @joonwonBaek

@yejinleee
Copy link
Member

제가 생각하기로는, input에만 두어도 될것 같습니다!

Copy link
Contributor

@loevray loevray left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ㅎㅎ
ci테스트겸 빈 커밋 하나 올려주실수 있으신가요?

src/pages/GamePage/hooks/useFocusInput.ts Show resolved Hide resolved
@dlsxjzld dlsxjzld merged commit ced5666 into main Apr 1, 2024
1 check passed
@dlsxjzld dlsxjzld deleted the Refactor-#302-useFocusInput branch April 1, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Refactor] useFocusInput 리팩토링
3 participants