better A11y for high contrast mode Frontend #3797
Conversation
Replaces inline styles for required field asterisks in add.twig and ask.twig templates with a reusable .pmf-required-asterisk CSS class. Adds corresponding styles to _global.scss and _theme-switcher.scss for consistent appearance across themes.
Replaced hardcoded color values in the high contrast theme with Bootstrap CSS variables for better maintainability and consistency. Enhanced accessibility and visual appearance of links, logo, headings, and form elements. Removed Atkinson Hyperlegible font files from the assets.
WalkthroughAdds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
phpmyfaq/assets/templates/default/ask.twig (1)
58-58: Remove the debug code.The
{{ dump(categories) }}statement outputs debugging information and should not be present in production code.Apply this diff:
- <select name="category" class="form-select" id="category" {{ id5_required }}> - {{ dump(categories) }} - + <select name="category" class="form-select" id="category" {{ id5_required }}> {% for cat in categories %}
🧹 Nitpick comments (1)
phpmyfaq/assets/scss/layout/_theme-switcher.scss (1)
577-582: Consider scoping the transitions more selectively.The universal selector
*applies transitions to all elements, which may impact performance on complex pages. Consider limiting transitions to specific components or using a class-based approach for theme-aware elements.For example:
// More targeted approach body, .card, .btn, .form-control, .alert, a { transition: background-color 0.3s ease, border-color 0.3s ease, color 0.3s ease; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
phpmyfaq/assets/fonts/AtkinsonHyperlegible-Bold.ttfis excluded by!**/*.ttfphpmyfaq/assets/fonts/AtkinsonHyperlegible-BoldItalic.ttfis excluded by!**/*.ttfphpmyfaq/assets/fonts/AtkinsonHyperlegible-Italic.ttfis excluded by!**/*.ttfphpmyfaq/assets/fonts/AtkinsonHyperlegible-Regular.ttfis excluded by!**/*.ttf
📒 Files selected for processing (4)
phpmyfaq/assets/scss/_global.scss(1 hunks)phpmyfaq/assets/scss/layout/_theme-switcher.scss(7 hunks)phpmyfaq/assets/templates/default/add.twig(6 hunks)phpmyfaq/assets/templates/default/ask.twig(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{html,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use HTML5 and CSS3 standards for frontend code
Files:
phpmyfaq/assets/scss/layout/_theme-switcher.scssphpmyfaq/assets/scss/_global.scss
**/*.{html,scss,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bootstrap components and utilities for layout and styling
Files:
phpmyfaq/assets/scss/layout/_theme-switcher.scssphpmyfaq/assets/scss/_global.scss
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: phpMyFAQ 8.6 Test on ubuntu-latest
- GitHub Check: phpMyFAQ 8.5 Test on ubuntu-latest
- GitHub Check: phpMyFAQ 8.3 Test on ubuntu-latest
- GitHub Check: phpMyFAQ 8.4 Test on ubuntu-latest
🔇 Additional comments (7)
phpmyfaq/assets/scss/_global.scss (1)
72-76: LGTM! Good centralization of required field styling.The new
.pmf-required-asteriskclass properly uses CSS variables and integrates well with the theming system. This approach improves maintainability and supports high-contrast mode.phpmyfaq/assets/templates/default/add.twig (1)
31-31: LGTM! Consistent replacement of inline styles with the new class.All required field indicators now use the centralized
.pmf-required-asteriskclass instead of inline styling. The conditional logic is preserved correctly across all fields.Also applies to: 42-42, 53-53, 71-71, 84-84, 95-95
phpmyfaq/assets/templates/default/ask.twig (1)
35-35: LGTM! Clean implementation of the new required indicator class.The required field indicators for name, email, and question fields correctly use the centralized
.pmf-required-asteriskclass.Also applies to: 44-44, 73-73
phpmyfaq/assets/scss/layout/_theme-switcher.scss (4)
144-147: LGTM! Proper theming for required indicators in dark mode.The required asterisk correctly uses the danger color variable, maintaining consistency with the overall theme.
367-391: Verify logo legibility with the applied filters.The high-contrast logo styling applies
filter: brightness(0) invert(1) saturate(2), which significantly transforms the image. Ensure this maintains brand recognition and doesn't degrade the logo's visual quality.Test the logo appearance in high-contrast mode across different browsers to confirm:
- Brand identity is preserved
- Logo remains recognizable
- Colors maintain sufficient contrast
354-365: Consider the impact of increased link font size.All links in high-contrast mode use
font-size: 1.3remandfont-weight: 800. While this improves visibility, verify that it doesn't cause layout issues or text overflow in constrained spaces like navigation menus or tables.
416-420: LGTM! Excellent accessibility choice for required indicators.Using
var(--bs-primary)(yellow) with bold weight in high-contrast mode provides better visibility than red on dark backgrounds. This is a thoughtful accessibility improvement.
Scaled up the #phpmyfaq-logo for better visibility and applied color inversion to the logo image for high-contrast mode. Simplified logo image styling and removed redundant hover effects.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
phpmyfaq/assets/scss/layout/_theme-switcher.scss (2)
264-289: Fix nesting and indentation in high-contrast theme switcher buttons.The
.buttonselector on line 264 is not properly indented and should be nested within.pmf-theme-switcher. Also, closing brace on line 289 appears misaligned.Apply this diff to correct the nesting:
//Theme Switcher Button for high contrast mode .pmf-theme-switcher { - button { + button { color: var(--bs-light) !important; border: none; background: transparent; transition: all 0.3s ease; opacity: 1; &:hover { color: var(--bs-dark) !important; background: var(--bs-light) !important; opacity: 1; } &.active { color: var(--bs-dark) !important; opacity: 1; background: var(--bs-primary) !important; } i { font-size: 1.3rem; transition: transform 0.3s ease; } } -} + }
323-326: Replace hard-coded hex colors with CSS variables throughout high-contrast mode.The high-contrast section mixes CSS variables (
var(--bs-primary),var(--bs-dark)) with hard-coded hex colors (#000000,#ffffff,#ffff00,#00ff00,#ff0000,#00ffff, etc.). This inconsistency makes the file harder to maintain and update globally.For consistency with dark-mode styling and Bootstrap patterns, use CSS variables throughout. For example:
#000000→var(--bs-dark)or a dedicated--bs-blackvariable#ffffff→var(--bs-light)or a dedicated--bs-whitevariable#ffff00→ Consider a dedicated--bs-warningor theme-specific variable if not already availableThis approach improves maintainability and allows theme colors to be adjusted in one place without hunting through multiple hex values.
Also applies to: 330-332, 342-344, 348-350, 406-406, 416-416, 422-427, 441-459, 471-483, 495-517, 522-524, 528-544
🧹 Nitpick comments (1)
phpmyfaq/assets/scss/layout/_theme-switcher.scss (1)
354-365: Reconsider universal link styling; too broad for high-contrast mode.Lines 356–357 apply
font-size: 1.3remandfont-weight: 800to all<a>elements in high-contrast mode. This is likely too broad and will affect:
- Navigation links
- Button links (
.btn)- Badge links
- Breadcrumb links
- All other semantic links
Some of these elements may already have their own sizing and weight rules, leading to conflicts or unintended visual changes. Consider scoping this rule to specific link contexts (e.g., main content links only, or a dedicated
.pmf-main-linkclass) rather than applying it globally.Verify that the universal link styling does not conflict with existing styles for navigation, buttons, or other link-based components. You may want to run a visual regression test or inspect specific link elements in high-contrast mode to confirm.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
phpmyfaq/assets/scss/layout/_theme-switcher.scss(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{html,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use HTML5 and CSS3 standards for frontend code
Files:
phpmyfaq/assets/scss/layout/_theme-switcher.scss
**/*.{html,scss,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Bootstrap components and utilities for layout and styling
Files:
phpmyfaq/assets/scss/layout/_theme-switcher.scss
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: phpMyFAQ 8.3 Test on ubuntu-latest
- GitHub Check: phpMyFAQ 8.6 Test on ubuntu-latest
- GitHub Check: phpMyFAQ 8.4 Test on ubuntu-latest
- GitHub Check: phpMyFAQ 8.5 Test on ubuntu-latest
🔇 Additional comments (4)
phpmyfaq/assets/scss/layout/_theme-switcher.scss (4)
144-147: Good addition: required-asterisk styling in dark mode.The new
.pmf-required-asteriskclass uses the appropriate variable (var(--bs-danger)) and integrates well with the dark-mode color scheme.
366-384: Good A11y enhancement: logo styling for high-contrast mode.The logo scaling, border, and filter adjustments improve visibility in high-contrast mode. The
filter: brightness(0) invert(1) saturate(2)approach is creative for inverting logo colors, though consider testing with various logo image formats to ensure the filter produces the desired result across all images (SVG, PNG, JPEG, etc.).Verify that the logo filter renders correctly across all supported image formats used in the project.
386-393: Verify heading contrast in high-contrast mode.The heading styling uses
background-clip: textwithvar(--bs-primary), which depends on the primary color variable value. While the intent is good for A11y, verify that the resulting text/background contrast meets WCAG AA or AAA standards. If--bs-primaryis a color with insufficient contrast against the black background (#000000 implied), the heading may be difficult to read in high-contrast mode—defeating the purpose.Test the heading in high-contrast mode and confirm it meets WCAG color contrast requirements (minimum 4.5:1 for normal text, 3:1 for large text).
409-413: Good: required-asterisk styling in high-contrast mode.The required-asterisk class uses
var(--bs-primary)withfont-weight: bold, which is consistent with the high-contrast theming approach and enhances visibility. Good alignment with the A11y goals.
Moves the required asterisk inside the conditional so it only appears when the category field is required, improving form label accuracy.
2 participants
small style updates für high contrast mode
remove unused font files from asset directory
Summary by CodeRabbit
Style
Bug Fixes / Changes
✏️ Tip: You can customize this high-level summary in your review settings.