modal은 modal-root에서 렌더링하도록 Portal 생성#16
Conversation
Summary of ChangesHello, 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! 이 PR은 애플리케이션의 모달 관리 방식을 전반적으로 개선합니다. 기존에 페이지별로 분산되어 있던 모달 로직을 재사용 가능한 공통 컴포넌트로 통합하여 코드의 응집도를 높이고 유지보수를 용이하게 합니다. 특히, Highlights
Changelog
Activity
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}
| const getFocusableElements = () => { | ||
| if (!dialogRef.current) return [] | ||
|
|
||
| const focusables = dialogRef.current.querySelectorAll<HTMLElement>( | ||
| 'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])', | ||
| ) | ||
|
|
||
| return Array.from(focusables) | ||
| } | ||
|
|
||
| const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => { | ||
| if (event.key === 'Escape') { | ||
| event.preventDefault() | ||
| onClose() | ||
| return | ||
| } | ||
|
|
||
| if (event.key !== 'Tab') return | ||
|
|
||
| const focusables = getFocusableElements() | ||
| if (focusables.length === 0) { | ||
| event.preventDefault() | ||
| dialogRef.current?.focus() | ||
| return | ||
| } | ||
|
|
||
| const first = focusables[0] | ||
| const last = focusables[focusables.length - 1] | ||
| const active = document.activeElement | ||
|
|
||
| if (event.shiftKey && active === first) { | ||
| event.preventDefault() | ||
| last.focus() | ||
| return | ||
| } | ||
|
|
||
| if (!event.shiftKey && active === last) { | ||
| event.preventDefault() | ||
| first.focus() | ||
| } | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| if (!open) return | ||
|
|
||
| const appRoot = document.getElementById('app-root') | ||
| const previousOverflow = document.body.style.overflow | ||
| previouslyFocusedElementRef.current = | ||
| document.activeElement instanceof HTMLElement ? document.activeElement : null | ||
|
|
||
| appRoot?.setAttribute('aria-hidden', 'true') | ||
| appRoot?.setAttribute('inert', '') | ||
| document.body.style.overflow = 'hidden' | ||
|
|
||
| const focusables = getFocusableElements() | ||
| if (focusables.length > 0) { | ||
| focusables[0].focus() | ||
| } else { | ||
| dialogRef.current?.focus() | ||
| } | ||
|
|
||
| return () => { | ||
| appRoot?.removeAttribute('aria-hidden') | ||
| appRoot?.removeAttribute('inert') | ||
| document.body.style.overflow = previousOverflow | ||
| previouslyFocusedElementRef.current?.focus() | ||
| } | ||
| }, [open]) |
There was a problem hiding this comment.
getFocusableElements와 handleKeyDown 함수가 컴포넌트 렌더링 시마다 재생성되고 있습니다. 이는 성능에 영향을 줄 수 있으며, useEffect와 같은 React Hook에서 의존성 문제를 야기할 수 있습니다.
또한, useEffect Hook이 외부 스코프의 getFocusableElements 함수를 사용하고 있지만 의존성 배열에 포함하지 않아, 잠재적인 버그의 원인이 될 수 있습니다. (stale closure)
이 문제들을 해결하기 위해 useCallback을 사용하여 함수들을 메모이제이션하고, useEffect의 의존성 배열을 올바르게 수정하는 것을 제안합니다. useCallback을 react에서 import하는 것을 잊지 마세요.
const getFocusableElements = useCallback(() => {
if (!dialogRef.current) return []
const focusables = dialogRef.current.querySelectorAll<HTMLElement>(
'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])',
)
return Array.from(focusables)
}, [])
const handleKeyDown = useCallback(
(event: KeyboardEvent<HTMLDivElement>) => {
if (event.key === 'Escape') {
event.preventDefault()
onClose()
return
}
if (event.key !== 'Tab') return
const focusables = getFocusableElements()
if (focusables.length === 0) {
event.preventDefault()
dialogRef.current?.focus()
return
}
const first = focusables[0]
const last = focusables[focusables.length - 1]
const active = document.activeElement
if (event.shiftKey && active === first) {
event.preventDefault()
last.focus()
return
}
if (!event.shiftKey && active === last) {
event.preventDefault()
first.focus()
}
},
[onClose, getFocusableElements],
)
useEffect(() => {
if (!open) return
const appRoot = document.getElementById('app-root')
const previousOverflow = document.body.style.overflow
previouslyFocusedElementRef.current =
document.activeElement instanceof HTMLElement ? document.activeElement : null
appRoot?.setAttribute('aria-hidden', 'true')
appRoot?.setAttribute('inert', '')
document.body.style.overflow = 'hidden'
const focusables = getFocusableElements()
if (focusables.length > 0) {
focusables[0].focus()
} else {
dialogRef.current?.focus()
}
return () => {
appRoot?.removeAttribute('aria-hidden')
appRoot?.removeAttribute('inert')
document.body.style.overflow = previousOverflow
previouslyFocusedElementRef.current?.focus()
}
}, [open, getFocusableElements])
1 participant
#️⃣연관된 이슈
#15
📝작업 내용
목적
header z-50충돌을z-index의존 없이 해결변경 파일
app/layout.tsxshared/ui/portal.tsxshared/ui/modal.tsxapp/group/[id]/members/page.tsx핵심 변경
app/layout.tsx#app-root,#modal-root분리#app-root에isolate적용shared/ui/portal.tsxmodal-root로 변경shared/ui/modal.tsx(신규)role="dialog",aria-modal,aria-labelledby,aria-describedbyaria-hidden+inert처리app/group/[id]/members/page.tsxModal컴포넌트 소비 형태로 단순화이슈/해결
header의z-50이 모달 위에 표시됨app-root를isolate로 분리하고, 모달은modal-root에 렌더스크린샷 (선택)