Add white contrast ring around highlighted element outline - #1824
Conversation
The magenta outline used to highlight elements with issues can lack sufficient contrast against page backgrounds close to that color. A white box-shadow ring between the element and the outline ensures the indicator stays visible regardless of the underlying page colors. Fixes #1768
📝 WalkthroughWalkthroughThe ChangesHighlight ring contrast fix
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a white box-shadow ring to the highlighter element in app.scss to maintain visibility against various page backgrounds. The reviewer suggested increasing the box-shadow spread from 2px to 5px to match the outline-offset, eliminating a gap that could reduce contrast, and adding a comment explaining the use of !important.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| outline-color: magenta !important; | ||
| // White ring between the element and the magenta outline so the indicator | ||
| // stays visible even when the page background is close to the outline color. | ||
| box-shadow: 0 0 0 2px variables.$color-white !important; |
There was a problem hiding this comment.
The outline-offset is set to 5px !important, but the box-shadow spread is only 2px. This leaves a 3px gap of the page background between the white ring and the magenta outline. If the page background is close to magenta, the outline will blend into the background and the gap, losing its dashed appearance.
Increasing the box-shadow spread to 5px (to match the outline-offset) will fill the entire gap with white, ensuring the dashed magenta outline always sits directly against a high-contrast white background. Note that when using !important for compatibility reasons, it must be accompanied by an explanatory comment.
| box-shadow: 0 0 0 2px variables.$color-white !important; | |
| box-shadow: 0 0 0 5px variables.$color-white !important; // Required for compatibility overrides |
References
- If
!importantis required for compatibility reasons, its use is acceptable when accompanied by an explanatory 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontendHighlighterApp/sass/app.scss`:
- Around line 54-56: The SCSS comment in the app stylesheet needs a blank line
before it to satisfy Stylelint. Update the block around the `box-shadow`
declaration in `app.scss` so the `// White ring...` comment is separated from
the preceding rule by an empty line, keeping the `variables.$color-white` shadow
declaration and nearby indicator styles intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 652dd90f-bbe7-42be-a520-3e80e507ef81
📒 Files selected for processing (1)
src/frontendHighlighterApp/sass/app.scss
| // White ring between the element and the magenta outline so the indicator | ||
| // stays visible even when the page background is close to the outline color. | ||
| box-shadow: 0 0 0 2px variables.$color-white !important; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the blank line required before this comment.
Stylelint flags this new // comment because it needs an empty line before it. As written, this will fail the SCSS lint check.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 54-54: Expected empty line before comment (scss/double-slash-comment-empty-line-before)
(scss/double-slash-comment-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontendHighlighterApp/sass/app.scss` around lines 54 - 56, The SCSS
comment in the app stylesheet needs a blank line before it to satisfy Stylelint.
Update the block around the `box-shadow` declaration in `app.scss` so the `//
White ring...` comment is separated from the preceding rule by an empty line,
keeping the `variables.$color-white` shadow declaration and nearby indicator
styles intact.
Source: Linters/SAST tools
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 109bfeb47b
ℹ️ 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".
| outline-color: magenta !important; | ||
| // White ring between the element and the magenta outline so the indicator | ||
| // stays visible even when the page background is close to the outline color. | ||
| box-shadow: 0 0 0 2px variables.$color-white !important; |
There was a problem hiding this comment.
When the selected element has a white/light background and the surrounding page background is close to magenta, this 2px white shadow can blend into the element while the magenta outline still sits 5px away because of outline-offset: 5px, leaving about 3px of the original page background between the ring and the outline. In that case the outline remains adjacent to the same magenta-like page background the change is trying to guard against, so the highlighter can still disappear for the issue being fixed; the separator needs to reach the offset outline or the offset needs to be reduced.
Useful? React with 👍 / 👎.
2 participants
Short description: The frontend highlighter's magenta outline can lack sufficient contrast against page backgrounds close to that color (WCAG 1.4.11 Non-text Contrast). This adds a white
box-shadowring between the highlighted element and the magenta outline, so there's always a neutral separator regardless of the underlying page background.Change is in
src/frontendHighlighterApp/sass/app.scss(.edac-highlight-element-selected): addsbox-shadow: 0 0 0 8px $color-whitealongside the existing dashed magenta outline.Checklist
Generated by Claude Code
Summary by CodeRabbit