Skip to content

fix: 엔터 폼 제출 제한 및 팀 등록시 이미지 등록 관련 메시지 추가#468

Open
sungwonnoh wants to merge 8 commits intowip-v2from
fix/manager-update
Open

fix: 엔터 폼 제출 제한 및 팀 등록시 이미지 등록 관련 메시지 추가#468
sungwonnoh wants to merge 8 commits intowip-v2from
fix/manager-update

Conversation

@sungwonnoh
Copy link
Copy Markdown
Contributor

@sungwonnoh sungwonnoh commented Mar 27, 2026

🌍 이슈 번호

  • 이슈 번호

✅ 작업 내용

  • 팀 등록시 이미지가 없는 상황에 대한 alert메시지를 추가했어요
  • 경기 생성 시 팀원 검색 기능 추가
  • 경기 생성시 팀원 검색 중 엔터키를 누르면 경기가 생성되는 문제 발견
    • 학번 입력시 에러 메세지뜨는 문제 수정(옵셔널체이닝 추가)
    • form 태그 내에서 onKeyDown을 추가하여 엔터키로 폼이 제출되는 현상 방지 -> preventDefault를 추가하여 마지막 단계에서 완료 버튼을 눌러야만 폼이 전송되도록 수정
    • 어시스턴트 내에서도 엔터키에 대한 비슷한 이슈가 있어 onKeyDown추가

고민한 부분

form 태그 위에서 엔터키를 전체를 다 막는게 좋을지, 특정 입력 필드에만 적용하는게 좋을지가 고민이네요

@sungwonnoh sungwonnoh self-assigned this Mar 27, 2026
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
manager Ready Ready Preview, Comment Apr 1, 2026 9:07am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
spectator Skipped Skipped Apr 1, 2026 9:07am

@sungwonnoh sungwonnoh requested a review from seongminn March 27, 2026 02:49
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements measures to prevent accidental form submissions by intercepting the 'Enter' key, refactors player filtering logic in the game lineup step, and adds a validation check for team logos in the player registration assistant. Review feedback suggests that applying 'onKeyDown' handlers to entire forms or container divs is too broad and may degrade user experience or accessibility; it is recommended to target specific problematic inputs or resolve the underlying cause within child components instead.

@seongminn
Copy link
Copy Markdown
Member

Form에서 엔터키 입력을 방지하면, 키보드 사용자는 폼 입력을 할 수 없다는 구조적인 문제가 있어요. 이건 웹 접근성 측면에서 좀 더 깊이 고민해보면 좋을 문제인 것 같습니다.

그래서 제가 생각할 때는 선수 검색 시에 Enter를 눌러야 하는 상황이 왜 발생했는지 생각해보면 좋을 것 같은데요.
일단 엔터키 입력 없이, 선수 이름 타이핑만으로도 선수 검색이 가능해야 할 것 같습니다. 그리고 타이핑 시에 선수 검색이 진행되고 있다는 UI를 보여줌으로써 엔터키를 누르지 않아도 된다는 인상을 주는 방향으로 고려해보면 어떨까 싶네요.

@sungwonnoh
Copy link
Copy Markdown
Contributor Author

이미 타이핑만으로도 검색은 잘되고 있는데 중간에 엔터키를 칠 때 폼이 제출되는 문제인 것 같아요. 그리고 대부분 폼이 버튼으로 입력하는 구조라서 엔터키를 막아볼까 했는데, 말씀해주신대로 일단은 특정 부분만 적용시키는 걸로 수정할게요!

2026-03-28.120430.mp4

@seongminn
Copy link
Copy Markdown
Member

성원님 제가 이전에는 UI를 못보고 리뷰를 달긴 했는데요.

동영상 첨부해주신 거 보니까, 폼 제출 함수에서 필요한 경기 정보들이 다 들어 있는지, 그리고 step이 경기 영상인지 등을 체크하면 Enter를 굳이 안막아도 될 것 같은데 어떻게 생각하세요?

접근성을 고려했을 때 엔터를 막는 방법은 문제 해결이 아니라 새로운 문제를 만들어내는 거 같아서요.

@sungwonnoh
Copy link
Copy Markdown
Contributor Author

말씀해주신 부분으로 가는게 맞는것 같네용 36f7d03 수정했습니다!

<form
className="flex h-full w-full flex-col bg-white p-4"
onSubmit={form.handleSubmit(handleFormSubmit, handleFormError)}
onSubmit={(e) => e.preventDefault()}
Copy link
Copy Markdown
Member

@seongminn seongminn Mar 30, 2026

Choose a reason for hiding this comment

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

계속 말이 늘어지는 거 같아서 죄송하네요ㅠㅠ..

<GameEditVideoStep />에서 onSubmit 함수를 받고 버튼 타입을 button으로 변경하는 건, 앞서 계속 이야기 했던 것처럼 버튼이 올바른 타입으로 사용되지 못하는 거 같아요.

불가피한 경우가 아니라면, HTML 태그나 태그의 속성은 상황에 맞게 시멘틱하게 사용하는 게 가장 좋습니다. 그렇게 했을 때 브라우저가 각 요소에 대해 정확하게 이해할 수 있고, 이는 접근성, SEO 최적화, 성능 등에 유의미하게 영향을 줍니다.

그래서 제가 생각할 땐 handleFormSubmit 함수가 아래처럼 바뀌어야 할 것 같은데, 어떻게 생각하세요?

  const handleFormSubmit = (formData: GameUpdateFormType) => {
    // toast 문구는 예시
    if (step !== 2) toast.warning('모든 단계를 완료해야 경기 수정이 가능합니다.');
    if (!form.formState.isValid)
      return toast.error('입력값을 확인해주세요. 모든 단계의 입력값이 유효해야 합니다.');

    mutate(
      // ...
    );
  };

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

e5bcde5

이렇게 수정했는데 확인 부탁드려요 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants