Skip to content

Polish generated sitemap stylesheet#353

Merged
PrzemyslawKlys merged 1 commit intomainfrom
codex/polish-sitemap-css
May 7, 2026
Merged

Polish generated sitemap stylesheet#353
PrzemyslawKlys merged 1 commit intomainfrom
codex/polish-sitemap-css

Conversation

@PrzemyslawKlys
Copy link
Copy Markdown
Member

No description provided.

@claude
Copy link
Copy Markdown

claude Bot commented May 7, 2026

Code Review — PR #353: Polish generated sitemap stylesheet

Overview

This PR improves the browser-facing CSS stylesheet written alongside XML sitemaps when BrowserStylesheet is enabled. Changes are purely cosmetic — no sitemap XML behavior or parsing logic is touched. The stylesheet is embedded as a C# raw string literal in WebSitemapGenerator.cs and written to sitemap.css at build time.


What's Good

  • CSS counters: Using counter-reset/counter-increment/counter(sitemap-entry) to number entries is a clean, pure-CSS approach with no JS dependency.
  • Responsive breakpoint: The new @media (max-width: 720px) block was missing before. The adjusted paddings (52px 14px 14px) correctly account for the absolute-positioned counter badge (26px height + 14px top + gap), so the math is right.
  • link element visibility: Changing link from display: none to display: inline-block with content: attr(hreflang) is a good UX improvement — hreflang alternates are now visible to humans inspecting the sitemap, which is the whole point of the stylesheet.
  • border-left override order: border: 1px solid #d9e2ef followed by border-left: 4px solid #2563eb is intentional and correct — the cascade works as expected.
  • Tests updated: The two new assertions (counter(sitemap-entry) and link::before) catch regressions in the key new CSS features without over-specifying the full stylesheet content.

Observations / Minor Suggestions

  1. Duplicated ::before / ::after blocks for urlset vs sitemapindex
    urlset::before and sitemapindex::before share identical declarations (same max-width, margin, color, font-size, font-weight) except for content. Same for ::after. These could be collapsed:

    urlset::before, sitemapindex::before {
      /* shared properties */
    }
    urlset::before    { content: "XML sitemap"; }
    sitemapindex::before { content: "XML sitemap index"; }

    This would reduce duplication and make future tweaks a single-line change. Not a blocker, just a maintenance nit.

  2. min-height: 100vh on urlset/sitemapindex: These are XML element selectors. min-height: 100vh makes sense when the element maps to the page body — and since the browser renders the XML with urlset or sitemapindex as the document root, this is correct. No issue, just worth noting for future readers.

  3. border-radius: 999px on counter badges and pill labels: This is a widely-accepted hack for "always pill-shaped." No real concern here.

  4. Hardcoded descriptive strings in ::after: The strings "Generated for search engines. Styled only for human inspection." and "Sitemap index. Each entry points to a machine-readable sitemap." are baked into the C# source. If these ever need to be configurable or localized, they'd need a dedicated parameter. For a debugging/inspection stylesheet this is perfectly acceptable.

  5. Test coverage adequacy: The two added assertions are appropriate smoke tests. A test that verifies @media presence would be a nice addition but is not necessary for this kind of visual-only change.


Security / Performance

  • No security concerns — this is a static CSS file written to the output directory.
  • The additional CSS (89 lines) adds negligible size to the generated file.

Summary

This is a clean, self-contained improvement. The CSS is well-structured, the mobile breakpoint is correct, and the tests are appropriately updated. The only actionable suggestion is consolidating the duplicated ::before/::after declarations, which is optional but would improve maintainability.

Verdict: Looks good to merge. The consolidation nit is optional.

@PrzemyslawKlys PrzemyslawKlys merged commit ac5cf90 into main May 7, 2026
6 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the codex/polish-sitemap-css branch May 7, 2026 15:43
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