[style] unify body prose at max-w-prose across public site#2416
[style] unify body prose at max-w-prose across public site#2416dewierwan wants to merge 14 commits into
Conversation
|
@claude review |
render
Bot
temporarily deployed
to
dewi/cap-prose-line-length - bluedot-preview PR #2416
WalkthroughThe PR updates website layout styling across multiple components and pages. Documentation in CLAUDE.md is revised to clarify min-width/min-height usage and add prose width guidance. Across the codebase, numerous components receive structural updates to wrap content in max-width containers using max-w-prose and mx-auto classes. Page-level Section wrappers are changed from max-w-3xl to max-w-prose. Additionally, JobsListSection gains an invisible anchor for navigation, and QuoteSection receives an accessibility attribute. No changes to component signatures or public APIs occur. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Claude finished @dewierwan's task —— View job Code Review for Prose Width CappingI've completed my review of the prose width capping implementation. Here's my comprehensive analysis: Todo List
Review SummaryOverall Assessment: ✅ Excellent implementation The changes are clean, consistent, and well-executed. The implementation properly addresses the readability concerns mentioned in the PR description. Positive Aspects
Technical Details
Code Quality Notes
Potential Considerations (Not Issues)
RecommendationsNo changes needed. This is a solid implementation that:
The PR is ready for merge. |
Greptile SummaryThis PR caps prose line length on the
Confidence Score: 5/5Safe to merge — purely additive CSS wrapper changes with no logic, no data flow, and no behavioural side-effects. All four components receive an identical, mechanically correct structural change: a No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["<section class='section-body'>\nflex flex-col · max-w-1436px · mx-auto"] --> B["<div class='max-w-prose mx-auto'>\n~65ch · centred in flex column"]
B --> C["<h3> Section heading"]
B --> D["Body content\n(paragraphs / mapped items)"]
style A fill:#e8f0fe,stroke:#4a90d9
style B fill:#d4edda,stroke:#28a745
style C fill:#fff3cd,stroke:#ffc107
style D fill:#fff3cd,stroke:#ffc107
Reviews (1): Last reviewed commit: "[chore] PR #2416: drop screenshots (embe..." | Re-trigger Greptile |
|
Bot pass:
No findings to address. Holding for human eyeball. |
render
Bot
temporarily deployed
to
dewi/cap-prose-line-length - bluedot-preview PR #2416
render
Bot
temporarily deployed
to
dewi/cap-prose-line-length - bluedot-preview PR #2416
|
@claude review |
|
Claude finished @dewierwan's task —— View job Code Review for Prose Width CappingI've completed my review of the prose width capping implementation. Here's my comprehensive analysis: Todo List
Review SummaryOverall Assessment: ✅ Excellent implementation The changes are clean, consistent, and well-executed. The implementation properly addresses the readability concerns mentioned in the PR description. Positive Aspects
Technical Details
Code Quality Notes
Potential Considerations (Not Issues)
RecommendationsNo changes needed. This is a solid implementation that:
The PR is ready for merge. |
render
Bot
temporarily deployed
to
dewi/cap-prose-line-length - bluedot-preview PR #2416
render
Bot
temporarily deployed
to
dewi/cap-prose-line-length - bluedot-preview PR #2416
|
We use 840px elsewhere (e.g. on lander pages) to solve this problem, and there is at least one place where the 840px/65ch values clash to create conflicting column widths: 
I suggest adopting 840px across the board, since it's the standard we had already generally adopted. This is flagged as out of scope in the PR description, " Update: Oh I see (happy to make this change and finish off the PR overall if you want) |
|
(This is Claude replying on Dewi's behalf — Dewi will weigh in too.) Pushback with a counter-proposal — would value your take. The team's own So the counter: keep this PR at Concretely the sweep would be:
Yes please on the offer to help — happy to coordinate. Want to chat for 5 min before swinging the hammer, or shall Dewi just push and you can react to the diff? |
Body copy on /about (Intro, Beliefs, Values) and /join-us (Why us) ran full-page-width on desktop (~1436px ≈ 130+ chars per line), making it harder to read. Cap at max-w-prose (~65ch) per Baymard / WCAG 1.4.8 readability guidance. Verified at 320/480/720/1024/1440/1920 — max ch is 65 across all widths, 0 violators.
Wrap heading + prose container in a single max-w-prose mx-auto wrapper per section. Avoids the "orphaned heading flush-left, prose centred" mismatch — heading and body share one anchor.
Open roles + Support our mission lists were full-page-width while the new Why us section above them was 65ch-centred. The width discontinuity was visually jarring. Cap both list sections at max-w-3xl (768px) — wider than max-w-prose so titles + Learn-more CTAs have breathing room, narrow enough to unify the page geometry.
Why us at max-w-prose and JobsListSection at max-w-3xl gave the page two slightly-different-width centred columns — visually jarring. Cap both at max-w-prose so the page reads as one editorial column from Why us through the role list. Also keeps everything inside the 80ch readability ceiling (max-w-3xl was ~87ch, technically over the limit even though list rows are exempt from the strict prose rule).
Per Will's feedback on the original About+Join Us PR, unify body prose width across the public site at max-w-prose (~65ch) — the team's documented standard in apps/website/CLAUDE.md and the WCAG 1.4.8 sweet spot. Lander prose sections and policy/markdown wrappers were running 840–928px (~96–105ch), past the readability ceiling. - 5 lander prose components (WhoIsThisForTextSection, HowTheCourseWorksSection, CourseBenefitsTextSection, FieldBuildingSection, QuoteSection): max-w-text → max-w-prose. Card/grid sections (Schedule, Pathways, CourseInformation, CourseOutcomes, etc.) untouched — they're not prose. - 7 markdown wrapper pages (privacy-policy, code-of-conduct, attendance-policy, our-community, resources, missions/[slug], join-us/[slug]): Section className max-w-3xl → max-w-prose. - apps/website/CLAUDE.md: tightened the convention to distinguish prose (max-w-prose) from structured layouts (max-w-text / max-w-section-wide). Verified at 320/480/720/1024/1440/1920 via Playwright; all body prose ≤65ch on every changed page (hero subtitles excluded per CLAUDE.md). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dewierwan
force-pushed
the
dewi/cap-prose-line-length
branch
from
25b097e to
8179cc3
Compare
Extends PR #2416's prose-width sweep to bespoke per-page sections on program/grant landers that were running 100ch+ — the actual sites users read end-to-end. Pure-prose sections (whole inner div wrapped in max-w-prose mx-auto): - incubator-week: AboutBlueDotSection, TrackRecordSection, LogisticsSection, TheWeekSection - advising: WhatThisIsForSection, WhatToExpectSection, WhoYouAreSection - career-transition-grant: ExpectationsSection Mixed sections (prose group wrapped, card grids left at section width): - incubator-week: WhatThisIsForSection (3 prose groups + About-you grid) - career-transition-grant: WhatThisIsForSection (prose + 3-card support grid) - rapid-grants: WhatThisIsForSection (prose + 2-card decision grid) List pages (PageListGroup wrapped): - pages/programs.tsx: ProgramsList wrapped in max-w-prose - rapid-grants: FundedProjectsSection (GranteesListSection) wrapped Verified at 320/480/720/1024/1440/1920 via Playwright on /programs, /programs/incubator-week, /programs/advising, /programs/career-transition-grant, /programs/rapid-grants, /grants/rapid. All body prose ≤69ch on every page; remaining 76ch hits are FAQ accordion answers (under 80ch ceiling) and hero subtitles (excluded per CLAUDE.md). Card/grid sections (Schedule, Pathways, CourseInformation, Outcomes, etc.) intentionally untouched — they are structured layouts, not prose. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
render
Bot
temporarily deployed
to
dewi/cap-prose-line-length - bluedot-preview PR #2416
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/website/src/components/incubator-week/WhatThisIsForSection.tsx (1)
63-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win"About you" heading sits outside the
max-w-prosewrapper, breaking alignment with the headings above it."What you'll do" and "Track record" (lines 32, 43) are centred at ~65ch inside the new wrapper. The "About you"
h3at line 65 is outside it, so on wide viewports it is left-aligned at the section's full content width — a noticeable horizontal jump relative to the prose headings above.If the heading is meant to label the 3-column card grid (which rightly stays full-width), the current placement is intentional but the visual misalignment may still read oddly. One option is to wrap just the heading in its own
max-w-prose mx-autodiv so it aligns with the prose column, while keeping the grid unrestricted:✏️ Suggested fix
</div> </div> - <h3 className={`${pageSectionHeadingClass} pt-4`}>About you</h3> + <div className="max-w-prose mx-auto w-full"> + <h3 className={`${pageSectionHeadingClass} pt-4`}>About you</h3> + </div> <div className="pt-2 grid gap-8 grid-cols-1 bd-md:grid-cols-3">🤖 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 `@apps/website/src/components/incubator-week/WhatThisIsForSection.tsx` around lines 63 - 67, The "About you" h3 is outside the max-width prose wrapper causing horizontal misalignment; wrap that heading in a container with the same prose constraints (e.g., a div with classes "max-w-prose mx-auto") or move the h3 into the existing prose wrapper so it uses the same width as the other headings — update the element that renders the About you heading (the h3 using pageSectionHeadingClass) so it is placed inside (or wrapped by) a max-w-prose mx-auto wrapper while leaving the following grid (the div with "pt-2 grid gap-8 grid-cols-1 bd-md:grid-cols-3") full-width.
🧹 Nitpick comments (1)
apps/website/CLAUDE.md (1)
120-121: ⚡ Quick winClarify the prose-width carve-out on the adjacent "avoid max-width" rule.
Line 120 says to avoid
max-width/max-height, while line 121 immediately prescribesmax-w-prose. Without a bridging note, an LLM assistant treating these as independent bullet points could apply line 120 and ignore line 121, or flagmax-w-proseusages as violations.✏️ Suggested wording
-- Use `min-width`/`min-height` for flexibility; avoid `max-width`/`max-height`. -- **Prose width:** body-text columns … +- Use `min-width`/`min-height` for flexibility; avoid `max-width`/`max-height` on structural elements (prose text is the exception — see rule below). +- **Prose width (exception to the rule above):** body-text columns …🤖 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 `@apps/website/CLAUDE.md` around lines 120 - 121, Clarify the apparent contradiction between the general "avoid `max-width`/`max-height`" rule and the specific `max-w-prose` guidance by adding a bridging sentence that explicitly states `max-w-prose` is an intentional exception for body-text (prose) columns; update the CLAUDE.md bullets so the "avoid `max-width`/`max-height`" rule remains general while a follow-up line (referencing `max-w-prose`, `max-w-text`, and `max-w-section-wide`) explains these allowed exceptions and when to use each (prose vs cards/grids/sections).
🤖 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 `@apps/website/src/components/join-us/JobsListSection.tsx`:
- Line 29: The invisible anchor div with id "open-roles-anchor" in
JobsListSection.tsx currently uses the utility class "bottom-48" which shifts
the anchor 192px upward; verify this large offset against the design and, if not
intentional, reduce it to match the sticky nav height plus a small buffer (e.g.,
replace "bottom-48" with "bottom-24" or "bottom-32") so the scroll target aligns
correctly under the sticky nav; update the class on the <div
id="open-roles-anchor"> element accordingly.
In `@apps/website/src/pages/join-us/`[slug].tsx:
- Line 96: The Section component on the join-us page is missing the mx-auto
utility so the constrained max-w-prose block is left-aligned; update the JSX
where Section is rendered (the Section component in
apps/website/src/pages/join-us/[slug].tsx) to include the className "max-w-prose
mx-auto" (or append "mx-auto" if className already exists) so the prose block is
centered on wide viewports.
In `@apps/website/src/pages/programs.tsx`:
- Around line 29-31: The wrapper div around ProgramsList currently uses the
utility class "max-w-prose", which is too narrow for structured rows; update
that div's class to use "max-w-text" instead of "max-w-prose" so ProgramsList
(rendering PageListGroup/PageListRow) has the appropriate 840px max width for
desktop layouts.
---
Outside diff comments:
In `@apps/website/src/components/incubator-week/WhatThisIsForSection.tsx`:
- Around line 63-67: The "About you" h3 is outside the max-width prose wrapper
causing horizontal misalignment; wrap that heading in a container with the same
prose constraints (e.g., a div with classes "max-w-prose mx-auto") or move the
h3 into the existing prose wrapper so it uses the same width as the other
headings — update the element that renders the About you heading (the h3 using
pageSectionHeadingClass) so it is placed inside (or wrapped by) a max-w-prose
mx-auto wrapper while leaving the following grid (the div with "pt-2 grid gap-8
grid-cols-1 bd-md:grid-cols-3") full-width.
---
Nitpick comments:
In `@apps/website/CLAUDE.md`:
- Around line 120-121: Clarify the apparent contradiction between the general
"avoid `max-width`/`max-height`" rule and the specific `max-w-prose` guidance by
adding a bridging sentence that explicitly states `max-w-prose` is an
intentional exception for body-text (prose) columns; update the CLAUDE.md
bullets so the "avoid `max-width`/`max-height`" rule remains general while a
follow-up line (referencing `max-w-prose`, `max-w-text`, and
`max-w-section-wide`) explains these allowed exceptions and when to use each
(prose vs cards/grids/sections).
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 05099864-d7f7-4e97-b442-757fcfdd7d73
⛔ Files ignored due to path filters (6)
apps/website/src/__tests__/pages/__snapshots__/about.test.tsx.snapis excluded by!**/*.snapapps/website/src/__tests__/pages/__snapshots__/join-us.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/about/__snapshots__/BeliefsSection.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/about/__snapshots__/IntroSection.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/admin/__snapshots__/ImpersonationBadge.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/lander/__snapshots__/AgiStrategyLander.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (31)
apps/website/CLAUDE.mdapps/website/src/components/about/BeliefsSection.tsxapps/website/src/components/about/IntroSection.tsxapps/website/src/components/about/ValuesSection.tsxapps/website/src/components/advising/WhatThisIsForSection.tsxapps/website/src/components/advising/WhatToExpectSection.tsxapps/website/src/components/advising/WhoYouAreSection.tsxapps/website/src/components/career-transition-grant/ExpectationsSection.tsxapps/website/src/components/career-transition-grant/WhatThisIsForSection.tsxapps/website/src/components/incubator-week/AboutBlueDotSection.tsxapps/website/src/components/incubator-week/LogisticsSection.tsxapps/website/src/components/incubator-week/TheWeekSection.tsxapps/website/src/components/incubator-week/TrackRecordSection.tsxapps/website/src/components/incubator-week/WhatThisIsForSection.tsxapps/website/src/components/join-us/JobsListSection.tsxapps/website/src/components/join-us/WhyUsSection.tsxapps/website/src/components/lander/components/CourseBenefitsTextSection.tsxapps/website/src/components/lander/components/FieldBuildingSection.tsxapps/website/src/components/lander/components/HowTheCourseWorksSection.tsxapps/website/src/components/lander/components/QuoteSection.tsxapps/website/src/components/lander/components/WhoIsThisForTextSection.tsxapps/website/src/components/rapid-grants/FundedProjectsSection.tsxapps/website/src/components/rapid-grants/WhatThisIsForSection.tsxapps/website/src/pages/attendance-policy.tsxapps/website/src/pages/code-of-conduct.tsxapps/website/src/pages/join-us/[slug].tsxapps/website/src/pages/missions/[slug].tsxapps/website/src/pages/our-community.tsxapps/website/src/pages/privacy-policy.tsxapps/website/src/pages/programs.tsxapps/website/src/pages/resources.tsx
| @@ -27,23 +27,27 @@ const JobsListSection = ({ jobs }: { jobs: JobsListSectionProps }) => { | |||
| <> | |||
| <section className="section section-body"> | |||
| <div id="open-roles-anchor" className="invisible relative bottom-48" /> | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check sticky nav height and section-body padding-top to validate the bottom-48 (12rem / 192px) anchor offset.
# Find the nav/header component to inspect its height classes
rg -n "sticky|fixed" --type=tsx -C 2 apps/website/src/components/ 2>/dev/null | grep -i "nav\|header\|top-0" | head -40
# Find section-body CSS definition
rg -n "section-body" apps/website/src/ --type=css -A 5 2>/dev/null
rg -n "section-body" apps/website/src/ --type=tsx -C 2 2>/dev/null | head -30
# Check globals.css for section-body padding
fd -e css -e scss . apps/website/src/ --exec grep -n "section-body" {} \;Repository: bluedotimpact/bluedot
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Read the JobsListSection file to understand the context
cat -n apps/website/src/components/join-us/JobsListSection.tsx | head -60
# Search for nav/header components more broadly
find apps/website/src/components -name "*nav*" -o -name "*header*" -o -name "*Nav*" -o -name "*Header*" 2>/dev/null | head -20
# Look for CSS files with section definitions
fd -e css -e scss apps/website/src/ 2>/dev/null | head -20Repository: bluedotimpact/bluedot
Length of output: 3326
🏁 Script executed:
#!/bin/bash
# Read Nav.tsx to see the sticky nav height classes
cat -n apps/website/src/components/Nav/Nav.tsx | head -80
# Find and read CSS files that define section and section-body
find apps/website/src -name "globals.css" -o -name "*.css" -o -name "*.scss" 2>/dev/null | head -10
# Search for section-body class definition in CSS/SCSS
rg "section-body" apps/website/src --type=css --type=scss -B 2 -A 5 2>/dev/nullRepository: bluedotimpact/bluedot
Length of output: 3302
🏁 Script executed:
#!/bin/bash
# Read globals.css to find nav height and section-body CSS variables
cat -n apps/website/src/globals.css | head -150
# Also search for any CSS definitions of section-body and section classes
rg "section|nav-height" apps/website/src/globals.css -B 1 -A 3Repository: bluedotimpact/bluedot
Length of output: 6642
🏁 Script executed:
#!/bin/bash
# Search for section-body and section class definitions
rg "section-body|section-base|\.section" apps/website/src --type=css --type=scss -B 2 -A 5 2>/dev/null | head -100
# Also check for Tailwind arbitrary section classes
rg "section" apps/website/tailwind.config.ts -B 3 -A 3 2>/dev/nullRepository: bluedotimpact/bluedot
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Find all CSS and config files in apps/website
find apps/website -name "*.css" -o -name "*.scss" -o -name "tailwind.config*" 2>/dev/null
# Try a broader search for section class definitions
rg "section" apps/website --type=tsx -B 1 -A 1 2>/dev/null | grep -i "classname.*section" | head -20
# Check if section-body is defined as a Tailwind utility or custom CSS
rg "section-body\|section-base" apps/website/src -B 2 -A 3 --type=tsx 2>/dev/null | head -50Repository: bluedotimpact/bluedot
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Search for section-body usage in TSX files to understand what it does
rg "section-body|section-base" apps/website/src --type=tsx -B 2 -A 2 2>/dev/null | head -60
# Check tailwind.config files at root and apps level
find . -name "tailwind.config*" -type f 2>/dev/null | grep -E "(root|apps)" | head -5
# Read the join-us page to see the full context of JobsListSection usage
rg "JobsListSection" apps/website/src --type=tsx -B 5 -A 5 2>/dev/null | head -60Repository: bluedotimpact/bluedot
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Read the tailwind config to see if section is defined there
find . -maxdepth 3 -name "tailwind.config*" -type f 2>/dev/null
# Check libraries/ui for section-body definition
rg "section-body" . --type=tsx --type=css 2>/dev/null | head -20
# Search for any references to section class in apps/website
grep -r "section-body\|section-base" apps/website/src --include="*.tsx" --include="*.css" 2>/dev/null | head -30
# Look at the join-us page directory for context
ls -la apps/website/src/pages/join-us/ 2>/dev/null
cat -n apps/website/src/pages/join-us/index.tsx 2>/dev/null | head -80Repository: bluedotimpact/bluedot
Length of output: 6097
🏁 Script executed:
#!/bin/bash
# Search for section and section-body class definitions in the libraries/ui folder
rg "section-body|\.section" libraries/ui/src --type=css --type=scss -B 2 -A 5 2>/dev/null | head -100
# Also check if they're defined in a component
rg "section-body|\.section" libraries/ui/src --type=tsx -B 2 -A 3 2>/dev/null | grep -i "classname\|css" | head -30
# Search the entire apps/website for any CSS that defines these classes
find apps/website -type f \( -name "*.css" -o -name "*.scss" \) -exec grep -l "section" {} \; 2>/dev/nullRepository: bluedotimpact/bluedot
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Check the entire globals.css file to see all content
wc -l apps/website/src/globals.css
# Read the complete globals.css
cat apps/website/src/globals.css
# Check if section/section-body are Tailwind `@apply` classes
grep -n "@apply\|section" apps/website/src/globals.cssRepository: bluedotimpact/bluedot
Length of output: 5849
🏁 Script executed:
#!/bin/bash
# Check the UI library's tailwind config for section definitions
find libraries/ui -name "tailwind.css" -o -name "tailwind.config*" 2>/dev/null
# Look for section in the UI library default config
cat libraries/ui/src/default-config/tailwind.css 2>/dev/null | grep -A 10 "section\|@apply" | head -100
# Also check if section is a standard Tailwind class or custom in UI lib
rg "section\|section-body" libraries/ui/src --type=css -B 2 -A 5 2>/dev/null | head -80Repository: bluedotimpact/bluedot
Length of output: 3603
🏁 Script executed:
#!/bin/bash
# Verify the px-spacing-x value used by section-body
rg "spacing-x|--spacing-x" libraries/ui/src/default-config/tailwind.css -B 2 -A 2Repository: bluedotimpact/bluedot
Length of output: 1910
Confirm the bottom-48 (192 px) scroll offset is intentional.
The invisible anchor uses position: relative; bottom: 12rem to shift its rendered position 192 px above its natural layout position. When the browser scrolls to #open-roles-anchor, it positions that visual point at the viewport top, creating 192 px of clearance for the sticky nav (60–76 px on mobile/desktop) and additional scroll context.
This offset is unusually large—more than double the sticky nav height. Confirm this is intentional by checking against the design or scroll behaviour requirements. If not, consider reducing it to align with the nav height plus a modest buffer (e.g., bottom-24 or bottom-32 for 96–128 px total clearance).
🤖 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 `@apps/website/src/components/join-us/JobsListSection.tsx` at line 29, The
invisible anchor div with id "open-roles-anchor" in JobsListSection.tsx
currently uses the utility class "bottom-48" which shifts the anchor 192px
upward; verify this large offset against the design and, if not intentional,
reduce it to match the sticky nav height plus a small buffer (e.g., replace
"bottom-48" with "bottom-24" or "bottom-32") so the scroll target aligns
correctly under the sticky nav; update the class on the <div
id="open-roles-anchor"> element accordingly.
| /> | ||
| <Breadcrumbs route={currentRoute} /> | ||
| <Section className="max-w-3xl"> | ||
| <Section className="max-w-prose"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the Section component from `@bluedot/ui` to check whether mx-auto is applied by default.
rg -n "mx-auto\|margin.*auto\|Section" --type tsx -C 3 packages/ui/src/components/Section.tsx 2>/dev/null \
|| fd -i "Section.tsx" --exec cat {}Repository: bluedotimpact/bluedot
Length of output: 50378
Add mx-auto to centre the prose block – currently left-aligned without it.
The Section component from @bluedot/ui does not apply mx-auto internally. Without it, the constrained width will hug the left edge rather than centre on wide viewports. This violates the guideline requiring prose sections to use max-w-prose mx-auto.
Proposed fix
- <Section className="max-w-prose">
+ <Section className="max-w-prose mx-auto">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Section className="max-w-prose"> | |
| <Section className="max-w-prose mx-auto"> |
🤖 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 `@apps/website/src/pages/join-us/`[slug].tsx at line 96, The Section component
on the join-us page is missing the mx-auto utility so the constrained
max-w-prose block is left-aligned; update the JSX where Section is rendered (the
Section component in apps/website/src/pages/join-us/[slug].tsx) to include the
className "max-w-prose mx-auto" (or append "mx-auto" if className already
exists) so the prose block is centered on wide viewports.
| <div className="max-w-prose mx-auto w-full flex flex-col gap-12 lg:gap-14"> | ||
| <ProgramsList /> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect ProgramsList to determine if it renders prose or cards/grids.
fd -i "ProgramsList.tsx" --exec cat {}Repository: bluedotimpact/bluedot
Length of output: 2152
Use max-w-text instead of max-w-prose for the ProgramsList wrapper.
ProgramsList renders a structured list of program rows (via PageListGroup and PageListRow), not prose content. Per the coding guidelines, structured layouts require max-w-text (840px) or max-w-section-wide (1100px). Constraining it to max-w-prose (~65 characters) will produce unnecessarily cramped rows on desktop.
🤖 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 `@apps/website/src/pages/programs.tsx` around lines 29 - 31, The wrapper div
around ProgramsList currently uses the utility class "max-w-prose", which is too
narrow for structured rows; update that div's class to use "max-w-text" instead
of "max-w-prose" so ProgramsList (rendering PageListGroup/PageListRow) has the
appropriate 840px max width for desktop layouts.
|
@claude review |
|
Claude finished @dewierwan's task —— View job Code Review for Prose Width CappingI've completed my review of the prose width capping implementation. Here's my comprehensive analysis: Todo List
Review SummaryOverall Assessment: ✅ Excellent implementation with a few minor fixes needed. The changes are clean, consistent, and well-executed. The implementation properly addresses the readability concerns mentioned in the PR description while following established patterns. Positive Aspects
Issues Found (from CodeRabbit analysis)Critical Issues:
Minor Issues:
Strategic Direction: 65ch vs 840px DebateMy recommendation: Stay with 65ch approach for the following reasons:
Recommendations
Technical Details
The PR demonstrates excellent technical execution and addresses a real accessibility concern. After fixing the 4 identified issues, this will be ready for merge. |
dewierwan
changed the title
@dewierwan on the counter-proposal: IMO the main issue with this is that it changes things across the site, which practically means we need to audit every place that is affected and check for any visual issues. The difference between There are ~20 pages in principle affected. I just spot checked a few and did find some new issues of mixed-widths (this is On the CLAUDE.md point: I think this is probably a general best practice added by Claude rather than a long-term rule we endorse. And per the above the difference between max-w-text and max-w-prose is small, so I would suggest removing that from CLAUDE.md. Still happy to jump in if you want! |
2 participants
Summary
Body prose was running 95–126 chars per line on wide viewports across the public marketing site (lander pages, policies, missions, program/grant pages), past WCAG 1.4.8's 80ch ceiling and the Baymard 50–75ch sweet spot. This PR unifies all body prose at
max-w-prose(~65ch / ~580px @ 16px body) — the team's documented standard inapps/website/CLAUDE.mdand the readability optimum.Per Will's feedback on the original About+Join Us scope: rather than widening About/Join Us to
max-w-text(840px ≈ ~105ch, off-spec for body prose), this expands the sweep so the whole site lands at one readable width. Card/grid sections (Schedule, Pathways, CourseInformation, History timeline, Team grid, etc.) intentionally stay onmax-w-text/max-w-section-wide— they're structured layouts, not prose.Scope
About + Join Us —
max-w-prosewrapper:about/IntroSection,BeliefsSection,ValuesSectionjoin-us/WhyUsSection,JobsListSectionCourseLander prose components (
max-w-text→max-w-prose):WhoIsThisForTextSection,HowTheCourseWorksSection,CourseBenefitsTextSection,FieldBuildingSection,QuoteSectionProgram/grant page sections:
AboutBlueDotSection,TrackRecordSection,LogisticsSection,TheWeekSection,WhatThisIsForSectionWhatThisIsForSection,WhatToExpectSection,WhoYouAreSectionExpectationsSection,WhatThisIsForSectionWhatThisIsForSection,FundedProjectsSection(GranteesListSection wrap)pages/programs.tsx: ProgramsList wrappedMarkdown wrapper pages (
max-w-3xl→max-w-prose):privacy-policy,code-of-conduct,attendance-policy,our-community,resources,missions/[slug],join-us/[slug]Documentation:
apps/website/CLAUDE.md: tightened the prose convention to distinguish prose (max-w-prose) from structured layouts (max-w-text/max-w-section-wide).Verified
1chwidth (getBoundingClientRecton a hidden<span>0</span>injected into each<p>).npm testpasses (103/103 test files, 620 tests).CourseInformationSectioncard has a label-description row at ~83ch (label column 160px + description column 680px inside an 840px card). Treating as exempt under the "table cells / form labels" carve-out — it's structured data, not flowing prose.Screenshots
/about/join-us/privacy-policy/programs/incubator-week/programs/advisingOut of scope
UnitLayout.tsx— bespoke responsive widths tied to sidebar/video).max-w-modal(600px) is fine.max-w-narrowcallouts — intentional design choice.max-w-texttoken — separate cleanup; just stop using it for prose.Test plan
🤖 Generated with Claude Code