Uh oh!
There was an error while loading. Please reload this page.
🎨 Palette: [UX improvement] 외부 링크에 새 창 열림 툴팁(title) 추가 - #193
Conversation
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough외부 링크에 새 창 열림 안내를 추가했습니다. 한국어와 영어 메시지를 정의했습니다. Changes외부 링크 접근성 안내
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk:⚪ Minimal · up to The PR adds multilingual new-window guidance to external links through localized tooltip text. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Devin Review found 3 potential issues.
⚠️ 1 issue in files not directly in the diff
⚠️ Missing CHANGELOG entry for user-facing change
CLAUDE.md requires CHANGELOG.md to be updated under [Unreleased] in Korean for user-facing changes. This PR adds external-link tooltips, a user-facing change, but adds no CHANGELOG entry.
| if (!isInitialDefault) { | ||
| if (!i18nNodes) { | ||
| i18nNodes = document.querySelectorAll("[data-i18n]"); | ||
| i18nNodes = document.querySelectorAll("[data-i18n], [data-i18n-title]"); | ||
| } | ||
| // Only update textContent if it actually changed to avoid layout recalculations | ||
| i18nNodes.forEach((node) => { | ||
| const newText = dict[node.dataset.i18n]; | ||
| if (newText && node.textContent !== newText) { | ||
| node.textContent = newText; | ||
| if (node.hasAttribute("data-i18n")) { | ||
| const newText = dict[node.dataset.i18n]; | ||
| if (newText && node.textContent !== newText) { | ||
| node.textContent = newText; | ||
| } | ||
| } | ||
| if (node.hasAttribute("data-i18n-title")) { | ||
| const newTitle = dict[node.dataset.i18nTitle]; | ||
| if (newTitle && node.getAttribute("title") !== newTitle) { | ||
| node.setAttribute("title", newTitle); | ||
| } | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
📝 Info: Initial Korean load skips title translation, stays consistent
On initial Korean load isInitialDefault skips the node traversal, so data-i18n-title titles are never set by JS. Safe because every external link carries an inline Korean title matching a11y.newWindow, and later language switches populate i18nNodes and update titles correctly.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| **Learning:** Using an anchor tag to wrap an entire card (block-level element) can result in verbose and confusing screen reader output. However, restricting the clickable area to just the title makes the UI harder to interact with (violating Fitts's Law). | ||
| **Action:** Apply `position: relative` to the card container and use a `::after` pseudo-element with `position: absolute; inset: 0;` on the title's anchor tag. This expands the clickable area to the whole card while keeping semantic and accessible HTML structure. | ||
| ## 2024-08-25 - 외부 링크에 접근성 향상을 위한 title 속성과 i18n 적용 |
There was a problem hiding this comment.
📝 Info: Palette entry date likely off by two years
The new entry is dated 2024-08-25 while the change lands in 2026. Given the journal records recent incidents, this reads as a typo, though other entries also use 2024 dates so intent is ambiguous.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:22927712ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <a href="#forks" data-i18n="nav.forks">Fork</a> | ||
| <a href="#work" data-i18n="nav.work">작업</a> | ||
| <a target="_blank" rel="noopener noreferrer" href="https://github.com/ContextualWisdomLab">GitHub</a> | ||
| <a target="_blank" rel="noopener noreferrer" href="https://github.com/ContextualWisdomLab" title="(새 창에서 열림)" data-i18n-title="a11y.newWindow">GitHub</a> |
There was a problem hiding this comment.
For keyboard-only and touch users, the only new cue is a title tooltip, which is generally exposed through mouse hover and is not reliably announced by assistive technologies when the link already has visible text. Consequently, these users can still activate the link without learning that it opens a new tab, defeating the accessibility goal of this change; provide the localized hint through visible or visually hidden text, or associate it with the link using aria-describedby instead.
Useful? React with 👍 / 👎.
💡 What: 외부 링크(
target="_blank")에 다국어를 지원하는title툴팁을 추가하여 새 창에서 열림을 안내하도록 개선했습니다.🎯 Why: 스크린 리더 및 시각 사용자에게 외부 링크 이동 시 새 탭이 열린다는 맥락을 제공하여 인지적 혼란을 막기 위함입니다.
📸 Before/After: 해당 없음 (툴팁만 노출됨).
♿ Accessibility: 스크린 리더가 링크를 읽을 때 새 창 열림 안내를 병합하여 공지할 수 있도록 다국어(한국어/영어) 지원을 i18n 구조에 확장 적용했습니다.
PR created automatically by Jules for task 11946162662865325152 started by @seonghobae
Summary by CodeRabbit
접근성 개선
문서