Fix Academy comparison table rendering - #7936
Conversation
Signed-off-by: DS123-ally <dineshseervi33716@gmail.com>
📝 WalkthroughWalkthroughThe Academy vs Moocit comparison page now uses HTML tables with textual status values. Scoped styles define table borders, spacing, alignment, themed headers, and status-specific text styling. Both themes now define an error color. ChangesComparison table presentation
Theme color definitions
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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: 2
🧹 Nitpick comments (1)
src/collections/resources/comparison/layer5-academy-vs-moocit/index.mdx (1)
38-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd explicit column scopes to every table header.
Each table uses
<th>withoutscope="col". Addscope="col"toFeature,Moocit,Layer5, andNotesin every<thead>.As per coding guidelines, MDX must meet WCAG 2.1 Level AA accessibility requirements and use semantic HTML.
Also applies to: 103-106, 174-177, 251-254
🤖 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/collections/resources/comparison/layer5-academy-vs-moocit/index.mdx` around lines 38 - 41, Add scope="col" to each table-header <th> element in every thead, including the headers Feature, Moocit, Layer5, and Notes in all referenced table sections. Preserve the existing header text and table structure while ensuring every column header has an explicit column scope.Source: Coding guidelines
🤖 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/collections/resources/Resources.style.js`:
- Around line 20-31: Add status-specific hooks to the comparison table cells in
the layer5 academy-versus-moocit content, preserving the Yes, Partial, and No
labels. Update the comparison styles around div.comparison td to target those
hooks and apply theme-based teal, yellow, and red colors respectively; use
data-status attributes or BEM-like class names, not generic classes.
- Around line 14-18: Update the comparison table styling around the
div.comparison table selector to prevent narrow-screen overflow by adding a
comparison__table-wrapper with horizontal auto-scrolling, or an equivalent
responsive media-query rule. Preserve the table’s native display and existing
desktop styling while ensuring long notes and multiple columns remain usable on
mobile.
---
Nitpick comments:
In `@src/collections/resources/comparison/layer5-academy-vs-moocit/index.mdx`:
- Around line 38-41: Add scope="col" to each table-header <th> element in every
thead, including the headers Feature, Moocit, Layer5, and Notes in all
referenced table sections. Preserve the existing header text and table structure
while ensuring every column header has an explicit column scope.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 519cf2a7-e7ba-4d3f-83c4-716b306f0b99
📒 Files selected for processing (2)
src/collections/resources/Resources.style.jssrc/collections/resources/comparison/layer5-academy-vs-moocit/index.mdx
| div.comparison table { | ||
| border-collapse: collapse; | ||
| margin: 1.5rem 0 2rem; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent horizontal overflow on narrow screens.
width: 100% does not prevent the table’s intrinsic width from exceeding a narrow viewport. Four columns and long notes can create page-level horizontal scrolling on mobile.
Wrap each table in a comparison__table-wrapper with overflow-x: auto, or add an equivalent mobile breakpoint. Keep the table’s native display.
As per coding guidelines, CSS changes must maintain responsive design with media queries.
Example wrapper style
+ div.comparison__table-wrapper {
+ max-width: 100%;
+ overflow-x: auto;
+ }🤖 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/collections/resources/Resources.style.js` around lines 14 - 18, Update
the comparison table styling around the div.comparison table selector to prevent
narrow-screen overflow by adding a comparison__table-wrapper with horizontal
auto-scrolling, or an equivalent responsive media-query rule. Preserve the
table’s native display and existing desktop styling while ensuring long notes
and multiple columns remain usable on mobile.
Source: Coding guidelines
Signed-off-by: DS123-ally <dineshseervi33716@gmail.com>
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/theme/app/themeStyles.js`:
- Around line 302-304: Update the dark-theme errorColor value in themeStyles to
a lighter color such as `#FF6B6B`, ensuring its contrast against the `#121212`
background meets WCAG 2.1 AA for normal text; leave other theme colors
unchanged.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e2a5d19-9985-4c1e-a6cf-d222bc4cde21
📒 Files selected for processing (3)
src/collections/resources/Resources.style.jssrc/collections/resources/comparison/layer5-academy-vs-moocit/index.mdxsrc/theme/app/themeStyles.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/collections/resources/Resources.style.js
- src/collections/resources/comparison/layer5-academy-vs-moocit/index.mdx
| // error red | ||
| errorColor: "#D32F2F", | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a lighter error color in the dark theme.
Resources.style.js applies errorColor to td[data-status="no"]. Against the dark theme background #121212, #D32F2F does not provide sufficient contrast for normal-size text. WCAG 2.1 AA requires a contrast ratio of at least 4.5:1 for normal text. (w3.org)
Use a dark-theme-specific value, such as #FF6B6B, and verify the rendered table background.
As per coding guidelines, **/*.{js,jsx,md,mdx} must meet WCAG 2.1 Level AA accessibility requirements, including adequate color contrast.
🤖 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/theme/app/themeStyles.js` around lines 302 - 304, Update the dark-theme
errorColor value in themeStyles to a lighter color such as `#FF6B6B`, ensuring its
contrast against the `#121212` background meets WCAG 2.1 AA for normal text; leave
other theme colors unchanged.
Source: Coding guidelines
|
@Sbragul26 okay |
Sbragul26
left a comment
Sbragul26
left a comment
There was a problem hiding this comment.
@DS123-ally, Please address the CodeRabbit comments.
okay |
2 participants
Description
Fixes the comparison tables on the Layer5 Academy vs Moocit page. Previously, the Markdown table syntax was displayed as raw text with
|characters instead of rendered rows and columns.##Issue No: 7935
Changes
Yes,No, andPartiallabels for comparison states.Comparisonspelling in the page frontmatter.Expected behavior
The comparison content is now displayed as properly structured tables, making the features of Layer5 Academy and Moocit easier to read and compare.
Validation
<table>elements are generated.git diff --checkpassed.Before
Comparison content appeared as raw pipe-delimited text.
After
Comparison content is rendered in structured rows and columns.
Summary by CodeRabbit
Summary by CodeRabbit