patch: tutorials list ui and header issues#17635
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Critical Issue FoundEmpty At the top of the tutorials wrapper (line ~273 in the diff), there's: This heading has no text content. It's always rendered regardless of filter state. Screen readers will announce "heading level 2" with nothing after it — that's an accessibility anti-pattern and arguably worse than having no heading at all. Contrast this with the conditional Minor Observations (non-critical)
Everything Else Looks Clean
Reviewed by Claude (Opus 4.6) |
|
Patched above issues
|
myelinated-wackerow
left a comment
There was a problem hiding this comment.
Solid cleanup PR. A few notes:
Spacing change is intentional but worth eyeballing: space-y-6 (24px uniform) replaces the previous mixed margins (mb-8/mt-6), which tightens the title-to-meta gap by ~8px. Assuming this is the intended fix per the PR description — worth a quick check on the deploy preview on mobile to make sure it doesn't feel too tight with the skill tag alongside the title.
External link behavior improvement: Worth noting that BaseLink opens external URLs in new tabs (target="_blank"), whereas the old bare <a> navigated in the same tab. This is probably the better UX for external tutorials — just calling it out as a behavioral change.
Overall: heading hierarchy fix (p → h3, sr-only h2) is a real a11y win, BaseLink over raw <a> is strictly better for internal navigation, skill tag color coding and conditional rendering are both improvements. Clean removal of dead code. Looks good.
🤖 Reviewed by Claude / claude-opus-4-6

Description
sr-onlyh2 when results are available, with h3 used for titleTextcomponent;space-y-6used from parent insteadsr-onlyh2 stringPreview link
https://deploy-preview-17635.ethereum.it/developers/tutorials
cc: @konopkja