Feat/component/modal/DEVING-38 #14
Conversation
Walkthroughμ΄ ν 리νμ€νΈλ μλ‘μ΄ λͺ¨λ¬ κ΄λ ¨ UI λ° νμ΄μ§ μ»΄ν¬λνΈλ₯Ό μΆκ°ν©λλ€. Changes
Sequence Diagram(s)sequenceDiagram
participant U as μ¬μ©μ
participant S as Some μ»΄ν¬λνΈ
participant M as Modal
U->>S: "νλ‘ν 보기" λ²νΌ ν΄λ¦
S->>S: handleProfileClick() μ€ν
S->>M: λͺ¨λ¬ μ΄κΈ° (μ νλ μ¬μ©μ λ°μ΄ν° μ λ¬)
U->>M: "νμΈ" λ²νΌ ν΄λ¦
M->>S: handleSecondModalConfirm() μ€ν
S->>M: λͺ¨λ¬ λ«κΈ°
sequenceDiagram
participant U as μ¬μ©μ
participant P as ModalTestPage
participant M1 as 첫 λ²μ§Έ λͺ¨λ¬ (Some ν¬ν¨)
participant M2 as λ λ²μ§Έ λͺ¨λ¬ (ν
μ€νΈ λ©μμ§)
participant M3 as μΈ λ²μ§Έ λͺ¨λ¬ (λ«κΈ° μ μ©)
U->>P: 첫 λ²μ§Έ λ²νΌ ν΄λ¦
P->>M1: 첫 λ²μ§Έ λͺ¨λ¬ μ΄κΈ°
U->>M1: νμΈ/μ·¨μ λμ
M1->>P: 첫 λ²μ§Έ λͺ¨λ¬ λ«κΈ°
U->>P: λ λ²μ§Έ λ²νΌ ν΄λ¦
P->>M2: λ λ²μ§Έ λͺ¨λ¬ μ΄κΈ°
U->>M2: μ·¨μ λμ
M2->>P: λ λ²μ§Έ λͺ¨λ¬ λ«κΈ°
U->>P: μΈ λ²μ§Έ λ²νΌ ν΄λ¦
P->>M3: μΈ λ²μ§Έ λͺ¨λ¬ μ΄κΈ°
U->>M3: λ«κΈ° λμ
M3->>P: μΈ λ²μ§Έ λͺ¨λ¬ λ«κΈ°
Suggested labels
Suggested reviewers
Poem
πͺ§ TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git βοΈ
|
Codecov ReportAll modified and coverable lines are covered by tests β |
CI Status Reportκ²μ¬ κ²°κ³Ό
β λͺ¨λ κ²μ¬κ° ν΅κ³Όλμμ΅λλ€. |
CI Status Reportκ²μ¬ κ²°κ³Ό
β λͺ¨λ κ²μ¬κ° ν΅κ³Όλμμ΅λλ€. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
π§Ή Nitpick comments (2)
src/app/preview/modal/page.tsx (1)
8-14: μν κ΄λ¦¬ μ΅μ ν μ μ
handleConfirmν¨μκ° μ²« λ²μ§Έ λͺ¨λ¬μμλ§ μ¬μ©λκ³ μμΌλ©°, λ λ²μ§Έ λͺ¨λ¬μμλ λμΌν ν¨μλ₯Ό μ¬μ©νκ³ μμ΅λλ€. μ΄λ μ μ¬μ μΌλ‘ νΌλμ μΌκΈ°ν μ μμ΅λλ€.κ° λͺ¨λ¬μ λν΄ λ³λμ
handleConfirmν¨μλ₯Ό λ§λ€κ±°λ, λͺ¨λ¬ IDλ₯Ό 맀κ°λ³μλ‘ λ°λ λ¨μΌ ν¨μλ₯Ό μ¬μ©νλ κ²μ κ³ λ €ν΄λ³΄μΈμ:-const handleConfirm = () => { - setIsModalOpen(false); -}; +const handleModalConfirm = (modalId: number) => { + if (modalId === 1) { + setIsModalOpen(false); + } else { + setIsModalOpen2(false); + } +};src/components/ui/modal/alert/Modal.tsx (1)
92-99: λ²νΌ μ κ·Όμ± λ° μλ―Έλ‘ μ λ§ν¬μ κ°μ νμλ²νΌμ μ μ ν
aria-labelμ΄ μμΌλ©°, νμΈ λ²νΌμ μλ―Έκ° λΆλͺ νν μ μμ΅λλ€.λ€μκ³Ό κ°μ΄ μμ νμΈμ:
<Button onClick={onClose} variant={'outline'} type="button" + aria-label={`λͺ¨λ¬ ${cancelText}`} > {cancelText} </Button> <Button onClick={onConfirm} type="button" + aria-label={`λͺ¨λ¬ ${confirmText}`} > {confirmText} </Button>
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (4)
src/app/preview/modal/Some.tsx(1 hunks)src/app/preview/modal/page.tsx(1 hunks)src/components/ui/modal/alert/Modal.tsx(1 hunks)tailwind.config.ts(2 hunks)
π§° Additional context used
πͺ ESLint
tailwind.config.ts
[error] 18-18: Delete Β·
(prettier/prettier)
β° Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: check
π Additional comments (2)
src/app/preview/modal/Some.tsx (1)
90-103: μμ μ κ·Όμ± κ²μ¦ νμμν νμ μμμ΄ μ κ·Όμ± κΈ°μ€μ λ§μ‘±νλμ§ νμΈμ΄ νμν©λλ€.
λ€μ μ€ν¬λ¦½νΈλ₯Ό μ€ννμ¬ μμ λλΉλ₯Ό νμΈνμΈμ:
#!/usr/bin/python3 import colorsys import math def calculate_contrast_ratio(color1, color2): # Convert hex to RGB def hex_to_rgb(hex_color): hex_color = hex_color.lstrip('#') return tuple(int(hex_color[i:i+2], 16) for i in (0, 2, 4)) # Calculate relative luminance def get_luminance(rgb): rgb = [v/255 for v in rgb] rgb = [v/12.92 if v <= 0.03928 else ((v+0.055)/1.055)**2.4 for v in rgb] return 0.2126 * rgb[0] + 0.7152 * rgb[1] + 0.0722 * rgb[2] rgb1 = hex_to_rgb(color1) rgb2 = hex_to_rgb(color2) l1 = get_luminance(rgb1) l2 = get_luminance(rgb2) ratio = (max(l1, l2) + 0.05) / (min(l1, l2) + 0.05) return ratio # Background color bg = '#0F0F0F' # Status colors colors = { 'μΉμΈ': '#22C55E', # green-600 'λκΈ°': '#CA8A04', # yellow-600 'κ±°μ ': '#DC2626', # red-600 'κ°ν΄': '#4B5563' # gray-600 } print("μμ λλΉμ¨ κ²μ¬ κ²°κ³Ό:") for status, color in colors.items(): ratio = calculate_contrast_ratio(bg, color) meets_wcag = "μΆ©μ‘±" if ratio >= 4.5 else "λ―ΈμΆ©μ‘±" print(f"{status}: {ratio:.2f}:1 (WCAG AA κΈ°μ€ {meets_wcag})")tailwind.config.ts (1)
18-18: μμ κ° ν¬λ§·ν κ°μ λ¨
BG_2μμ κ°μ΄ μ¬λ°λ₯Έ HEX νμμΌλ‘ μμ λμμ΅λλ€. λΆνμν 곡백λ μ κ±°λμμ΅λλ€.Also applies to: 35-35
π§° Tools
πͺ ESLint
[error] 18-18: Delete
Β·(prettier/prettier)
| <div className="flex min-h-screen flex-col items-center justify-center gap-4 bg-BG p-4"> | ||
| <Button onClick={() => setIsModalOpen(true)}>μ΄μ€ λͺ¨λ¬</Button> | ||
| <Button onClick={() => setIsModalOpen2(true)}>κ·Έλ₯ λͺ¨λ¬</Button> | ||
| <Modal | ||
| isOpen={isModalOpen} | ||
| onClose={() => setIsModalOpen(false)} | ||
| onConfirm={handleConfirm} | ||
| confirmText="μμ " | ||
| cancelText="μ·¨μ" | ||
| modalClassName="w-96" | ||
| > | ||
| <Some /> | ||
| </Modal> | ||
| <Modal | ||
| isOpen={isModalOpen2} | ||
| onClose={() => setIsModalOpen2(false)} | ||
| onConfirm={handleConfirm} | ||
| confirmText="μμ " | ||
| cancelText="μ·¨μ" | ||
| modalClassName="w-96" | ||
| > | ||
| <p className="text-white">μλ νμΈμ©μ©</p> | ||
| </Modal> | ||
| </div> |
There was a problem hiding this comment.
π οΈ Refactor suggestion
μ κ·Όμ± κ°μ νμ
λͺ¨λ¬μ μ¬λ λ²νΌλ€μ λν μ κ·Όμ± μμ±μ΄ λΆμ‘±ν©λλ€.
λ²νΌμ μ μ ν aria-labelκ³Ό aria-controls μμ±μ μΆκ°νμ¬ μ κ·Όμ±μ κ°μ νμΈμ:
-<Button onClick={() => setIsModalOpen(true)}>μ΄μ€ λͺ¨λ¬</Button>
-<Button onClick={() => setIsModalOpen2(true)}>κ·Έλ₯ λͺ¨λ¬</Button>
+<Button
+ onClick={() => setIsModalOpen(true)}
+ aria-label="μ΄μ€ λͺ¨λ¬ μ΄κΈ°"
+ aria-controls="dual-modal"
+>
+ μ΄μ€ λͺ¨λ¬
+</Button>
+<Button
+ onClick={() => setIsModalOpen2(true)}
+ aria-label="κΈ°λ³Έ λͺ¨λ¬ μ΄κΈ°"
+ aria-controls="simple-modal"
+>
+ κ·Έλ₯ λͺ¨λ¬
+</Button>λν κ° Modal μ»΄ν¬λνΈμ κ³ μ ν idλ₯Ό μΆκ°νλ κ²μ΄ μ’μ΅λλ€:
<Modal
+ id="dual-modal"
isOpen={isModalOpen}
...
>
<Modal
+ id="simple-modal"
isOpen={isModalOpen2}
...
>π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="flex min-h-screen flex-col items-center justify-center gap-4 bg-BG p-4"> | |
| <Button onClick={() => setIsModalOpen(true)}>μ΄μ€ λͺ¨λ¬</Button> | |
| <Button onClick={() => setIsModalOpen2(true)}>κ·Έλ₯ λͺ¨λ¬</Button> | |
| <Modal | |
| isOpen={isModalOpen} | |
| onClose={() => setIsModalOpen(false)} | |
| onConfirm={handleConfirm} | |
| confirmText="μμ " | |
| cancelText="μ·¨μ" | |
| modalClassName="w-96" | |
| > | |
| <Some /> | |
| </Modal> | |
| <Modal | |
| isOpen={isModalOpen2} | |
| onClose={() => setIsModalOpen2(false)} | |
| onConfirm={handleConfirm} | |
| confirmText="μμ " | |
| cancelText="μ·¨μ" | |
| modalClassName="w-96" | |
| > | |
| <p className="text-white">μλ νμΈμ©μ©</p> | |
| </Modal> | |
| </div> | |
| <div className="flex min-h-screen flex-col items-center justify-center gap-4 bg-BG p-4"> | |
| <Button | |
| onClick={() => setIsModalOpen(true)} | |
| aria-label="μ΄μ€ λͺ¨λ¬ μ΄κΈ°" | |
| aria-controls="dual-modal" | |
| > | |
| μ΄μ€ λͺ¨λ¬ | |
| </Button> | |
| <Button | |
| onClick={() => setIsModalOpen2(true)} | |
| aria-label="κΈ°λ³Έ λͺ¨λ¬ μ΄κΈ°" | |
| aria-controls="simple-modal" | |
| > | |
| κ·Έλ₯ λͺ¨λ¬ | |
| </Button> | |
| <Modal | |
| id="dual-modal" | |
| isOpen={isModalOpen} | |
| onClose={() => setIsModalOpen(false)} | |
| onConfirm={handleConfirm} | |
| confirmText="μμ " | |
| cancelText="μ·¨μ" | |
| modalClassName="w-96" | |
| > | |
| <Some /> | |
| </Modal> | |
| <Modal | |
| id="simple-modal" | |
| isOpen={isModalOpen2} | |
| onClose={() => setIsModalOpen2(false)} | |
| onConfirm={handleConfirm} | |
| confirmText="μμ " | |
| cancelText="μ·¨μ" | |
| modalClassName="w-96" | |
| > | |
| <p className="text-white">μλ νμΈμ©μ©</p> | |
| </Modal> | |
| </div> |
There was a problem hiding this comment.
ariaμμ±κ³Ό μ κ·Όμ±μ λνμ 보, κ·Έλ¦¬κ³ Modalμ Idλ₯Ό μΆκ°ν΄μΌνλ μ΄μ
There was a problem hiding this comment.
𧩠Analysis chain
@clyde-yoonjae ARIA(Accessible Rich Internet Applications) μμ±κ³Ό μ κ·Όμ±μ λν΄ μ€λͺ λλ¦¬κ² μ΅λλ€:
-
ARIA μμ±μ΄λ?
- ARIAλ μ€ν¬λ¦° 리λμ κ°μ 보쑰 κΈ°μ μ΄ μΉ μ½ν μΈ λ₯Ό λ μ μ΄ν΄ν μ μλλ‘ λλ μΆκ° μ 보λ₯Ό μ 곡ν©λλ€
aria-label: μμμ λν μ€λͺ μ μ 곡 (μ: "μ΄μ€ λͺ¨λ¬ μ΄κΈ°")aria-controls: νμ¬ μμκ° μ μ΄νλ λ€λ₯Έ μμμμ κ΄κ³λ₯Ό λͺ μ
-
λͺ¨λ¬ IDκ° νμν μ΄μ
aria-controlsκ° μ λλ‘ μλνλ €λ©΄ μ μ΄ λμ μμ(λͺ¨λ¬)μ κ³ μ ν IDκ° νμν©λλ€- μ€ν¬λ¦° 리λλ μ΄ IDλ₯Ό ν΅ν΄ λ²νΌκ³Ό λͺ¨λ¬μ κ΄κ³λ₯Ό νμ ν μ μμ΅λλ€
- μ: λ²νΌμ
aria-controls="dual-modal"μ IDκ° "dual-modal"μΈ λͺ¨λ¬μ μ μ΄νλ€λ κ²μ λνλ λλ€
-
μ κ·Όμ± κ°μ ν¨κ³Ό
- μκ° μ₯μ μΈ μ¬μ©μκ° μ΄λ€ λ²νΌμ΄ μ΄λ€ λͺ¨λ¬μ μ μ΄νλμ§ μ΄ν΄ν μ μμ΅λλ€
- ν€λ³΄λ μ¬μ©μκ° λͺ¨λ¬κ³Ό κ΄λ ¨ 컨νΈλ‘€μ λ μ½κ² νμν μ μμ΅λλ€
- μ λ°μ μΈ μ¬μ©μ κ²½νμ΄ ν₯μλ©λλ€
(_/)
(^.^)
(> <) π μλ‘μ΄ μ§μμ λ°°μ μ΄μ!
π Web query:
What are ARIA attributes and why are they important for accessibility?
π‘ Result:
ARIA (Accessible Rich Internet Applications) attributes are a set of HTML extensions developed by the World Wide Web Consortium (W3C) to enhance web accessibility for users with disabilities[1][4][6]. They provide supplemental semantic information about elements' roles, states, and properties to assistive technologies like screen readers[5][8].
Key Components of ARIA
Roles define an element's purpose (e.g., role="button" converts a <div> into a button for screen readers)[5][9].
Properties describe characteristics (aria-label, aria-describedby) that help establish relationships between elements[5][7].
States indicate dynamic conditions (aria-expanded="true", aria-checked="false") that update during user interactions[5][9].
Why ARIA Matters for Accessibility
- Bridges HTML gaps: Enables accessible implementation of complex widgets (menus, sliders) that lack native HTML equivalents[4][6].
- Enhances dynamic content: Makes live updates (e.g., chat messages) accessible through attributes like
aria-live[3][6]. - Improves navigation: Landmark roles (
navigation,main) help users quickly locate page sections[3][6]. - Supports WCAG compliance: Helps meet accessibility standards by:
- Providing text alternatives via
aria-label[3] - Indicating required fields with
aria-required[3] - Managing focus states in modals (
aria-modal="true")[9]
- Providing text alternatives via
Best Practices
- Use native HTML first: Prefer semantic elements (
<button>) over ARIA-enhanced<div>s when possible[4][8]. - Test thoroughly: 41% more errors occur on pages with poorly implemented ARIA compared to those without[4].
- Layer progressively: Add roles/states only when necessary to avoid overwhelming users[2][5].
Example of proper ARIA usage:
<div role="checkbox" aria-checked="false" tabindex="0">
Receive newsletter
</div>This creates a checkbox component accessible via keyboard and screen readers[9].
ARIA's importance lies in its ability to make modern web applications perceivable and operable for users who rely on assistive technologies while maintaining compatibility with evolving web standards[1][6][8]. When implemented correctly alongside semantic HTML, it creates equitable digital experiences without compromising functionality[2][4].
Citations:
- 1: https://accessibe.com/glossary/aria
- 2: https://www.accessi.org/blog/the-role-of-aria-in-web-accessibility/
- 3: https://top5accessibility.com/blog/mastering-aria-attributes-clearer-coding-for-better-web-accessibility/
- 4: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA
- 5: https://web.dev/learn/accessibility/aria-html
- 6: https://www.w3.org/WAI/standards-guidelines/aria/
- 7: https://prismic.io/blog/what-is-aria
- 8: https://www.smashingmagazine.com/2022/09/wai-aria-guide/
- 9: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes
- 10: https://www.w3.org/TR/html-aria/
@clyde-yoonjae ARIAμ μ κ·Όμ±μ λν΄ λ μμΈν μ€λͺ λλ¦¬κ² μ΅λλ€:
-
ARIA (Accessible Rich Internet Applications)
- W3Cμμ κ°λ°ν HTML νμ₯ κ·κ²©μ λλ€
- μΈ κ°μ§ μ£Όμ κ΅¬μ± μμκ° μμ΅λλ€:
- Roles: μμμ λͺ©μ μ μ (μ:
role="dialog"for λͺ¨λ¬) - Properties: μμμ νΉμ± μ€λͺ
(
aria-label,aria-controls) - States: λμ μν νμ (
aria-expanded,aria-hidden)
- Roles: μμμ λͺ©μ μ μ (μ:
-
λͺ¨λ¬μμ ARIA μμ±μ μ€μμ±
<!-- μ’μ μμ --> <button aria-label="μ΄μ€ λͺ¨λ¬ μ΄κΈ°" aria-controls="dual-modal"> μ΄μ€ λͺ¨λ¬ </button> <div id="dual-modal" role="dialog" aria-modal="true" aria-labelledby="modal-title" > <!-- λͺ¨λ¬ λ΄μ© --> </div>
-
IDκ° νμν μ΄μ
aria-controlsμaria-labelledbyκ°μ ARIA κ΄κ³ μμ±λ€μ΄ μ λλ‘ μλνλ €λ©΄ λμ μμμ IDκ° νμν©λλ€- μ€ν¬λ¦° 리λκ° μ΄ κ΄κ³λ₯Ό μΈμνκ³ μ¬μ©μμκ² μ μ ν 컨ν μ€νΈλ₯Ό μ 곡ν μ μμ΅λλ€
-
λͺ¨λ² μ¬λ‘
- κ°λ₯ν ν μλ§¨ν± HTML μμλ₯Ό λ¨Όμ μ¬μ©νμΈμ
- ARIAλ HTMLλ§μΌλ‘ μΆ©λΆνμ§ μμ λ 보μμ μΌλ‘ μ¬μ©ν©λλ€
- λͺ¨λ¬ ꡬν μ
role="dialog"μaria-modal="true"λ₯Ό ν¨κ» μ¬μ©νλ©΄ μ’μ΅λλ€
(_/)
(^.^)
(> <) π μ κ·Όμ±μ μΈκ³λ μ λ§ κΉλ€μ!
βοΈ Learnings added
Learnt from: clyde-yoonjae
PR: MoimService/Moim-FE#14
File: src/app/preview/modal/page.tsx:17-40
Timestamp: 2025-02-19T02:34:10.245Z
Learning: ARIA μμ±μ μΉ μ κ·Όμ±μ ν₯μμν€λ μ€μν λꡬμ
λλ€:
- aria-label: μμμ λν μ€λͺ
μ μΈ λ μ΄λΈμ μ 곡
- aria-controls: νμ¬ μμκ° μ μ΄νλ λ€λ₯Έ μμμμ κ΄κ³λ₯Ό λͺ
μ
λͺ¨λ¬ μ»΄ν¬λνΈμλ νμ κ³ μ ν IDλ₯Ό λΆμ¬νκ³ , ν΄λΉ λͺ¨λ¬μ μ μ΄νλ λ²νΌμλ aria-controls μμ±μΌλ‘ λͺ¨λ¬ IDλ₯Ό μ°κ²°ν΄μΌ ν©λλ€.
Learnt from: clyde-yoonjae
PR: MoimService/Moim-FE#14
File: src/app/preview/modal/page.tsx:17-40
Timestamp: 2025-02-19T02:34:10.245Z
Learning: ARIA μμ±μ μΈ κ°μ§ μ£Όμ μΉ΄ν
κ³ λ¦¬λ‘ κ΅¬λΆλ©λλ€:
1. Roles: μμμ λͺ©μ μ μ (μ: role="dialog")
2. Properties: μμμ νΉμ± μ€λͺ
(μ: aria-label, aria-controls)
3. States: λμ μν νμ (μ: aria-expanded, aria-hidden)
λͺ¨λ¬ ꡬν μ νμ ARIA μμ±:
- λͺ¨λ¬ νΈλ¦¬κ±° λ²νΌ: aria-label, aria-controls
- λͺ¨λ¬ 컨ν
μ΄λ: role="dialog", aria-modal="true", aria-labelledby
λͺ¨λ¬κ³Ό κ΄λ ¨ μμλ€μ λ°λμ κ³ μ ν IDλ₯Ό κ°μ ΈμΌ νλ©°, μ΄λ ARIA κ΄κ³ μμ±λ€μ΄ μ¬λ°λ₯΄κ² μλνκΈ° μν΄ νμν©λλ€.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| <div | ||
| className="fixed inset-0 z-50 flex items-center justify-center bg-BG_2 bg-opacity-50" | ||
| onClick={handleBackdropClick} | ||
| onKeyDown={handleKeyDown} | ||
| role="button" | ||
| tabIndex={0} | ||
| aria-label="Close modal" | ||
| > |
There was a problem hiding this comment.
backdropμ μ κ·Όμ± λ° μ΄λ²€νΈ μ²λ¦¬ κ°μ νμ
backdrop divμ role="button"μ μ¬μ©νλ κ²μ μ μ νμ§ μμ΅λλ€. λν ν€λ³΄λ μ΄λ²€νΈ μ²λ¦¬κ° λΆμμ ν©λλ€.
λ€μκ³Ό κ°μ΄ μμ νμΈμ:
<div
className="fixed inset-0 z-50 flex items-center justify-center bg-BG_2 bg-opacity-50"
onClick={handleBackdropClick}
onKeyDown={handleKeyDown}
- role="button"
+ role="presentation"
tabIndex={0}
- aria-label="Close modal"
>π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| className="fixed inset-0 z-50 flex items-center justify-center bg-BG_2 bg-opacity-50" | |
| onClick={handleBackdropClick} | |
| onKeyDown={handleKeyDown} | |
| role="button" | |
| tabIndex={0} | |
| aria-label="Close modal" | |
| > | |
| <div | |
| className="fixed inset-0 z-50 flex items-center justify-center bg-BG_2 bg-opacity-50" | |
| onClick={handleBackdropClick} | |
| onKeyDown={handleKeyDown} | |
| role="presentation" | |
| tabIndex={0} | |
| > |
There was a problem hiding this comment.
tabIndexλ μνΈμμ© κ°λ₯ν μμμΈ λ²νΌμ΄λ λ§ν¬μλ§ λ¬ μ μλλ° μ΄λ κ² μμ νλκ² λ§μκΉ?
There was a problem hiding this comment.
𧩠Analysis chain
@clyde-yoonjae λ€, μ’μ μ§μ μ
λλ€! backdrop divμλ tabIndexκ° νμνμ§ μμ΅λλ€. λμ λ€μκ³Ό κ°μ΄ μμ νλ κ²μ΄ μ’μ΅λλ€:
- backdrop divμμ
tabIndexμrole="button"μ μ κ±° - μ€μ modal containerμ μ μ ν ARIA μμ± μΆκ°
// backdrop
<div
className="fixed inset-0 z-50 flex items-center justify-center bg-BG_2 bg-opacity-50"
onClick={handleBackdropClick}
role="presentation"
>
{/* modal container */}
<div
role="dialog"
aria-modal="true"
className={`relative bg-white rounded-lg ${modalClassName}`}
>
{children}
</div>
</div>μ΄λ κ² νλ©΄ μ€ν¬λ¦° 리λ μ¬μ©μλ€μκ² λ λͺ νν μλ―Έλ₯Ό μ λ¬ν μ μμ΅λλ€.
π Web query:
ARIA modal dialog best practices for backdrop and focus management
π‘ Result:
To create accessible ARIA modal dialogs with proper backdrop handling and focus management, follow these key practices:
Backdrop Implementation
- Use nested
<div>elements when working with native<dialog>to independently control modal window and backdrop styling/behavior[2][10] - Prevent background interaction by:
- Applying
aria-hidden="true"to non-dialog content - Setting
tabindex="-1"on interactive background elements[1][4] - Disabling scroll via CSS (
overflow: hiddenon body)[2][10]
- Applying
<div class="backdrop" aria-hidden="true">
<div role="dialog" aria-modal="true">
<!-- Dialog content -->
</div>
</div>Focus Management
-
Trap keyboard focus using JavaScript to:
- Cycle TAB within modal boundaries[5][7]
- Close on ESC key press[3][10]
-
Initial focus placement:
- First interactive element (default for
<dialog>)[3] - Static content container (
tabindex="-1") when needing introductory context[2][7]
- First interactive element (default for
// Focus trapping example
document.addEventListener('keydown', (e) => {
if (e.key === 'Tab' && modalOpen) {
// Cycle focus within modal
}
});ARIA Attributes Configuration
| Attribute | Purpose | Example |
|---|---|---|
role="dialog" |
Identifies dialog container | <div role="dialog"> |
aria-modal="true" |
Declares modal nature | Mandatory for all modals[4] |
aria-labelledby |
Associates dialog title | Points to heading ID[10] |
aria-describedby |
Optional content description | Points to content ID[7] |
Visual/Interaction Requirements
- Backdrop styling should visually distinguish modal content:
.dialog-backdrop {
position: fixed;
background: rgba(0,0,0,0.5);
inset: 0;
}- Close functionality must include:
- Visible close button (X icon)
- ESC key support[3][5]
- Backdrop click (optional)[2]
When implementing modals, always test with screen readers like NVDA/VoiceOver and follow WAI-ARIA Authoring Practices[7]. Native <dialog> element usage is recommended when possible due to built-in browser accessibility features[3][9], but custom implementations require careful attention to focus trapping and ARIA semantics[1][4].
Citations:
- 1: https://www.deque.com/blog/aria-modal-alert-dialogs-a11y-support-series-part-2/
- 2: https://www.a11y-collective.com/blog/modal-accessibility/
- 3: https://www.freecodecamp.org/news/how-to-build-an-accessible-modal-with-example-code/
- 4: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-modal
- 5: https://clearleft.com/thinking/accessibility-in-modal-dialogs
- 6: https://www.w3.org/WAI/GL/wiki/Using_ARIA_role=dialog_to_implement_a_modal_dialog_box
- 7: https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/
- 8: Dialog (Modal) Pattern. A question of modality and focus managementΒ w3c/aria-practices#2924
- 9: https://knowledge.evinced.com/components/modal/
- 10: https://universaldesign.ie/communications-digital/web-and-mobile-accessibility/web-accessibility-techniques/developers-introduction-and-index/ensure-custom-widgets-are-accessible/create-accessible-modal-dialogues
@clyde-yoonjae μ£μ‘ν©λλ€. μ΄μ μ μμ΄ λΆμμ νλ€μ. ARIA λ² μ€νΈ νλν°μ€μ λ°λ₯΄λ©΄, λͺ¨λ¬ ꡬνμ λ€μκ³Ό κ°μ ν¨ν΄μ μ¬μ©ν΄μΌ ν©λλ€:
const Modal: React.FC<AlertModalProps> = ({
isOpen,
onClose,
onConfirm,
confirmText = 'νμΈ',
cancelText = 'μ·¨μ',
children,
modalClassName = '',
contentClassName = '',
buttonClassName = '',
}) => {
// ... existing code ...
return (
// backdrop
<div
className="fixed inset-0 z-50 flex items-center justify-center bg-BG_2 bg-opacity-50"
onClick={handleBackdropClick}
aria-hidden="true"
>
{/* modal container */}
<div
role="dialog"
aria-modal="true"
aria-labelledby="modal-title"
className={`relative bg-white rounded-lg ${modalClassName}`}
>
<h2 id="modal-title" className="sr-only">λͺ¨λ¬ λ€μ΄μΌλ‘κ·Έ</h2>
<div className={contentClassName}>
{children}
</div>
<div className={buttonClassName}>
<Button onClick={onConfirm}>{confirmText}</Button>
<Button onClick={onClose}>{cancelText}</Button>
</div>
</div>
</div>
);
};μ£Όμ λ³κ²½μ¬ν:
- backdrop divμμ
tabIndexμroleμ κ±°νκ³ λμaria-hidden="true"μΆκ° - λͺ¨λ¬ 컨ν
μ΄λμ μ μ ν ARIA μμ±λ€ μΆκ° (
role="dialog",aria-modal="true",aria-labelledby) - μ€ν¬λ¦° 리λλ₯Ό μν μ λͺ© μΆκ°
μΆκ°λ‘ ν¬μ»€μ€ κ΄λ¦¬λ₯Ό μν λ‘μ§λ νμνμ§λ§, 그건 λ³λμ PRμμ λ€λ£¨λ κ²μ΄ μ’μ κ² κ°μ΅λλ€.
βοΈ Learnings added
Learnt from: clyde-yoonjae
PR: MoimService/Moim-FE#14
File: src/components/ui/modal/alert/Modal.tsx:75-82
Timestamp: 2025-02-19T04:05:58.610Z
Learning: Modal backdrop divλ `tabIndex`λ `role="button"`μ μ¬μ©νμ§ μμμΌ νλ©°, λμ `role="presentation"`μ μ¬μ©ν©λλ€. μ€μ modal containerμ `role="dialog"` λ° `aria-modal="true"`λ₯Ό μ μ©νμ¬ μ κ·Όμ±μ 보μ₯ν΄μΌ ν©λλ€.
Learnt from: clyde-yoonjae
PR: MoimService/Moim-FE#14
File: src/components/ui/modal/alert/Modal.tsx:75-82
Timestamp: 2025-02-19T04:05:58.610Z
Learning: λͺ¨λ¬ ꡬνμ μ κ·Όμ±μ μν ARIA μμ± κ°μ΄λ:
1. Backdrop divλ `aria-hidden="true"`λ₯Ό μ¬μ©νκ³ `tabIndex`λ `role` μμ±μ μ¬μ©νμ§ μμ
2. λͺ¨λ¬ 컨ν
μ΄λλ `role="dialog"`, `aria-modal="true"`, `aria-labelledby` μμ±μ ν¬ν¨ν΄μΌ ν¨
3. μ€ν¬λ¦° 리λλ₯Ό μν μ λͺ©μ μ 곡νκ³ `aria-labelledby`λ‘ μ°κ²°ν΄μΌ ν¨
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| <div | ||
| className={`rounded-lg bg-BG_2 shadow-xl ${modalClassName}`} | ||
| role="dialog" | ||
| aria-modal="true" | ||
| tabIndex={-1} | ||
| > |
There was a problem hiding this comment.
π οΈ Refactor suggestion
λͺ¨λ¬ λ€μ΄μΌλ‘κ·Έμ ν¬μ»€μ€ κ΄λ¦¬ κ°μ νμ
λͺ¨λ¬μ΄ μ΄λ¦΄ λ μλμΌλ‘ ν¬μ»€μ€λ₯Ό λ°κ³ , λ«ν λ μ΄μ ν¬μ»€μ€λ‘ λμκ°λ κΈ°λ₯μ΄ νμν©λλ€.
useEffectλ₯Ό μ¬μ©νμ¬ ν¬μ»€μ€ κ΄λ¦¬λ₯Ό ꡬννμΈμ:
+import { useEffect, useRef } from 'react';
const Modal: React.FC<AlertModalProps> = ({...}) => {
+ const dialogRef = useRef<HTMLDivElement>(null);
+ const previousFocusRef = useRef<HTMLElement | null>(null);
+
+ useEffect(() => {
+ if (isOpen) {
+ previousFocusRef.current = document.activeElement as HTMLElement;
+ dialogRef.current?.focus();
+ } else if (previousFocusRef.current) {
+ previousFocusRef.current.focus();
+ }
+ }, [isOpen]);
return (
<div role="presentation">
<div
+ ref={dialogRef}
className={`rounded-lg bg-BG_2 shadow-xl ${modalClassName}`}
role="dialog"
aria-modal="true"
- tabIndex={-1}
+ tabIndex={0}
>π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| className={`rounded-lg bg-BG_2 shadow-xl ${modalClassName}`} | |
| role="dialog" | |
| aria-modal="true" | |
| tabIndex={-1} | |
| > | |
| import React, { useEffect, useRef } from 'react'; | |
| interface AlertModalProps { | |
| isOpen: boolean; | |
| modalClassName?: string; | |
| children?: React.ReactNode; | |
| // ...other props | |
| } | |
| const Modal: React.FC<AlertModalProps> = ({ isOpen, modalClassName = '', children, ...props }) => { | |
| const dialogRef = useRef<HTMLDivElement>(null); | |
| const previousFocusRef = useRef<HTMLElement | null>(null); | |
| useEffect(() => { | |
| if (isOpen) { | |
| previousFocusRef.current = document.activeElement as HTMLElement; | |
| dialogRef.current?.focus(); | |
| } else if (previousFocusRef.current) { | |
| previousFocusRef.current.focus(); | |
| } | |
| }, [isOpen]); | |
| return ( | |
| <div role="presentation"> | |
| <div | |
| ref={dialogRef} | |
| className={`rounded-lg bg-BG_2 shadow-xl ${modalClassName}`} | |
| role="dialog" | |
| aria-modal="true" | |
| tabIndex={0} | |
| {...props} | |
| > | |
| {children} | |
| </div> | |
| </div> | |
| ); | |
| }; | |
| export default Modal; |
| const Some = () => { | ||
| const [isSecondModalOpen, setIsSecondModalOpen] = useState(false); | ||
| const [selectedUser, setSelectedUser] = useState<UserData | null>(null); | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
μ»΄ν¬λνΈ μ΄λ¦ λ° μν κ΄λ¦¬ κ°μ νμ
μ»΄ν¬λνΈ μ΄λ¦ 'Some'μ΄ μλ―Έλ₯Ό λͺ νν μ λ¬νμ§ λͺ»νλ©°, μν λ³μλͺ λ μ§κ΄μ μ΄μ§ μμ΅λλ€.
λ€μκ³Ό κ°μ΄ μμ νμΈμ:
-const Some = () => {
- const [isSecondModalOpen, setIsSecondModalOpen] = useState(false);
- const [selectedUser, setSelectedUser] = useState<UserData | null>(null);
+const UserProfileList = () => {
+ const [isProfileModalOpen, setIsProfileModalOpen] = useState(false);
+ const [selectedUser, setSelectedUser] = useState<UserData | null>(null);π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const Some = () => { | |
| const [isSecondModalOpen, setIsSecondModalOpen] = useState(false); | |
| const [selectedUser, setSelectedUser] = useState<UserData | null>(null); | |
| const UserProfileList = () => { | |
| const [isProfileModalOpen, setIsProfileModalOpen] = useState(false); | |
| const [selectedUser, setSelectedUser] = useState<UserData | null>(null); |
| <div className="flex flex-col space-y-4"> | ||
| <div className="grid grid-cols-4 gap-4 p-2 font-semibold"> | ||
| <div className="text-white">μ΄λ¦</div> | ||
| <div className="text-white">μν</div> | ||
| <div className="text-white">νλ‘ν</div> | ||
| </div> | ||
| <div className="w-full border-t border-white"></div> | ||
| {mockUsers.map((user) => ( | ||
| <div | ||
| key={user.id} | ||
| className="grid grid-cols-4 items-center gap-4 border-b pb-2" | ||
| > | ||
| <div className="text-white">{user.name}</div> | ||
| <div className={getStatusColor(user.status)}>{user.status}</div> | ||
| <div> | ||
| <Button | ||
| onClick={() => handleProfileClick(user)} | ||
| variant="outline" | ||
| size="sm" | ||
| > | ||
| νλ‘ν 보기 | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
π οΈ Refactor suggestion
그리λ λ μ΄μμ λ° μ κ·Όμ± κ°μ νμ
ν
μ΄λΈ ννμ λ°μ΄ν°λ₯Ό divλ‘ κ΅¬ννλ©΄ μ€ν¬λ¦° 리λ μ¬μ©μκ° λ°μ΄ν° κ°μ κ΄κ³λ₯Ό μ΄ν΄νκΈ° μ΄λ ΅μ΅λλ€.
μλ§¨ν± ν μ΄λΈ λ§ν¬μ μ μ¬μ©νλλ‘ μμ νμΈμ:
-<div className="flex flex-col space-y-4">
- <div className="grid grid-cols-4 gap-4 p-2 font-semibold">
- <div className="text-white">μ΄λ¦</div>
- <div className="text-white">μν</div>
- <div className="text-white">νλ‘ν</div>
- </div>
+<div className="flex flex-col space-y-4">
+ <table className="w-full">
+ <thead>
+ <tr className="grid grid-cols-4 gap-4 p-2 font-semibold">
+ <th scope="col" className="text-white text-left">μ΄λ¦</th>
+ <th scope="col" className="text-white text-left">μν</th>
+ <th scope="col" className="text-white text-left">νλ‘ν</th>
+ </tr>
+ </thead>
+ <tbody>Committable suggestion skipped: line range outside the PR's diff.
dbswl701
left a comment
dbswl701
left a comment
There was a problem hiding this comment.
footerλ‘ λ²νΌμ΄ νλλ§ μλ κ²½μ°(yes)λ μκ³ λ²νΌμ΄ 2κ°μΈ κ²½μ°(yes, no)λ μκ³ λ²νΌμ΄ μ‘΄μ¬νμ§ μλ κ²½μ°(λ§΄λ² λ¦¬μ€νΈ)λ μμ΄μ λͺ¨λ κ²½μ°κ° λ€ κ³ λ €κ° λμ΄μΌ ν κ² κ°μ΅λλ€.
CI Status Reportκ²μ¬ κ²°κ³Ό
β λͺ¨λ κ²μ¬κ° ν΅κ³Όλμμ΅λλ€. |
chore(DEVING-38): λͺ¨λ¬ κ²½λ‘ μμ λ° backdrop roleμμ ν΄λΉ 컀λ°μ λ°μνμ΅λλ€! |
There was a problem hiding this comment.
Actionable comments posted: 7
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (3)
src/app/preview/modal/Some.tsx(1 hunks)src/app/preview/modal/page.tsx(1 hunks)src/components/ui/modal/Modal.tsx(1 hunks)
π§° Additional context used
π§ Learnings (1)
src/app/preview/modal/page.tsx (3)
Learnt from: clyde-yoonjae
PR: MoimService/Moim-FE#14
File: src/app/preview/modal/page.tsx:17-40
Timestamp: 2025-02-19T02:34:10.468Z
Learning: ARIA μμ±μ μΉ μ κ·Όμ±μ ν₯μμν€λ μ€μν λꡬμ
λλ€:
- aria-label: μμμ λν μ€λͺ
μ μΈ λ μ΄λΈμ μ 곡
- aria-controls: νμ¬ μμκ° μ μ΄νλ λ€λ₯Έ μμμμ κ΄κ³λ₯Ό λͺ
μ
λͺ¨λ¬ μ»΄ν¬λνΈμλ νμ κ³ μ ν IDλ₯Ό λΆμ¬νκ³ , ν΄λΉ λͺ¨λ¬μ μ μ΄νλ λ²νΌμλ aria-controls μμ±μΌλ‘ λͺ¨λ¬ IDλ₯Ό μ°κ²°ν΄μΌ ν©λλ€.
Learnt from: clyde-yoonjae
PR: MoimService/Moim-FE#14
File: src/app/preview/modal/page.tsx:17-40
Timestamp: 2025-02-19T02:34:10.468Z
Learning: ARIA μμ±μ μΈ κ°μ§ μ£Όμ μΉ΄ν
κ³ λ¦¬λ‘ κ΅¬λΆλ©λλ€:
1. Roles: μμμ λͺ©μ μ μ (μ: role="dialog")
2. Properties: μμμ νΉμ± μ€λͺ
(μ: aria-label, aria-controls)
3. States: λμ μν νμ (μ: aria-expanded, aria-hidden)
λͺ¨λ¬ ꡬν μ νμ ARIA μμ±:
- λͺ¨λ¬ νΈλ¦¬κ±° λ²νΌ: aria-label, aria-controls
- λͺ¨λ¬ 컨ν
μ΄λ: role="dialog", aria-modal="true", aria-labelledby
λͺ¨λ¬κ³Ό κ΄λ ¨ μμλ€μ λ°λμ κ³ μ ν IDλ₯Ό κ°μ ΈμΌ νλ©°, μ΄λ ARIA κ΄κ³ μμ±λ€μ΄ μ¬λ°λ₯΄κ² μλνκΈ° μν΄ νμν©λλ€.
Learnt from: clyde-yoonjae
PR: MoimService/Moim-FE#14
File: src/components/ui/modal/alert/Modal.tsx:75-82
Timestamp: 2025-02-19T04:05:58.731Z
Learning: λͺ¨λ¬ ꡬνμ μ κ·Όμ±μ μν ARIA μμ± κ°μ΄λ:
1. Backdrop divλ `aria-hidden="true"`λ₯Ό μ¬μ©νκ³ `tabIndex`λ `role` μμ±μ μ¬μ©νμ§ μμ
2. λͺ¨λ¬ 컨ν
μ΄λλ `role="dialog"`, `aria-modal="true"`, `aria-labelledby` μμ±μ ν¬ν¨ν΄μΌ ν¨
3. μ€ν¬λ¦° 리λλ₯Ό μν μ λͺ©μ μ 곡νκ³ `aria-labelledby`λ‘ μ°κ²°ν΄μΌ ν¨
π Additional comments (3)
src/app/preview/modal/page.tsx (1)
19-21: μ κ·Όμ± μμ± μΆκ° νμλͺ¨λ¬μ μ¬λ λ²νΌλ€μ λν μ κ·Όμ± μμ±μ΄ λΆμ‘±ν©λλ€.
src/app/preview/modal/Some.tsx (2)
82-85: μ»΄ν¬λνΈ μ΄λ¦ λ° μν κ΄λ¦¬ κ°μ νμμ»΄ν¬λνΈ μ΄λ¦ 'Some'μ΄ μλ―Έλ₯Ό λͺ νν μ λ¬νμ§ λͺ»νλ©°, μν λ³μλͺ λ μ§κ΄μ μ΄μ§ μμ΅λλ€.
111-135: 그리λ λ μ΄μμ λ° μ κ·Όμ± κ°μ νμν μ΄λΈ ννμ λ°μ΄ν°λ₯Ό
divλ‘ κ΅¬ννλ©΄ μ€ν¬λ¦° 리λ μ¬μ©μκ° λ°μ΄ν° κ°μ κ΄κ³λ₯Ό μ΄ν΄νκΈ° μ΄λ ΅μ΅λλ€.
| <Modal | ||
| isOpen={isModalOpen3} | ||
| onClose={() => setIsModalOpen3(false)} | ||
| closeOnly | ||
| cancelText="λ«κΈ°" | ||
| modalClassName="w-96" | ||
| > | ||
| <p className="text-white">λ«κΈ° λ²νΌλ§ μλ λͺ¨λ¬μ λλ€!</p> | ||
| </Modal> |
There was a problem hiding this comment.
π οΈ Refactor suggestion
μΈ λ²μ§Έ λͺ¨λ¬μ μ κ·Όμ± κ°μ νμ
λ«κΈ° μ μ© λͺ¨λ¬μλ μ λͺ©κ³Ό μ€λͺ μ λͺ μμ μΌλ‘ μ°κ²°νλ ARIA μμ±μ΄ νμν©λλ€.
λ€μκ³Ό κ°μ΄ μμ νμΈμ:
<Modal
isOpen={isModalOpen3}
onClose={() => setIsModalOpen3(false)}
closeOnly
cancelText="λ«κΈ°"
modalClassName="w-96"
+ aria-labelledby="third-modal-title"
+ aria-describedby="third-modal-content"
>
+ <h2 id="third-modal-title" className="sr-only">λ«κΈ° μ μ© λͺ¨λ¬</h2>
- <p className="text-white">λ«κΈ° λ²νΌλ§ μλ λͺ¨λ¬μ
λλ€!</p>
+ <p id="third-modal-content" className="text-white">λ«κΈ° λ²νΌλ§ μλ λͺ¨λ¬μ
λλ€!</p>
</Modal>π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Modal | |
| isOpen={isModalOpen3} | |
| onClose={() => setIsModalOpen3(false)} | |
| closeOnly | |
| cancelText="λ«κΈ°" | |
| modalClassName="w-96" | |
| > | |
| <p className="text-white">λ«κΈ° λ²νΌλ§ μλ λͺ¨λ¬μ λλ€!</p> | |
| </Modal> | |
| <Modal | |
| isOpen={isModalOpen3} | |
| onClose={() => setIsModalOpen3(false)} | |
| closeOnly | |
| cancelText="λ«κΈ°" | |
| modalClassName="w-96" | |
| aria-labelledby="third-modal-title" | |
| aria-describedby="third-modal-content" | |
| > | |
| <h2 id="third-modal-title" className="sr-only">λ«κΈ° μ μ© λͺ¨λ¬</h2> | |
| <p id="third-modal-content" className="text-white">λ«κΈ° λ²νΌλ§ μλ λͺ¨λ¬μ λλ€!</p> | |
| </Modal> |
| <Modal | ||
| isOpen={isModalOpen} | ||
| onClose={() => setIsModalOpen(false)} | ||
| onConfirm={handleConfirm} | ||
| confirmText="μμ " | ||
| cancelText="μ·¨μ" | ||
| modalClassName="w-96" | ||
| > | ||
| <Some /> | ||
| </Modal> |
There was a problem hiding this comment.
π οΈ Refactor suggestion
λͺ¨λ¬μ μ κ·Όμ± κ°μ νμ
λͺ¨λ¬μ μ λͺ©μ λͺ
μμ μΌλ‘ μ°κ²°νλ aria-labelledby μμ±μ΄ λλ½λμμ΅λλ€.
λ€μκ³Ό κ°μ΄ μμ νμΈμ:
<Modal
isOpen={isModalOpen}
onClose={() => setIsModalOpen(false)}
onConfirm={handleConfirm}
confirmText="μμ "
cancelText="μ·¨μ"
modalClassName="w-96"
+ aria-labelledby="first-modal-title"
>
+ <h2 id="first-modal-title" className="sr-only">μ΄μ€ λͺ¨λ¬</h2>
<Some />
</Modal>π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Modal | |
| isOpen={isModalOpen} | |
| onClose={() => setIsModalOpen(false)} | |
| onConfirm={handleConfirm} | |
| confirmText="μμ " | |
| cancelText="μ·¨μ" | |
| modalClassName="w-96" | |
| > | |
| <Some /> | |
| </Modal> | |
| <Modal | |
| isOpen={isModalOpen} | |
| onClose={() => setIsModalOpen(false)} | |
| onConfirm={handleConfirm} | |
| confirmText="μμ " | |
| cancelText="μ·¨μ" | |
| modalClassName="w-96" | |
| aria-labelledby="first-modal-title" | |
| > | |
| <h2 id="first-modal-title" className="sr-only">μ΄μ€ λͺ¨λ¬</h2> | |
| <Some /> | |
| </Modal> |
| <Modal | ||
| isOpen={isModalOpen2} | ||
| onClose={() => setIsModalOpen2(false)} | ||
| onConfirm={handleConfirm} | ||
| confirmText="μμ " | ||
| cancelText="μ·¨μ" | ||
| modalClassName="w-96" | ||
| > | ||
| <p className="text-white">μλ νμΈμ©μ©</p> | ||
| </Modal> |
There was a problem hiding this comment.
π οΈ Refactor suggestion
λ λ²μ§Έ λͺ¨λ¬μ μ κ·Όμ± κ°μ νμ
λͺ¨λ¬μ μ λͺ©κ³Ό μ€λͺ μ λͺ μμ μΌλ‘ μ°κ²°νλ ARIA μμ±μ΄ νμν©λλ€.
λ€μκ³Ό κ°μ΄ μμ νμΈμ:
<Modal
isOpen={isModalOpen2}
onClose={() => setIsModalOpen2(false)}
onConfirm={handleConfirm}
confirmText="μμ "
cancelText="μ·¨μ"
modalClassName="w-96"
+ aria-labelledby="second-modal-title"
+ aria-describedby="second-modal-content"
>
+ <h2 id="second-modal-title" className="sr-only">κΈ°λ³Έ λͺ¨λ¬</h2>
- <p className="text-white">μλ
νμΈμ©μ©</p>
+ <p id="second-modal-content" className="text-white">μλ
νμΈμ©μ©</p>
</Modal>π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Modal | |
| isOpen={isModalOpen2} | |
| onClose={() => setIsModalOpen2(false)} | |
| onConfirm={handleConfirm} | |
| confirmText="μμ " | |
| cancelText="μ·¨μ" | |
| modalClassName="w-96" | |
| > | |
| <p className="text-white">μλ νμΈμ©μ©</p> | |
| </Modal> | |
| <Modal | |
| isOpen={isModalOpen2} | |
| onClose={() => setIsModalOpen2(false)} | |
| onConfirm={handleConfirm} | |
| confirmText="μμ " | |
| cancelText="μ·¨μ" | |
| modalClassName="w-96" | |
| aria-labelledby="second-modal-title" | |
| aria-describedby="second-modal-content" | |
| > | |
| <h2 id="second-modal-title" className="sr-only">κΈ°λ³Έ λͺ¨λ¬</h2> | |
| <p id="second-modal-content" className="text-white">μλ νμΈμ©μ©</p> | |
| </Modal> |
| interface AlertModalProps { | ||
| isOpen: boolean; | ||
| onClose: () => void; | ||
| onConfirm?: () => void; | ||
| confirmText?: string; | ||
| cancelText?: string; | ||
| children: React.ReactNode; | ||
| modalClassName?: string; | ||
| contentClassName?: string; | ||
| buttonClassName?: string; | ||
| closeOnly?: boolean; | ||
| } |
There was a problem hiding this comment.
π οΈ Refactor suggestion
μ κ·Όμ± κ΄λ ¨ νλ‘νΌν° μΆκ° νμ
λͺ¨λ¬μ μ κ·Όμ±μ κ°μ νκΈ° μν΄ νμμ μΈ ARIA μμ±λ€μ νλ‘νΌν°λ‘ μΆκ°ν΄μΌ ν©λλ€.
λ€μκ³Ό κ°μ΄ μΈν°νμ΄μ€λ₯Ό νμ₯νμΈμ:
interface AlertModalProps {
isOpen: boolean;
onClose: () => void;
onConfirm?: () => void;
confirmText?: string;
cancelText?: string;
children: React.ReactNode;
modalClassName?: string;
contentClassName?: string;
buttonClassName?: string;
closeOnly?: boolean;
+ 'aria-labelledby'?: string;
+ 'aria-describedby'?: string;
}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface AlertModalProps { | |
| isOpen: boolean; | |
| onClose: () => void; | |
| onConfirm?: () => void; | |
| confirmText?: string; | |
| cancelText?: string; | |
| children: React.ReactNode; | |
| modalClassName?: string; | |
| contentClassName?: string; | |
| buttonClassName?: string; | |
| closeOnly?: boolean; | |
| } | |
| interface AlertModalProps { | |
| isOpen: boolean; | |
| onClose: () => void; | |
| onConfirm?: () => void; | |
| confirmText?: string; | |
| cancelText?: string; | |
| children: React.ReactNode; | |
| modalClassName?: string; | |
| contentClassName?: string; | |
| buttonClassName?: string; | |
| closeOnly?: boolean; | |
| 'aria-labelledby'?: string; | |
| 'aria-describedby'?: string; | |
| } |
| return ( | ||
| <div | ||
| className="fixed inset-0 z-50 flex items-center justify-center bg-BG_2 bg-opacity-50" | ||
| onClick={handleBackdropClick} | ||
| onKeyDown={handleKeyDown} | ||
| role="presentation" | ||
| aria-label="Close modal" | ||
| > | ||
| <div | ||
| className={`rounded-lg bg-BG_2 shadow-xl ${modalClassName}`} | ||
| role="dialog" | ||
| aria-modal="true" | ||
| tabIndex={-1} | ||
| > | ||
| <div className={`p-6 ${contentClassName}`}>{children}</div> | ||
|
|
||
| <div className={`flex justify-end gap-2 p-4 ${buttonClassName}`}> | ||
| {closeOnly ? ( | ||
| <Button onClick={onClose} type="button"> | ||
| {cancelText} | ||
| </Button> | ||
| ) : ( | ||
| <> | ||
| <Button onClick={onClose} variant={'outline'} type="button"> | ||
| {cancelText} | ||
| </Button> | ||
| <Button onClick={onConfirm} type="button"> | ||
| {confirmText} | ||
| </Button> | ||
| </> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
π οΈ Refactor suggestion
μλ§¨ν± κ΅¬μ‘°μ μ€ν¬λ¦° 리λ μ§μ κ°μ νμ
λͺ¨λ¬μ μλ§¨ν± κ΅¬μ‘°μ μ€ν¬λ¦° 리λ μ¬μ©μλ₯Ό μν μ§μμ΄ λ―Έν‘ν©λλ€.
λ€μκ³Ό κ°μ΄ κ°μ νμΈμ:
return (
<div
className="fixed inset-0 z-50 flex items-center justify-center bg-BG_2 bg-opacity-50"
onClick={handleBackdropClick}
onKeyDown={handleKeyDown}
- role="presentation"
- aria-label="Close modal"
+ aria-hidden="true"
>
<div
+ ref={modalRef}
className={`rounded-lg bg-BG_2 shadow-xl ${modalClassName}`}
role="dialog"
aria-modal="true"
+ aria-labelledby={props['aria-labelledby']}
+ aria-describedby={props['aria-describedby']}
tabIndex={-1}
>
<div className={`p-6 ${contentClassName}`}>{children}</div>
<div className={`flex justify-end gap-2 p-4 ${buttonClassName}`}>
{closeOnly ? (
<Button onClick={onClose} type="button">
{cancelText}
</Button>
) : (
<>
<Button onClick={onClose} variant={'outline'} type="button">
{cancelText}
</Button>
<Button onClick={onConfirm} type="button">
{confirmText}
</Button>
</>
)}
</div>
</div>
</div>
);| const handleBackdropClick = (e: React.MouseEvent<HTMLDivElement>): void => { | ||
| if (e.target === e.currentTarget) { | ||
| onClose(); | ||
| } | ||
| }; | ||
|
|
||
| const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>): void => { | ||
| if (e.key === 'Escape') { | ||
| onClose(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
π οΈ Refactor suggestion
ν€λ³΄λ μ κ·Όμ± κ°μ νμ
λͺ¨λ¬μ΄ μ΄λ Έμ λ ν¬μ»€μ€ κ΄λ¦¬μ ν€λ³΄λ νμμ΄ μ λλ‘ κ΅¬νλμ΄ μμ§ μμ΅λλ€.
λ€μ μ¬νλ€μ κ°μ νμΈμ:
- λͺ¨λ¬μ΄ μ΄λ¦΄ λ 첫 λ²μ§Έ ν¬μ»€μ€ κ°λ₯ν μμλ‘ ν¬μ»€μ€ μ΄λ
- Tab ν€λ‘ λͺ¨λ¬ λ΄λΆ μμλ€λ§ μννλλ‘ ν¬μ»€μ€ νΈλ© ꡬν
- λͺ¨λ¬μ΄ λ«ν λ μ΄μ ν¬μ»€μ€ μμΉλ‘ 볡μ
+import { useEffect, useRef } from 'react';
+
const Modal: React.FC<AlertModalProps> = ({
// ... props
}) => {
+ const modalRef = useRef<HTMLDivElement>(null);
+ const previousFocusRef = useRef<HTMLElement | null>(null);
+
+ useEffect(() => {
+ if (isOpen) {
+ previousFocusRef.current = document.activeElement as HTMLElement;
+ const focusableElements = modalRef.current?.querySelectorAll(
+ 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
+ );
+ if (focusableElements?.[0]) {
+ (focusableElements[0] as HTMLElement).focus();
+ }
+ } else if (previousFocusRef.current) {
+ previousFocusRef.current.focus();
+ }
+ }, [isOpen]);
+
+ const handleTabKey = (e: React.KeyboardEvent) => {
+ if (e.key !== 'Tab') return;
+
+ const focusableElements = modalRef.current?.querySelectorAll(
+ 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
+ );
+ if (!focusableElements) return;
+
+ const firstElement = focusableElements[0] as HTMLElement;
+ const lastElement = focusableElements[focusableElements.length - 1] as HTMLElement;
+
+ if (e.shiftKey && document.activeElement === firstElement) {
+ e.preventDefault();
+ lastElement.focus();
+ } else if (!e.shiftKey && document.activeElement === lastElement) {
+ e.preventDefault();
+ firstElement.focus();
+ }
+ };
+
const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>): void => {
if (e.key === 'Escape') {
onClose();
}
+ handleTabKey(e);
};π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleBackdropClick = (e: React.MouseEvent<HTMLDivElement>): void => { | |
| if (e.target === e.currentTarget) { | |
| onClose(); | |
| } | |
| }; | |
| const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>): void => { | |
| if (e.key === 'Escape') { | |
| onClose(); | |
| } | |
| }; | |
| import { useEffect, useRef } from 'react'; | |
| const Modal: React.FC<AlertModalProps> = ({ | |
| // ... props (including isOpen, onClose, etc.) | |
| }) => { | |
| const modalRef = useRef<HTMLDivElement>(null); | |
| const previousFocusRef = useRef<HTMLElement | null>(null); | |
| useEffect(() => { | |
| if (isOpen) { | |
| previousFocusRef.current = document.activeElement as HTMLElement; | |
| const focusableElements = modalRef.current?.querySelectorAll( | |
| 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])' | |
| ); | |
| if (focusableElements?.[0]) { | |
| (focusableElements[0] as HTMLElement).focus(); | |
| } | |
| } else if (previousFocusRef.current) { | |
| previousFocusRef.current.focus(); | |
| } | |
| }, [isOpen]); | |
| const handleTabKey = (e: React.KeyboardEvent) => { | |
| if (e.key !== 'Tab') return; | |
| const focusableElements = modalRef.current?.querySelectorAll( | |
| 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])' | |
| ); | |
| if (!focusableElements) return; | |
| const firstElement = focusableElements[0] as HTMLElement; | |
| const lastElement = focusableElements[focusableElements.length - 1] as HTMLElement; | |
| if (e.shiftKey && document.activeElement === firstElement) { | |
| e.preventDefault(); | |
| lastElement.focus(); | |
| } else if (!e.shiftKey && document.activeElement === lastElement) { | |
| e.preventDefault(); | |
| firstElement.focus(); | |
| } | |
| }; | |
| const handleBackdropClick = (e: React.MouseEvent<HTMLDivElement>): void => { | |
| if (e.target === e.currentTarget) { | |
| onClose(); | |
| } | |
| }; | |
| const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>): void => { | |
| if (e.key === 'Escape') { | |
| onClose(); | |
| } | |
| handleTabKey(e); | |
| }; | |
| return ( | |
| <div | |
| ref={modalRef} | |
| onKeyDown={handleKeyDown} | |
| // ... other props and JSX for the modal | |
| > | |
| {/* Modal content */} | |
| </div> | |
| ); | |
| }; |
| <Modal | ||
| isOpen={isSecondModalOpen} | ||
| onClose={() => setIsSecondModalOpen(false)} | ||
| onConfirm={handleSecondModalConfirm} | ||
| confirmText="νμΈ" | ||
| cancelText="λ«κΈ°" | ||
| modalClassName="w-96" | ||
| > | ||
| {selectedUser && ( | ||
| <div className="p-4"> | ||
| <h2 className="mb-4 text-xl font-bold text-white"> | ||
| {selectedUser.name}λμ νλ‘ν | ||
| </h2> | ||
| <p className="text-gray-600">{selectedUser.introduction}</p> | ||
| </div> | ||
| )} | ||
| </Modal> |
There was a problem hiding this comment.
π οΈ Refactor suggestion
λͺ¨λ¬ λ΄μ©μ μ κ·Όμ± κ°μ νμ
λͺ¨λ¬ λ΄μ©μ ꡬ쑰μ μ κ·Όμ± μμ±μ΄ λ―Έν‘ν©λλ€.
λ€μκ³Ό κ°μ΄ κ°μ νμΈμ:
<Modal
isOpen={isSecondModalOpen}
onClose={() => setIsSecondModalOpen(false)}
onConfirm={handleSecondModalConfirm}
confirmText="νμΈ"
cancelText="λ«κΈ°"
modalClassName="w-96"
+ aria-labelledby="profile-modal-title"
+ aria-describedby="profile-modal-content"
>
{selectedUser && (
<div className="p-4">
<h2
+ id="profile-modal-title"
className="mb-4 text-xl font-bold text-white"
>
{selectedUser.name}λμ νλ‘ν
</h2>
- <p className="text-gray-600">{selectedUser.introduction}</p>
+ <p
+ id="profile-modal-content"
+ className="text-gray-600"
+ >
+ {selectedUser.introduction}
+ </p>
</div>
)}
</Modal>π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Modal | |
| isOpen={isSecondModalOpen} | |
| onClose={() => setIsSecondModalOpen(false)} | |
| onConfirm={handleSecondModalConfirm} | |
| confirmText="νμΈ" | |
| cancelText="λ«κΈ°" | |
| modalClassName="w-96" | |
| > | |
| {selectedUser && ( | |
| <div className="p-4"> | |
| <h2 className="mb-4 text-xl font-bold text-white"> | |
| {selectedUser.name}λμ νλ‘ν | |
| </h2> | |
| <p className="text-gray-600">{selectedUser.introduction}</p> | |
| </div> | |
| )} | |
| </Modal> | |
| <Modal | |
| isOpen={isSecondModalOpen} | |
| onClose={() => setIsSecondModalOpen(false)} | |
| onConfirm={handleSecondModalConfirm} | |
| confirmText="νμΈ" | |
| cancelText="λ«κΈ°" | |
| modalClassName="w-96" | |
| aria-labelledby="profile-modal-title" | |
| aria-describedby="profile-modal-content" | |
| > | |
| {selectedUser && ( | |
| <div className="p-4"> | |
| <h2 | |
| id="profile-modal-title" | |
| className="mb-4 text-xl font-bold text-white" | |
| > | |
| {selectedUser.name}λμ νλ‘ν | |
| </h2> | |
| <p | |
| id="profile-modal-content" | |
| className="text-gray-600" | |
| > | |
| {selectedUser.introduction} | |
| </p> | |
| </div> | |
| )} | |
| </Modal> |
πΒ μ£Όμ μμ λ΄μ©
νλ‘μ νΈ μ μμμ μ¬μ©ν λͺ¨λ¬μ»΄ν¬λνΈ μ§μ ꡬννμμ΅λλ€.
/prview/modal
ν΄λΉκ²½λ‘μ μΌλ° μμμ mock λ°μ΄ν° λ£μ΄λμ 2μ€λͺ¨λ¬ μ¬λ €λμμ΅λλ€.
πΊΒ μ€ν¬λ¦°μ·
π μ°Έκ³ μ¬ν
tailwind-configμ BG_2μμμ # μ΄ λΉ μ Έμμ΄ μμ νμμ΅λλ€.
divνκ·Έμ μμ±κ΄λ ¨νμ¬ λ°°μμ΄ μμμ΅λλ€.
divμμμ μ΄λ²€νΈ 리μ€λ μ§μ μΆκ° μ λ°μνλ λ¬Έμ μ 1. μ κ·Όμ± λ¬Έμ
2. μλ―Έλ‘ μ (Semantic) λ¬Έμ
divλ 컨ν μΈ λ₯Ό ꡬλΆνλ μ©λλ‘ μ€κ³λμμ§, μ¬μ©μ μνΈμμ©μ μ²λ¦¬νλλ‘ μ€κ³λμ§ μμ3. UX(μ¬μ©μ κ²½ν) λ¬Έμ
ν΄κ²° λ°©μ
μνΈμμ©μ΄ νμν κ²½μ°μλ λ€μ λ κ°μ§ λ°©λ² μ€ νλλ₯Ό μ νν΄μΌ ν©λλ€:
buttonκ³Ό κ°μ μνΈμμ©μ© μμλ₯Ό μ¬μ©roleκ³ΌtabIndexλ₯Ό μΆκ°νμ¬ ν΄λΉ μμμ μνΈμμ© λͺ©μ μ λͺ νν μ§μ π¬Β 리뷰 μꡬμ¬ν
ex) μ€μ μ μΌλ‘ 리뷰ν΄μ€¬μΌλ©΄ νλ λΆλΆ
πΒ κ΄λ ¨ μ΄μ
DEVING-38
Summary by CodeRabbit
μλ‘μ΄ κΈ°λ₯
μ€νμΌ