Skip to content

Center Bible card content#243

Open
cameronapak wants to merge 3 commits into
mainfrom
codex/ype-2573-card-content-width
Open

Center Bible card content#243
cameronapak wants to merge 3 commits into
mainfrom
codex/ype-2573-card-content-width

Conversation

@cameronapak
Copy link
Copy Markdown
Collaborator

@cameronapak cameronapak commented May 21, 2026

Summary

  • remove fixed max-width behavior from BibleCard while centering its 600px content group
  • apply the same centered content treatment to VerseOfTheDay
  • add Storybook wide-container coverage and align default stories with full-width usage

Ticket

https://lifechurch.atlassian.net/browse/YPE-2573

BibleCard centered content group VerseOfTheDay centered content group

Test plan

  • pnpm --filter @youversion/platform-react-ui test -- bible-card.test.tsx verse-of-the-day.test.tsx
  • pnpm --filter @youversion/platform-react-ui test:integration -- bible-card.stories.tsx
  • pnpm --filter @youversion/platform-react-ui test:integration -- bible-card.stories.tsx verse-of-the-day.stories.tsx

Greptile Summary

This PR removes the fixed max-width from BibleCard and VerseOfTheDay and 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.

  • BibleCard and VerseOfTheDay: yv:max-w-md is dropped from the root <section>; all inner content is wrapped in a centered 600px <div> using maxWidth: 600 and marginInline: auto.
  • bible-reader.css: max-width: 65ch is removed from [data-slot='yv-bible-renderer'], shifting the readability constraint from the text-renderer primitive up to the card wrappers.
  • Tests and stories: New WideContainer integration stories use getBoundingClientRect assertions 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

Filename Overview
packages/core/src/styles/bible-reader.css Removes the max-width: 65ch readability guardrail globally, affecting all consumers of the public BibleTextView component, not just BibleCard and VerseOfTheDay.
packages/ui/src/components/bible-card.tsx Removes yv:max-w-md from the root section and wraps all card content in a centered 600px div; uses inline styles for layout constraints instead of Tailwind utilities.
packages/ui/src/components/verse-of-the-day.tsx Mirrors the BibleCard layout change, removing yv:max-w-md and centering content in a 600px wrapper div with inline styles.
packages/ui/src/components/verse.test.tsx Adds two tests checking inline styles that were always empty, providing no regression coverage for the CSS class change in bible-reader.css.
.changeset/pretty-bibles-sit.md Marks a minor release but does not mention the removal of max-width: 65ch from the public BibleTextView CSS.

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
    end
Loading

Fix All in Claude Code Fix All in Cursor

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
packages/core/src/styles/bible-reader.css:5-7
**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.

### Issue 2 of 3
packages/ui/src/components/verse.test.tsx:639-666
**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.

### Issue 3 of 3
packages/ui/src/components/bible-card.tsx:163-166
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">
```

Reviews (1): Last reviewed commit: "feat(ui): add card layout changeset" | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

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-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 21, 2026

🦋 Changeset detected

Latest commit: 17b6144

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@youversion/platform-react-ui Minor
vite-react Patch
@youversion/platform-core Minor
@youversion/platform-react-hooks Minor

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.
@cameronapak cameronapak marked this pull request as ready for review May 21, 2026 17:35
Comment on lines 5 to 7
display: block;

width: 100%;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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.

Fix in Claude Code Fix in Cursor

Comment on lines +639 to +666
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('');
});
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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.

Fix in Claude Code Fix in Cursor

Comment on lines +163 to +166
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' }}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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.

Suggested change
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!

Fix in Claude Code Fix in Cursor

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