Skip to content

fix(docs): move feature icon inside <dt> for valid <dl> content model#1171

Merged
chhoumann merged 1 commit into
masterfrom
fix/feature-dl-content-model
Apr 23, 2026
Merged

fix(docs): move feature icon inside <dt> for valid <dl> content model#1171
chhoumann merged 1 commit into
masterfrom
fix/feature-dl-content-model

Conversation

@chhoumann
Copy link
Copy Markdown
Owner

@chhoumann chhoumann commented Apr 23, 2026

Summary

Follow-up to #1168. Post-merge review from Devin flagged that the feature grid in docs/src/pages/index.tsx:280-288 places a <span className={styles.featureIcon}> as a direct child of the <div> that wraps each <dt>/<dd> pair.

Per the HTML spec, when <div> is a child of <dl> its content model is restricted to <dt> and <dd> (plus script-supporting elements). The icon span violates that, fails validation, and can confuse assistive tech that relies on strict term/description pairing.

Fix

Moved the icon span inside <dt>. The icon is semantically part of the term, so this is the natural home for it. The visual layout is unchanged — the <dt> is now a small flex column (icon chip above title text, 0.75rem gap), and the <div> now contains exactly <dt> + <dd>.

Test plan

  • DOM inspection: <div> inside <dl> has zero <span> children; <dt> contains icon span + title span
  • Visual regression check: title text is still 17px/600, icon chip is still 36×36 with soft-purple background, spacing matches pre-fix layout
  • aria-hidden="true" on the icon span so screen readers read the term as just the title text
  • Eyeball in a browser before merging

Open in Devin Review

Summary by CodeRabbit

  • Style
    • Improved feature title layout with refined spacing and visual hierarchy in the documentation.
    • Enhanced accessibility by properly handling decorative icons in feature listings.

Per HTML spec, a <div> child of <dl> may only contain <dt> and <dd>
elements. The icon <span> was sitting as a sibling of <dt>/<dd>, which
fails validation and can confuse assistive tech that relies on strict
term/description pairing. The icon is semantically part of the term, so
nesting it in <dt> is the natural fix.
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
quickadd Ready Ready Preview Apr 23, 2026 1:28pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8373eb0b-aff4-48a9-916e-c67db6a183fd

📥 Commits

Reviewing files that changed from the base of the PR and between dd1ea21 and 259aa5a.

📒 Files selected for processing (2)
  • docs/src/pages/index.module.css
  • docs/src/pages/index.tsx

📝 Walkthrough

Walkthrough

The landing page feature list undergoes a markup and styling restructure. The CSS changes redefine .featureTitle as a flex column container with explicit gap spacing, removes margins from .featureIcon, and introduces .featureTitleText for isolated title typography. The HTML markup places the icon (with aria-hidden="true") and title text within the same dt element for semantic clarity.

Changes

Cohort / File(s) Summary
Feature List Restructure
docs/src/pages/index.module.css, docs/src/pages/index.tsx
CSS reworked to move spacing control from .featureIcon margin to .featureTitle gap property; new .featureTitleText class isolates title styling. HTML markup updated to nest icon (aria-hidden) and title text within the same element with appropriate class bindings.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰✨ We tuck icons away with a whisper so light,
And titles stand tall in their own special height,
With gaps that align, and with ARIA's grace,
The features now flourish in their perfect space! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: moving the feature icon into the
element to fix the HTML content model validity of the
structure.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/feature-dl-content-model

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying quickadd with  Cloudflare Pages  Cloudflare Pages

Latest commit: 259aa5a
Status: ✅  Deploy successful!
Preview URL: https://4dced3ab.quickadd.pages.dev
Branch Preview URL: https://fix-feature-dl-content-model.quickadd.pages.dev

View logs

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@chhoumann chhoumann merged commit 7dcc8e1 into master Apr 23, 2026
5 checks passed
@chhoumann chhoumann deleted the fix/feature-dl-content-model branch April 23, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant