Center Bible card content#243
Conversation
Remove the outer BibleCard max width while keeping the rendered card content centered at a readable line length. This lets host layouts size the card while preserving balanced whitespace inside wide containers.
Center the VerseOfTheDay content within a full-width card and update Storybook framing for realistic width behavior. Keep card shells border-box so full-width cards fit narrow containers without adding minimum widths.
🦋 Changeset detectedLatest commit: 17b6144 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Add a minor changeset for the card layout update so the UI package version captures the new full-width centered card behavior.
| display: block; | ||
|
|
||
| width: 100%; |
There was a problem hiding this comment.
Removal of
max-width: 65ch affects all standalone BibleTextView consumers
BibleTextView is a public export (packages/ui/src/components/index.ts). Removing max-width: 65ch from the [data-slot='yv-bible-renderer'] CSS rule changes its rendered width for any consumer using BibleTextView directly — not wrapped in a BibleCard or VerseOfTheDay. Those callers previously got a readability constraint "for free"; they'll now render unbounded unless they add their own wrapper. The changeset description only mentions the card-centering behavior and doesn't call out this broader behavioral change to the low-level component.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/styles/bible-reader.css
Line: 5-7
Comment:
**Removal of `max-width: 65ch` affects all standalone `BibleTextView` consumers**
`BibleTextView` is a public export (`packages/ui/src/components/index.ts`). Removing `max-width: 65ch` from the `[data-slot='yv-bible-renderer']` CSS rule changes its rendered width for any consumer using `BibleTextView` directly — not wrapped in a `BibleCard` or `VerseOfTheDay`. Those callers previously got a readability constraint "for free"; they'll now render unbounded unless they add their own wrapper. The changeset description only mentions the card-centering behavior and doesn't call out this broader behavioral change to the low-level component.
How can I resolve this? If you propose a fix, please make it concise.| it('should not set layout width on BibleTextView', async () => { | ||
| const { container } = render( | ||
| <BibleTextView | ||
| reference="JHN.3.16" | ||
| versionId={3034} | ||
| passageState={{ | ||
| passage: mockPassage, | ||
| loading: false, | ||
| error: null, | ||
| }} | ||
| />, | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| const wrapper = container.querySelector('[data-yv-sdk]'); | ||
| expect((wrapper as HTMLElement).style.width).toBe(''); | ||
| expect((wrapper as HTMLElement).style.maxWidth).toBe(''); | ||
| }); | ||
| }); | ||
|
|
||
| it('should leave Verse.Html without a text width class', async () => { | ||
| const { container } = render(<Verse.Html html={mockPassage.content} />); | ||
|
|
||
| await waitFor(() => { | ||
| const section = container.querySelector('section[data-slot="yv-bible-renderer"]'); | ||
| expect((section as HTMLElement).style.maxWidth).toBe(''); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
New tests check inline styles, not CSS-class-based
max-width
element.style.maxWidth in JSDOM only reflects inline styles, not rules applied via a stylesheet. The max-width: 65ch that was removed from bible-reader.css was a CSS class rule, so style.maxWidth was already '' before this PR. Both new tests were trivially passing before any code changed and won't catch a future regression of the CSS removal.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/components/verse.test.tsx
Line: 639-666
Comment:
**New tests check inline styles, not CSS-class-based `max-width`**
`element.style.maxWidth` in JSDOM only reflects inline styles, not rules applied via a stylesheet. The `max-width: 65ch` that was removed from `bible-reader.css` was a CSS class rule, so `style.maxWidth` was already `''` before this PR. Both new tests were trivially passing before any code changed and won't catch a future regression of the CSS removal.
How can I resolve this? If you propose a fix, please make it concise.| className="yv:w-full yv:flex yv:flex-col yv:grow yv:bg-card yv:p-6 yv:rounded-2xl" | ||
| style={{ boxSizing: 'border-box' }} | ||
| > | ||
| <div className="yv:flex yv:w-full yv:justify-between yv:items-center yv:mb-4"> | ||
| {passage && !passageError ? ( | ||
| <div className="yv:grow yv:flex yv:items-center yv:gap-1.5"> | ||
| <BibleCardHeaderReference passage={passage} version={version} /> | ||
| {showSpinner ? ( | ||
| <LoaderIcon className="yv:size-3 yv:animate-spin yv:text-muted-foreground" /> | ||
| ) : null} | ||
| </div> | ||
| ) : passageError ? ( | ||
| <BibleCardHeaderError /> | ||
| ) : ( | ||
| <LoaderIcon className="yv:size-3 yv:animate-spin yv:text-muted-foreground" /> | ||
| )} | ||
|
|
||
| {showVersionPicker && !passageError ? ( | ||
| <BibleCardVersionPicker | ||
| versionId={versionNum} | ||
| onVersionChange={setVersionNum} | ||
| <div style={{ width: '100%', maxWidth: 600, marginInline: 'auto' }}> |
There was a problem hiding this comment.
Inline styles for layout constraints (
maxWidth, marginInline, boxSizing) are inconsistent with the Tailwind-first approach used throughout the rest of this component. The equivalent utilities yv:max-w-[600px], yv:mx-auto, yv:w-full, and yv:box-border would keep the styling layer uniform.
| className="yv:w-full yv:flex yv:flex-col yv:grow yv:bg-card yv:p-6 yv:rounded-2xl" | |
| style={{ boxSizing: 'border-box' }} | |
| > | |
| <div className="yv:flex yv:w-full yv:justify-between yv:items-center yv:mb-4"> | |
| {passage && !passageError ? ( | |
| <div className="yv:grow yv:flex yv:items-center yv:gap-1.5"> | |
| <BibleCardHeaderReference passage={passage} version={version} /> | |
| {showSpinner ? ( | |
| <LoaderIcon className="yv:size-3 yv:animate-spin yv:text-muted-foreground" /> | |
| ) : null} | |
| </div> | |
| ) : passageError ? ( | |
| <BibleCardHeaderError /> | |
| ) : ( | |
| <LoaderIcon className="yv:size-3 yv:animate-spin yv:text-muted-foreground" /> | |
| )} | |
| {showVersionPicker && !passageError ? ( | |
| <BibleCardVersionPicker | |
| versionId={versionNum} | |
| onVersionChange={setVersionNum} | |
| <div style={{ width: '100%', maxWidth: 600, marginInline: 'auto' }}> | |
| className="yv:w-full yv:flex yv:flex-col yv:grow yv:bg-card yv:p-6 yv:rounded-2xl yv:box-border" | |
| > | |
| <div className="yv:w-full yv:max-w-[600px] yv:mx-auto"> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/components/bible-card.tsx
Line: 163-166
Comment:
Inline styles for layout constraints (`maxWidth`, `marginInline`, `boxSizing`) are inconsistent with the Tailwind-first approach used throughout the rest of this component. The equivalent utilities `yv:max-w-[600px]`, `yv:mx-auto`, `yv:w-full`, and `yv:box-border` would keep the styling layer uniform.
```suggestion
className="yv:w-full yv:flex yv:flex-col yv:grow yv:bg-card yv:p-6 yv:rounded-2xl yv:box-border"
>
<div className="yv:w-full yv:max-w-[600px] yv:mx-auto">
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Ticket
https://lifechurch.atlassian.net/browse/YPE-2573
Test plan
Greptile Summary
This PR removes the fixed
max-widthfromBibleCardandVerseOfTheDayand instead lets both components fill their containers while centering a 600px content group internally. Wide-container Storybook stories and unit tests are added to verify the new layout geometry.BibleCardandVerseOfTheDay:yv:max-w-mdis dropped from the root<section>; all inner content is wrapped in a centered 600px<div>usingmaxWidth: 600andmarginInline: auto.bible-reader.css:max-width: 65chis removed from[data-slot='yv-bible-renderer'], shifting the readability constraint from the text-renderer primitive up to the card wrappers.WideContainerintegration stories usegetBoundingClientRectassertions to confirm centering; new unit tests verify the DOM shape via inline-style checks.Confidence Score: 4/5
Safe to merge for the card components; the bible-reader.css change has a wider blast radius than the PR description suggests.
The card layout refactor is clean and well-tested. The removal of max-width: 65ch from bible-reader.css affects every consumer of the public BibleTextView component, not only BibleCard and VerseOfTheDay. Callers embedding BibleTextView standalone will now render text unbounded unless they add their own constraint, and the changeset does not mention this.
packages/core/src/styles/bible-reader.css deserves a changeset note about the BibleTextView behavioral change; packages/ui/src/components/verse.test.tsx new tests do not guard against CSS regressions.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD subgraph Before A1["section data-yv-sdk\nyv:max-w-md"] A2["section data-slot=yv-bible-renderer\nmax-width: 65ch CSS class"] A1 --> A2 end subgraph After B1["section data-yv-sdk\nyv:w-full fills container"] B2["div maxWidth 600px marginInline auto\n600px centered content group"] B3["section data-slot=yv-bible-renderer\nno max-width fills content group"] B1 --> B2 B2 --> B3 endPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(ui): add card layout changeset" | Re-trigger Greptile