fix: use semantic table backgrounds#2708
Conversation
|
Overall readability score: 53.98 (🟢 +0)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Code Review
This pull request replaces hardcoded hex color values with CSS custom properties across several documentation files, and defines the new --docs-info-bg variable in tokens.css. A review comment correctly identifies an issue with the CSS selector table:not(data-exclude) in roles-and-permissions-for-organizations.md, suggesting it be updated to table:not([data-exclude]) to properly target the attribute.
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.
There was a problem hiding this comment.
Pull Request Overview
This review focused on the transition from hard-coded HEX values to semantic CSS tokens for documentation tables. While the core objective of defining and using the new --docs-info-bg token is met, two main areas require attention.
First, the CSS selector in the permissions documentation needs to be broader to include header cells (th), as the current td-only selector will likely fail to style table headers correctly. Second, several files exhibit inconsistent token usage where backgrounds are semantic but text colors remain hard-coded. This inconsistency reduces the effectiveness of the theme-aware tokens, particularly in dark mode. No security flaws were found, and Codacy analysis indicates the changes are up to standards, but these logic and consistency issues should be addressed prior to merging.
1 comment outside of the diff
docs/faq/code-analysis/why-does-codacy-show-unexpected-coverage-changes.md
line 393-400⚪ LOW RISK
Suggestion: To complete the migration to semantic tokens in this style block and ensure better dark mode support, consider replacing these hardcoded hex values with semantic variables.\n\nsuggestion\n.text-green {\n color: var(--docs-success);\n}\n\n/*Red text*/\n.text-red {\n color: var(--docs-danger);\n}\n
Test suggestions
- Missing recommended test scenario: Verify that .background-red correctly utilizes the --docs-danger-bg token\n- [ ] Missing recommended test scenario: Verify that the permissions table header row utilizes the --docs-bg-brand token\n- [ ] Missing recommended test scenario: Verify that .yes, .no, and .maybe classes use success, danger, and info background tokens respectively\n- [ ] Missing recommended test scenario: Verify the presence and correct values of --docs-info-bg in light and dark mode CSS blocks
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing recommended test scenario: Verify that .background-red correctly utilizes the --docs-danger-bg token\n- [ ] Missing recommended test scenario: Verify that the permissions table header row utilizes the --docs-bg-brand token\n- [ ] Missing recommended test scenario: Verify that .yes, .no, and .maybe classes use success, danger, and info background tokens respectively\n- [ ] Missing recommended test scenario: Verify the presence and correct values of --docs-info-bg in light and dark mode CSS blocks
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
Summary
Validation
mkdocs build --strict