[FEAT-3] Form 웹 접근성 개선 #10
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @hin6150, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 Multi-step Form의 웹 접근성과 사용성을 크게 개선하고, SSR 환경에서 발생할 수 있는 스타일 깜빡임(FOUC) 문제를 해결하는 데 중점을 둡니다. 주요 변경 사항으로는 에러 발생 시 필드 자동 포커스, Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| "version": "15.4.8", | ||
| "resolved": "https://registry.npmjs.org/next/-/next-15.4.8.tgz", | ||
| "integrity": "sha512-jwOXTz/bo0Pvlf20FSb6VXVeWRssA2vbvq9SdrOPEg9x8E1B27C2rQtvriAn600o9hH61kjrVRexEffv3JybuA==", | ||
| "deprecated": "This version has a security vulnerability. Please upgrade to a patched version. See https://nextjs.org/blog/security-update-2025-12-11 for more details.", |
There was a problem hiding this comment.
next.js 버전 15.4.8은 보안 취약점이 보고되어 deprecated 처리되었습니다. package-lock.json의 deprecated 메시지를 확인하시고, 가능한 한 빨리 패치된 버전으로 업그레이드하는 것을 강력히 권장합니다. 이는 애플리케이션의 보안을 위해 매우 중요합니다.
| display: flex; | ||
| flex-direction: column; | ||
| min-height: 0; | ||
| overflow-y: auto; |
There was a problem hiding this comment.
stepMainArea 스타일에서 overflow-y: auto를 제거하셨는데, 이로 인해 스크롤이 제대로 동작하지 않을 수 있습니다. 부모 요소인 form[stepFormLayout]에 overflow: hidden과 height: 100%가 적용되어 있어, stepMainArea의 내용이 넘칠 경우 스크롤바가 나타나지 않고 내용이 잘릴 수 있습니다.
window.scrollTo를 사용하기 위해 변경하신 것으로 보이지만, 현재 레이아웃 구조와는 맞지 않는 것 같습니다.
다음 두 가지 해결 방법 중 하나를 고려해 보세요:
stepMainArea에overflow-y: auto를 다시 추가하고,use-multi-step-form.ts의scrollToTop함수가stepMainArea엘리먼트를 직접 참조하여 스크롤하도록 수정합니다. (ref 전달 필요)stepFormLayout에서height: 100%와overflow: hidden을 제거하여window가 스크롤되도록 레이아웃 구조를 변경합니다.
현재 구조상 1번 방법이 다른 부분에 미치는 영향이 적어 더 안정적일 수 있습니다.
| const toFocusablePath = useCallback( | ||
| (field?: Path<TValues> | string) => { | ||
| if (!field) return undefined | ||
| const fieldName = String(field) | ||
|
|
||
| if (fieldName === 'quotes') { | ||
| const quotes = getValues('quotes' as Path<TValues>) | ||
| if (Array.isArray(quotes) && quotes.length > 0) { | ||
| const firstQuote = quotes[0] | ||
| if (firstQuote && typeof firstQuote === 'object') { | ||
| if ('content' in firstQuote) { | ||
| return 'quotes.0.content' | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return fieldName | ||
| }, | ||
| [getValues] | ||
| ) |
There was a problem hiding this comment.
toFocusablePath 함수 내부에 'quotes' 필드에 대한 처리 로직이 하드코딩되어 있어 범용성이 떨어집니다. 이로 인해 새로운 필드 배열이 추가될 때마다 이 함수를 수정해야 할 수 있습니다.
이 문제를 해결하기 위해 FormStep 타입에 focusTarget과 같은 선택적 프로퍼티를 추가하여, 각 스텝에서 포커스를 받아야 할 필드를 명시적으로 지정하는 방법을 제안합니다. 이렇게 하면 useStepController 훅의 재사용성과 유지보수성이 향상될 것입니다.
예시:
// FormStep 타입 정의에 추가
export type FormStep<TValues, TComponentId extends string> = {
// ... other properties
focusTarget?: Path<TValues> | string;
}
// multi-step-form.tsx 에서 steps 정의 시
const steps: FormStep<FormValues, StepId>[] = [
// ...
{
id: 3,
label: '인용구',
componentId: 'quoteSelection',
fields: ['quotes'],
schema: step4Schema,
focusTarget: 'quotes.0.content', // 포커스 대상 명시
},
// ...
]
// useStepController 훅 내부
// toFocusablePath 함수를 제거하고 아래와 같이 수정
useEffect(() => {
const step = steps[currentIndex];
if (!step) return;
const fieldToFocus = step.focusTarget ?? step.fields?.[0];
if (fieldToFocus) {
focusField(String(fieldToFocus));
}
}, [currentIndex, focusField, steps]);There was a problem hiding this comment.
Pull request overview
This PR enhances form accessibility and user experience in a multi-step form implementation by addressing FOUC (Flash of Unstyled Content) issues and implementing comprehensive focus management and keyboard navigation features.
Key Changes:
- Fixed SSR style flashing by properly injecting Emotion cache from
_documentto_app - Implemented auto-focus on validation errors and step changes with special handling for field arrays
- Added Enter key navigation to prevent accidental form submission and move focus to next field
- Enhanced accessibility with ARIA attributes, proper focus styles, and improved visually-hidden implementation
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/pages/_document.tsx |
Modified Emotion cache injection to pass cache as prop instead of using CacheProvider wrapper |
src/pages/_app.tsx |
Added MyAppProps interface to accept emotionCache prop for SSR style consistency |
src/features/book-form/multi-step-form.tsx |
Enabled shouldFocusError flag for automatic focus management on validation errors |
src/features/book-form/hooks/use-multi-step-form.ts |
Implemented focus management logic with field path resolution, auto-focus on step changes, and scroll-to-top behavior |
src/components/stepper/step-navigator.tsx |
Added Enter key handler to navigate between form fields instead of submitting |
src/components/rhf-inputs/rhf-segmented.tsx |
Added ARIA radiogroup attributes with labelledby for better screen reader support |
src/styles/form-styles.ts |
Enhanced segmented control with focus-visible styles and improved visually-hidden implementation following WCAG guidelines |
src/styles/step-styles.ts |
Removed overflow-y property from step main area |
package.json |
Updated Next.js to 15.4.8 and React to 19.1.2 |
package-lock.json |
Updated dependency lockfile with new versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const toFocusablePath = useCallback( | ||
| (field?: Path<TValues> | string) => { | ||
| if (!field) return undefined | ||
| const fieldName = String(field) | ||
|
|
||
| if (fieldName === 'quotes') { | ||
| const quotes = getValues('quotes' as Path<TValues>) | ||
| if (Array.isArray(quotes) && quotes.length > 0) { | ||
| const firstQuote = quotes[0] | ||
| if (firstQuote && typeof firstQuote === 'object') { | ||
| if ('content' in firstQuote) { | ||
| return 'quotes.0.content' | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return fieldName | ||
| }, | ||
| [getValues] | ||
| ) |
There was a problem hiding this comment.
The toFocusablePath function lacks documentation explaining its purpose, when it should be used, and why special handling for 'quotes' is necessary. Consider adding a JSDoc comment that describes the function's role in converting field array paths to focusable element paths, especially for nested structures.
| "next": "15.4.6", | ||
| "react": "19.1.0", | ||
| "react-dom": "19.1.0", | ||
| "next": "15.4.8", |
There was a problem hiding this comment.
The Next.js version 15.4.8 being upgraded to has a deprecated security vulnerability. The package-lock.json shows a deprecation warning indicating this version has a security issue and recommends upgrading to a patched version. Please upgrade to a more recent version of Next.js that addresses this security vulnerability.
| "next": "15.4.8", | |
| "next": "15.4.9", |
| ctx.renderPage = () => | ||
| originalRenderPage({ | ||
| enhanceApp: (App) => | ||
| enhanceApp: (App: ComponentType<ComponentProps<ComponentType<MyAppProps>>>) => |
There was a problem hiding this comment.
The type annotation for the App parameter is overly complex and difficult to read. The nested ComponentProps and ComponentType make the type harder to understand. Consider simplifying this by extracting a type alias or using a more direct type reference to MyAppProps.
| enhanceApp: (App: ComponentType<ComponentProps<ComponentType<MyAppProps>>>) => | |
| enhanceApp: (App: ComponentType<MyAppProps>) => |
| const toFocusablePath = useCallback( | ||
| (field?: Path<TValues> | string) => { | ||
| if (!field) return undefined | ||
| const fieldName = String(field) | ||
|
|
||
| if (fieldName === 'quotes') { | ||
| const quotes = getValues('quotes' as Path<TValues>) | ||
| if (Array.isArray(quotes) && quotes.length > 0) { | ||
| const firstQuote = quotes[0] | ||
| if (firstQuote && typeof firstQuote === 'object') { | ||
| if ('content' in firstQuote) { | ||
| return 'quotes.0.content' | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return fieldName | ||
| }, | ||
| [getValues] | ||
| ) |
There was a problem hiding this comment.
The toFocusablePath function has hardcoded special handling for the 'quotes' field which creates tight coupling and reduces reusability. This makes the function less generic and harder to extend for other field array cases. Consider implementing a more generic approach, such as accepting a configuration object that maps field names to their focusable paths, or using a naming convention to automatically detect field arrays.
| useEffect(() => { | ||
| const fields = steps[currentIndex]?.fields | ||
| if (!fields || fields.length === 0) return | ||
|
|
||
| const focusable = toFocusablePath(fields[0]) | ||
| focusField(focusable) | ||
| }, [currentIndex, focusField, steps, toFocusablePath]) |
There was a problem hiding this comment.
The useEffect at line 94 automatically focuses the first field whenever the step changes. This could be disruptive to user experience if a user navigates backward or uses keyboard shortcuts, as it forcefully moves focus without user intention. Consider only auto-focusing on forward navigation or when validation fails, not on all step changes.
| if (element.tagName === 'INPUT') { | ||
| const input = element as HTMLInputElement | ||
| if (input.type === 'radio') return false | ||
| } |
There was a problem hiding this comment.
The handleKeyDown function filters out radio button inputs from receiving Enter key focus, which could be confusing for keyboard users. Radio buttons are typically navigated with arrow keys and selected with Space or Enter. Preventing Enter from moving focus to the next radio button might be the intended behavior for this specific form, but it deviates from standard accessibility patterns where Enter can be used for selection in some contexts.
| const focusField = useCallback( | ||
| (field?: Path<TValues> | string) => { | ||
| if (!field) return | ||
| const focusTarget = String(field) as Path<TValues> | ||
| const focusExecutor = | ||
| typeof window !== 'undefined' && typeof window.requestAnimationFrame === 'function' | ||
| ? window.requestAnimationFrame.bind(window) | ||
| : (cb: FrameRequestCallback) => { | ||
| setTimeout(() => cb(Date.now()), 0) | ||
| } | ||
|
|
||
| focusExecutor(() => { | ||
| try { | ||
| setFocus(focusTarget, { shouldSelect: true }) | ||
| } catch (error) { | ||
| console.warn('Failed to focus field', focusTarget, error) | ||
| } | ||
| }) | ||
| }, | ||
| [setFocus] | ||
| ) |
There was a problem hiding this comment.
The focusField function redundantly calls requestAnimationFrame or setTimeout on every invocation even when the field parameter is undefined or when setFocus would immediately return. Consider moving the early return check before setting up the animation frame callback to avoid unnecessary scheduling when there's no field to focus.
📑 개요
Multi-step Form의 웹 접근성 및 사용성을 개선하고, SSR 환경에서의 스타일 깜빡임(FOUC) 현상을 해결했습니다.
🔑 주요 구현
1. FOUC 해결
_document에서 생성한emotionCache를_app의 Props로 직접 주입하여, 서버에서 생성된 스타일이 클라이언트에 정상적으로 적용되도록 수정했습니다.2. UX 및 웹 접근성 개선
Enter키 입력 시 즉시 제출(Submit)되는 것을 막고, 다음 필드로 포커스를 이동시켜 오작동을 방지했습니다.RHFSegmented)에aria-labelledby속성을 적용하고, 포커스 스타일(outline)을 추가했습니다.🤔 고민했던 지점
범용적인 포커스 관리 로직과 타입 안전성
step-navigator와use-multi-step-form에서 다양한 형태의 Input(특히 Field Array나 커스텀 컴포넌트)의 에러 필드를 추적하고 포커싱하는 기능을 구현하는 과정에서 고민이 있었습니다.최대한 범용적으로 동작하게 하려다 보니 DOM 요소를 탐색하는 과정에서
as를 사용한 타입 단언이 필요했고, 특정 케이스(quotes)를 처리하는 로직이 다소 선언적이지 못하게 구현되었습니다. 이 부분은 추후 더 타입 안전하면서도 깔끔한 구조로 개선할 수 있을지 지속적으로 고민해볼 예정입니다.