Skip to content

fix:피드백 수정#13

Open
kimbosung521 wants to merge 2 commits into
developfrom
feature-web-17
Open

fix:피드백 수정#13
kimbosung521 wants to merge 2 commits into
developfrom
feature-web-17

Conversation

@kimbosung521

Copy link
Copy Markdown

신입 부원 모집 이름 길이 제한 추가,
지원하기에서 다른 페이지로 이동 시 alert 창 추가

신입 부원 모집 이름 길이 제한 추가,
지원하기에서 다른 페이지로 이동 시 alert 창 추가

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

Copy link
Copy Markdown

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 introduces page leave confirmation logic to prevent accidental navigation away from the recruitment form, limits the name input to 8 characters, and restricts it to Korean characters. The reviewer provided valuable feedback on two main issues: first, the page leave confirmation should only trigger when the form is dirty and should also handle browser refreshes or tab closures via a beforeunload listener; second, replacing characters in real-time during Korean (IME) input can cause composition and cursor issues, so it is recommended to rely on validation and maxLength instead of real-time regex replacement.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/pages/visitor/RecruitPage.jsx Outdated
Comment on lines +39 to +54
useEffect(() => {
const handleLeavePage = (event) => {
const link = event.target.closest('a[href]');

if (!link || link.pathname === ROUTES.RECRUIT || link.pathname === ROUTES.RECRUIT_COMPLETE) {
return;
}

if (!window.confirm(LEAVE_CONFIRM_MESSAGE)) {
event.preventDefault();
}
};

document.addEventListener('click', handleLeavePage, true);
return () => document.removeEventListener('click', handleLeavePage, true);
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

현재 구현된 페이지 이탈 방지 로직은 다음과 같은 두 가지 아쉬운 점이 있습니다:

  1. 빈 폼에서도 알림 표시: 사용자가 아무런 입력도 하지 않은 상태(초기 상태)에서 다른 페이지로 이동할 때도 '입력하신 정보는 저장되지 않습니다.'라는 알림창이 나타나 사용자 경험(UX)을 해칩니다.
  2. 새로고침 및 탭 닫기 미지원: 브라우저 새로고침이나 탭 닫기, 주소창 직접 입력 등을 통한 이탈 시에는 click 이벤트가 발생하지 않으므로 입력한 정보가 경고 없이 유실됩니다.

이를 해결하기 위해 폼의 변경 여부(isDirty)를 감지하고, beforeunload 이벤트를 함께 등록하여 브라우저 종료/새로고침 시에도 경고를 띄우도록 개선하는 것을 제안합니다.

  const isDirty = Object.values(form).some((value) => value !== '');

  useEffect(() => {
    if (!isDirty) return;

    const handleLeavePage = (event) => {
      const link = event.target.closest('a[href]');

      if (!link || link.pathname === ROUTES.RECRUIT || link.pathname === ROUTES.RECRUIT_COMPLETE) {
        return;
      }

      if (!window.confirm(LEAVE_CONFIRM_MESSAGE)) {
        event.preventDefault();
      }
    };

    const handleBeforeUnload = (event) => {
      event.preventDefault();
      event.returnValue = '';
    };

    document.addEventListener('click', handleLeavePage, true);
    window.addEventListener('beforeunload', handleBeforeUnload);

    return () => {
      document.removeEventListener('click', handleLeavePage, true);
      window.removeEventListener('beforeunload', handleBeforeUnload);
    };
  }, [isDirty]);

Comment thread src/hooks/useRecruitForm.js Outdated
Comment on lines 23 to 32
let nextValue = value;

if (name === 'phone') {
nextValue = formatPhoneNumber(value);
}

if (name === 'name') {
nextValue = value.replace(/[^ㄱ-ㅎㅏ-ㅣ가-힣]/g, '').slice(0, 8);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

React에서 한글(IME) 입력 중에 onChange 이벤트에서 정규식을 통해 실시간으로 값을 대체(replace)하면, 글자 조합이 깨지거나 커서가 맨 뒤로 이동하는 등의 입력 오류(IME composition issue)가 발생할 수 있습니다.

이미 RecruitBasicInfoSection 컴포넌트의 FormInputmaxLength={8} 속성이 적용되어 있고, fieldValidators에서 한글 및 길이 검증(/^[가-힣]+$/ 및 2~8자)을 수행하고 있으므로, handleChange에서 실시간으로 한글 외의 문자를 제거하는 로직은 제거하는 것이 안전합니다.

Suggested change
let nextValue = value;
if (name === 'phone') {
nextValue = formatPhoneNumber(value);
}
if (name === 'name') {
nextValue = value.replace(/[^---]/g, '').slice(0, 8);
}
const nextValue = name === 'phone' ? formatPhoneNumber(value) : value;

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.

1 participant