-
Notifications
You must be signed in to change notification settings - Fork 1
이번 PR은 Lighthouse 및 스크린 리더 검사에서 지적된 다수의 접근성 이슈를 해결합니다 #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes update semantic HTML tags across multiple components to improve document structure, including converting headings and list containers to more appropriate tags. Some import paths are refactored for consistency, and minor styling and accessibility adjustments are made in the footer. No component signatures or exports are altered. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/features/home/Knowledge/KnowledgeSectionList.jsx (1)
28-28: Minor key usage nitpick
isModalOpen && <SubscriptionModal … />is fine, but the surrounding list still usesindexas React key (Line 18). Switching to a stable id (e.g.,item.id) avoids unnecessary re-renders when the data order changes.src/features/home/HeroSection.jsx (1)
18-23: Large heading on mobile—watch for overflow
h1font-size is 9 rem (desktop) / 4 rem (tablet & mobile). Even 4 rem can overflow narrow devices in some languages. Considerclamp()for responsive sizing.src/features/home/Knowledge/KnowledgeSectionItem.jsx (1)
27-31: Consider whether Summary should be a heading element.While the heading level adjustment follows the pattern, consider if the Summary should be a heading at all. It appears to be descriptive content rather than a section header. A
<p>element with appropriate styling might be more semantically correct.-const Summary = styled.h5` +const Summary = styled.p` font-size: 1.1rem; padding: 0 0 1.5rem 0; color: #333; + font-weight: 600; `;src/layout/Footer.jsx (1)
5-5: Color consistency but loss of theming flexibility.The changes from CSS variables (
var(--l-grey)) to fixedblackvalues create visual consistency but reduce theming flexibility. Consider if this tradeoff aligns with your design system goals.If you plan to support themes in the future, consider keeping CSS variables:
- background-color: #f8f8f8; + background-color: var(--footer-bg, #f8f8f8); - color: black; + color: var(--footer-text, black); - <FaGithub size={20} color="black" /> + <FaGithub size={20} color="var(--footer-text, black)" />Also applies to: 18-18, 40-40
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/features/content/ContentList.jsx(2 hunks)src/features/home/AboutSection.jsx(2 hunks)src/features/home/HeroSection.jsx(3 hunks)src/features/home/Knowledge/KnowledgeSectionItem.jsx(2 hunks)src/features/home/Knowledge/KnowledgeSectionList.jsx(2 hunks)src/features/home/KnowledgeSection.jsx(1 hunks)src/features/home/PopularTopicSection.jsx(1 hunks)src/layout/Footer.jsx(3 hunks)
🔇 Additional comments (12)
src/features/home/PopularTopicSection.jsx (1)
13-14: Confirm heading-level consistency within the page
SectionTitleis now an<h3>, which is semantically fine as long as an<h2>exists earlier in the DOM order on the same page. Please double-check the overall document outline (e.g., via Lighthouse or the browser’s accessibility tree) to ensure heading levels don’t skip.src/features/home/Knowledge/KnowledgeSectionList.jsx (1)
2-5: Ensure alias paths resolve in every build/test targetThe imports were switched to
@mocks/knowledgeDataand@hooks/useModal.
Verify that:
- Babel/webpack/ts-config
paths(oralias) are updated accordingly.- Jest (if used) has the same module-name-mapper so unit tests don’t break.
Without those mappings, CI/CD or Storybook will fail to compile.
src/features/home/KnowledgeSection.jsx (1)
12-17: Validate heading hierarchy change
SectionTitledrops from<h2>to<h3>. Confirm there is an<h2>ancestor (AboutSection now provides one) so headings remain sequential (h1 → h2 → h3).src/features/home/AboutSection.jsx (2)
58-66: CSS selector updated—check for stale styles elsewhereOnly the
h2 { … }block was updated. If any other component still renders this text as<h1>, the styles will no longer apply. Search for usages of the old selector in shared CSS or theme overrides to avoid regressions.
149-153: Heading level lowered—verify unique h1 remainsChanging the visible slogan to an
<h2>improves the overall outline, but make sure the page still contains exactly one top-level<h1>(now inHeroSection).src/features/home/HeroSection.jsx (1)
122-125: Multiple<h1>riskThis component now renders an
<h1>. Verify that no other component on the same page also outputs an<h1>(other than this Hero). Multipleh1s can confuse screen-reader landmarks and SEO.src/features/home/Knowledge/KnowledgeSectionItem.jsx (1)
17-17: LGTM: Heading level adjustment improves document structure.The change from
h3toh4aligns with the coordinated heading hierarchy improvements across the application, enhancing accessibility and document structure.src/features/content/ContentList.jsx (3)
6-12: Excellent semantic improvement!Converting from a generic
divto a properulwith reset padding/margin maintains the visual design while providing correct semantic structure for screen readers and other assistive technologies.
14-16: Proper list item styling.The
ContentListItemcomponent correctly removes default list styling while maintaining semantic structure. This is the right approach for custom-styled lists.
25-34: Correct key placement and structure.Moving the
keyprop to theliwrapper is semantically correct since theliis the actual list item React needs to track. TheContentItemprops are properly preserved.src/layout/Footer.jsx (2)
2-2: Minor: Consistent quote style.Good consistency improvement using single quotes throughout the import.
34-41: Excellent accessibility improvement!Adding the
aria-label="하루한 깃허브"provides a clear, localized description for screen readers, significantly improving accessibility for users who rely on assistive technologies.
E2E Test & Lighthouse Report
|
이로써 페이지 시맨틱 구조가 개선되어 스크린 리더 사용자와 접근성 지표 모두에서 올바르게 동작합니다.
Summary by CodeRabbit
Style
Accessibility