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 PhoneInput fix Cursor Error #4

Merged
merged 8 commits into from
Oct 7, 2024

Conversation

ChaeyeonAhn
Copy link
Contributor

@ChaeyeonAhn ChaeyeonAhn commented Sep 24, 2024

요약 *

It closes #3

전화번호 중간에 커서를 놓고 편집하면 커서가 다른 곳으로 가 버리는 문제가 있었습니다.
따라서 Input에 ChangeEvent가 있을 때마다 커서 위치를 저장해 놓고 따로 설정해서 위치를 고정해주는 로직을 짜기 위해 Input DOM 객체에 접근해야 했습니다.

그러나 PhoneInput에서 TextInput을 가져다 쓰는 방식은 Input DOM 객체에 접근하기가 어려워서, PhoneInput과 TextInput을 결합하는 방향으로 진행해 보았습니다.

이것이 제일 좋은 방법인지는 모르겠으나 동작은 원활하게 되는 것 같고,
TextInput 중에서도 전화번호는 특수한 케이스라고 여겨져 이렇게 따로 만드는 것도 괜찮다고 생각했습니다.
한 번 같이 고민해주시면 감사하겠습니다!

Update: React-hook-form 도입

스크린샷

Clubs /my에서 확인한 영상입니다.
https://github.com/user-attachments/assets/d0243fb5-bbe1-4c13-87bb-87b4a3ff89ed

React-hook-form 도입 이후
https://github.com/user-attachments/assets/6f58c073-125c-4da6-a33f-0c0422d61fa9

이후 Task *

  • PhoneInput 안에 react-hook-form 을 도입하지 않고 나중에 폼 안에 PhoneInput 쓸 때 바깥에서 쓰기로 결정! 그때 react-hook-form과 호환 가능한지 보고 고치기!!

@ChaeyeonAhn ChaeyeonAhn added the enhancement New feature or request label Sep 24, 2024
@ChaeyeonAhn ChaeyeonAhn self-assigned this Sep 24, 2024
@ChaeyeonAhn ChaeyeonAhn linked an issue Sep 24, 2024 that may be closed by this pull request
2 tasks
Copy link
Contributor

@wjeongchoi wjeongchoi left a comment

Choose a reason for hiding this comment

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

NewPhoneInput 대신 PhoneInput으로 바로 넣어도 될 것 같아요! 클럽스에 원 코드는 있으니까
잘 고친 것 같은데 한 가지 확인하면 좋을 점은 react-hook-form으로도 쓸 수 있는지 확인해주세용

두 가지 확인하고 다시 리뷰 요청 부탁드려요

@ChaeyeonAhn
Copy link
Contributor Author

react-hook-form을 쓸 수 있을 것 같습니다. 도입해서 수정할까요?

Copy link
Contributor

@wjeongchoi wjeongchoi left a comment

Choose a reason for hiding this comment

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

            <FormController
              name="phoneNumber"
              required
              control={control}
              defaultValue={profile?.phoneNumber}
              minLength={13}
              // TODO: phoneNumber validation
              // pattern={/^010-\d{4}-\d{4}$/}
              renderItem={props => (
                <PhoneInput
                  {...props}
                  label="대표자 전화번호"
                  placeholder="010-XXXX-XXXX"
                />
              )}
            />

클럽스에서 react-hook-form 쓰고 있는 부분인데, 지금처럼 내부에서 react-hook-form 쓰는게 나은지 이전처럼 밖에서 쓰는게 나을지는 약간 고민이 되네요.!

packages/web/src/common/components/Forms/PhoneInput.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@wjeongchoi wjeongchoi left a comment

Choose a reason for hiding this comment

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

굿굿 수고했습니당

@ChaeyeonAhn ChaeyeonAhn merged commit 1d3d643 into dev Oct 7, 2024
1 check passed
@ChaeyeonAhn ChaeyeonAhn deleted the 3-refactor-phoneinput-cursor branch October 7, 2024 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor PhoneInput Cursor
2 participants