refactor: migrate leaderboard div-based grid to semantic table (#180)#226
refactor: migrate leaderboard div-based grid to semantic table (#180)#226rishab11250 wants to merge 2 commits into
Conversation
…vg#180) Replace the CSS Grid-based leaderboard layout with a proper HTML table + thead/tbody structure for accessibility. Add th scope="col" and td elements with ARIA-native semantics instead of ARIA role workarounds from PR codepvg#184. Changes: - leaderboard.html: div.grid -> table + thead + tbody with th/td - render.js: createElement('tr')/('td') instead of 'div' - main.css: grid selectors -> table-layout: fixed with explicit column widths, sticky thead, mobile/compare-mode adjustments - Empty states use tr td colspan=N for valid table structure
|
Thank you for submitting a pull request. Please ensure your changes comply with the project's contribution guidelines and that all workflow checks pass successfully. Formatting and Branching
|
jagdish-15
left a comment
jagdish-15
left a comment
There was a problem hiding this comment.
Hey, did you test this locally after the migration?
There seem to be a few visual regressions compared to the current leaderboard. The Score column is no longer aligned correctly, there appears to be a lot more padding in those cells than before, and some of the header styling/colours seem to have been lost.
Could you take another look
- Reduce per-cell padding from 1.25rem to 0.3rem, use first/last-child for outer edge spacing (fixes excessive cell padding) - Move header border-bottom from tr (can't render in separate collapse) to th elements (fixes missing header border) - Remove display:flex from score-header th, use text-align:center (fixes Score column width in table-layout:fixed) - Override justify-content:flex-end to center in .score-cell .mobile-score (fixes Score data cell alignment)
|
|
Looking at the screenshot you attached, there are still a couple of visual differences compared to the current leaderboard on production (codepvg.onrender.com). The header colours seem to be missing, and the Score header is wrapping onto two lines with the tooltip icon appearing below the text. Could you take another look at those before we merge? |
2 participants
Description
Migrate the leaderboard
<div>-based CSS Grid layout to a semantic HTML<table>element for proper accessibility. Previously, ARIA role workarounds (PR #184) were added to the div grid as a stopgap — this change provides native table semantics instead.Linked Issue
Fixes #180
Changes Made
<div class="leaderboard">→<table class="leaderboard">,<div class="leaderboard-header">→<thead class="leaderboard-header"><tr>with<th scope="col">cells,<div id="leaderboard-body">→<tbody id="leaderboard-body">, skeleton rows →<tr>+<td>(9 cells per row, including hidden checkbox cell)renderLeaderboardRow()creates<tr>/<td>instead of<div>; empty states use<tr><td colspan="9">for valid table structuredisplay: grid/grid-template-columnsreplaced withdisplay: table/table-layout: fixed/ explicitth:nth-child()column widths; sticky positioning on<th>; cell alignment updated for table context; compare-mode checkbox usestable-cellinstead offlex; mobile breakpoint hides the<table>itself; skeleton rows usetdpadding + nth-child width selectorsType of Change
Testing
Tested locally
Tested on mobile viewport (if applicable)
No console errors introduced
npx prettier --check .passesnode --checkpasses on render.jsSkeleton loading → data transition confirmed
Compare mode checkbox show/hide confirmed
Mobile card layout preserved (table hidden at 940px breakpoint)
Checklist
npx prettier --write .before submittingfeature/*branch, not themainbranchScreenshots / Screen Recording
N/A — visual appearance is identical to the previous grid layout (same column widths, colors, spacing, hover states, sticky header, skeleton shimmer, mobile card view). No visual regression expected.