fix: correct mobile layout, stale pins and the pages left behind by the consolidation - #7
Conversation
The "trusted by" and community bands bleed past the content column with a -1.5rem margin. Material sets the root font to 125%, making that 30px, but below its 76.25em breakpoint the column only has a 16px gutter — so the bands hung 14px off the right edge and the whole page could be dragged sideways. Wide screens have a bigger gutter, which is why it only showed up below 1220px. Measured across 320-1440px: the overflow is gone at every width from 390px up, and the bands now stop exactly at the column edge instead of short of it. A 10px overflow remains at 320px, from Material's own off-canvas drawer rather than anything here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
The eighteen avatars on the landing page are hotlinked from github.com with no dimensions, so the browser could not reserve their boxes and the section reflowed as each one arrived. They also all loaded eagerly, below the fold, competing with the content above them. The grid already pins them to 60x60 in CSS; say so in the markup, and mark them lazy. Hotlinking itself is unchanged — the page still depends on github.com being reachable, which is worth revisiting separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Five snippets across four pages still pinned rev: v2.13.0 after 2.13.1 shipped. A stale pin is invisible — the snippet keeps working, it just installs an older release than the page around it describes — and nothing here read those lines, so they only move when somebody remembers. Add a guard alongside the existing anti-drift tests: every rev: outside the blog must name the version pytest has installed, which CI installs unpinned from PyPI. Blog posts are exempt because they are dated records of what was current when they were written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Eleven of them wrapped mid-identifier even at 1440px — `subject- capitalized` split across two lines is hard to scan and hard to copy. The stylesheet already carried a rule for this, written against a `table.rules-index` class no table ever had, so it never applied and was dropped as dead code. Restore the intent properly: attr_list cannot put a class on a table, so the four index tables get a wrapper div. Other tables hold long regexes and allow-lists that must stay wrappable, which is why this is scoped rather than applied to every table. On a phone the tables now scroll inside Material's own wrapper; the page itself still does not scroll horizontally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
configuration, migration and troubleshoot were the pages the earlier consolidation left untouched, and it showed: Title Case headings against sentence case everywhere else, and prose that never got the same pass. Headings are now sentence case throughout. Beyond that: configuration. "Configuration Files" and "Configuration File Locations" said where the file goes three times between them; they are one section. The worked example mixed defaults with tightened values and marked only two of them, so copy-pasting it silently narrowed allow_commit_types and allow_branch_types — every line that departs from a default now says so, and the claim is checked against get_default_config() rather than asserted. Adds a note that a losing value is not warned about, which is the usual reason a setting "does nothing". migration. Led with a YAML-to-TOML diff of two whole files, which is the least useful framing: v1 files were regex, v2 files are booleans, so nothing translates line by line. It now explains that, gives the check-to-option mapping as a table with rule IDs, and keeps the worked example short. Verified each mapping against rules_catalog. troubleshoot. One topic, on a failure that no longer happens out of the box, and the bypass was presented before the fix. It now starts from reading the diagnostic, distinguishes skipping one hook from skipping all, and says plainly that neither affects CI — the thing people discover by pushing. Adds the two failures that actually get reported: a rule firing that nobody enabled, and nothing running because the flag was missing. The examples no longer use a maintainer's real name and address as the value to copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
✅ Deploy Preview for commit-check ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR revises configuration, migration, and troubleshooting documentation; updates examples to v2.13.1; adds responsive documentation styling; and checks documented pre-commit revisions against the installed version. ChangesDocumentation updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (1)
docs/configuration.md (1)
167-179: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd contract tests for source mappings.
tests/docs_sync_test.pychecks documented option names and defaults. It does not check source precedence, file discovery order, inheritance behavior, orCCHK_*and CLI mappings. Add focused tests for these new documentation contracts.🤖 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 `@docs/configuration.md` around lines 167 - 179, Add focused contract tests alongside the existing documentation checks in tests/docs_sync_test.py for configuration source precedence, file discovery order, inheritance behavior, and mappings between option names, CCHK_* environment variables, and CLI arguments. Use the documented configuration symbols and expected defaults, keeping each behavior covered by a targeted test.
🤖 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 `@docs/configuration.md`:
- Around line 17-25: Separate rule defaults from activation behavior in
docs/configuration.md lines 17-25 and docs/troubleshoot.md lines 71-79: qualify
no-file enforcement statements with the required rule flags, and replace the
blanket claim that all allow_* restrictions are disabled with the actual
documented allow-list defaults. Use consistent wording across both files and
preserve the options table as the source of default values.
- Around line 228-230: Update the options table annotation for message_pattern
in the configuration documentation so an empty default is described as using the
built-in pattern, not as disabled. Keep the surrounding explanation and behavior
unchanged, and use wording such as “built-in pattern” or “custom override
disabled” consistently.
In `@docs/migration.md`:
- Around line 56-67: Update the v2 migration example in the TOML configuration
to preserve the v1 policy by explicitly including the previously accepted commit
type “revert” in allow_commit_types and branch type “task” in
allow_branch_types, or clearly document that the migration intentionally changes
those policies.
In `@docs/rules.md`:
- Around line 91-92: Fix the rule-table links for CC101, CC102, CC201, CC202,
and CC301 in docs/rules.md by adding matching IDs to their target headings or
updating the links to the headings’ generated fragments. Ensure every affected
entry navigates correctly to its corresponding rule details.
---
Nitpick comments:
In `@docs/configuration.md`:
- Around line 167-179: Add focused contract tests alongside the existing
documentation checks in tests/docs_sync_test.py for configuration source
precedence, file discovery order, inheritance behavior, and mappings between
option names, CCHK_* environment variables, and CLI arguments. Use the
documented configuration symbols and expected defaults, keeping each behavior
covered by a targeted test.
🪄 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: e52390b8-1e5d-46f4-b883-7fb5cd873918
📒 Files selected for processing (10)
docs/assets/extra.cssdocs/configuration.mddocs/example.mddocs/guides/integrations.mddocs/index.mddocs/migration.mddocs/rules.mddocs/stylesheets/extra.cssdocs/troubleshoot.mdtests/docs_sync_test.py
| | [CC101](#cc101) | `author-name` | The committer name seems invalid | `-n` | ✅ On | | ||
| | [CC102](#cc102) | `author-email` | The committer's email seems invalid | `-e` | ✅ On | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the invalid rule-index fragments.
markdownlint-cli2 reports #cc101, #cc102, #cc201, #cc202, and #cc301 as invalid fragments. Add matching IDs to the target headings or update these links to the generated fragments. Otherwise, these entries do not navigate to their rule details.
Also applies to: 104-105, 115-115
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 91-91: Link fragments should be valid
(MD051, link-fragments)
[warning] 92-92: Link fragments should be valid
(MD051, link-fragments)
🤖 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 `@docs/rules.md` around lines 91 - 92, Fix the rule-table links for CC101,
CC102, CC201, CC202, and CC301 in docs/rules.md by adding matching IDs to their
target headings or updating the links to the headings’ generated fragments.
Ensure every affected entry navigates correctly to its corresponding rule
details.
Source: Linters/SAST tools
…ion gaps Three corrections from review, all of them things the rewrite stated too loosely. The defaults note claimed a no-file run "still enforces" a list of rules without saying that a check only runs when its own flag is passed, while troubleshoot said the opposite in isolation. Both now say which flag brings which defaults. The same note claimed every allow_* restriction is off by default, which is wrong for the two that matter most: allow_commit_types and allow_branch_types are allow-lists enforcing from the start. The six that really do permit everything are named instead of generalised. An empty default was described as always meaning "use the built-in pattern". It means three different things — message_pattern leaves conventional_commits to generate the pattern, author_name_pattern falls back to the built-in one, and require_rebase_target genuinely disables its check — so each is spelled out, and the table cell that said "" (disabled) now says "" (no custom pattern). The migration example dropped two types silently: v1 accepted `revert` as a commit type and `task` as a branch type, and neither is in the v2 defaults. That is the same footgun this branch fixed on the configuration page, so it gets the same treatment — the gap is named, with the lists to restore it and a note that setting either option replaces the default rather than extending it. Verified against get_default_config(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
|
Thanks — three of the four were real and are fixed in cc8f20a. Notes on all five: Fixed: rule defaults vs. rule activation. Correct, and the two halves did contradict each other. The defaults note now says which flag brings which defaults, and Fixed: empty
Each is now spelled out, and the cell reads Fixed: migration drops accepted values. Correct, and worth more than the diff you proposed. Verified against Not a defect: the ### author-name (CC101) { #cc101 }MD051 computes fragments from heading text alone and does not parse Skipping: contract tests for precedence, discovery, inheritance and Generated by Claude Code |
2 participants
What
Five fixes found while reviewing the site after #6, ranging from a layout bug on every phone to the three pages the consolidation in #4 never rewrote.
The landing page scrolled sideways on phones and tablets
The "trusted by" and community bands bleed past the content column with a
-1.5remmargin. Material sets the root font to 125%, making that 30px — but below its76.25embreakpoint the column only has a 16px gutter to give back. The bands hung 14px off the right edge and the whole page could be dragged sideways. Wide screens have a bigger gutter, which is why it only showed up under 1220px.Measured across 320–1440px: gone at every width from 390px up, and the bands now stop exactly at the column edge rather than short of it. A 10px overflow remains at 320px, from Material's own off-canvas drawer rather than anything here.
Adopter logos caused layout shift
The eighteen avatars are hotlinked from github.com with no dimensions, so the browser could not reserve their boxes and the section reflowed as each arrived — and they all loaded eagerly, below the fold. The grid already pins them to 60×60 in CSS; the markup now says so, and they are lazy.
Hotlinking itself is unchanged, so the page still depends on github.com being reachable. Worth revisiting separately.
Install snippets pinned a version that had shipped
Five snippets across four pages still said
rev: v2.13.0after 2.13.1 released. A stale pin is invisible — the snippet keeps working, it just installs an older release than the page around it describes.Adds a guard alongside the existing anti-drift tests: every
rev:outside the blog must name the version pytest has installed, which CI takes unpinned from PyPI. Blog posts are exempt, being dated records of what was current when written. Verified the guard fails on a deliberately stale pin, not just that it passes today.Rule names wrapped mid-identifier
Eleven of them broke across two lines even at 1440px —
subject-capitalizedsplit in half is hard to scan and hard to copy. The stylesheet already carried a rule for this, written against atable.rules-indexclass no table ever had, so it never applied and #6 removed it as dead code. Restored properly:attr_listcannot put a class on a table, so the four index tables get a wrapper div, scoped so the tables holding long regexes stay wrappable.On a phone those tables now scroll inside Material's own wrapper; the page itself still does not.
The three pages the consolidation skipped
configuration,migrationandtroubleshootkept Title Case headings against sentence case everywhere else, and the prose never got the same pass. Headings are now consistent, and each had a substantive problem:configuration — "Configuration Files" and "Configuration File Locations" said where the file goes three times between them; now one section. The worked example mixed defaults with tightened values and marked only two, so copy-pasting it silently narrowed
allow_commit_typesandallow_branch_types— the latter from 21 types to 5, which starts rejectingdependabot/andclaude/branches. Every departure from a default is now marked, and the markers were checked againstget_default_config()rather than asserted.migration — led with a YAML-to-TOML diff of two whole files, the least useful framing: v1 files were regex, v2 files are booleans, so nothing translates line by line. Now says that, and gives the check-to-option mapping as a table with rule IDs, each verified against
rules_catalog.troubleshoot — one topic, on a failure that no longer happens out of the box, presenting the bypass before the fix. Now starts from reading the diagnostic, separates skipping one hook from skipping all, and states that neither affects CI — the part people discover by pushing. Adds the two failures that actually get reported: a rule firing nobody enabled, and nothing running because the flag was missing. Examples no longer use a maintainer's real name and address as the value to copy.
Test plan
mkdocs build --strictclean, including anchor validation for the new cross-referencesNot verified
The proxy here blocks external hosts, so I could not open the deploy preview. Everything above was measured against a local build reproducing Netlify's exact mode (
NETLIFY=true,SITE_URLset), plus a base-commit build for before/after comparison. Worth a look at the preview before merging.Generated by Claude Code
Summary by CodeRabbit