Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pretty-bibles-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@youversion/platform-react-ui": minor
---

Center card content within full-width BibleCard and VerseOfTheDay layouts.
2 changes: 0 additions & 2 deletions packages/core/src/styles/bible-reader.css
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
display: block;

width: 100%;
Comment on lines 5 to 7
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

/* This helps readability and is a standard max width for text */
max-width: 65ch;

--yv-reader-font-size: 20px;
--yv-reader-line-height: 1.625;
Expand Down
48 changes: 46 additions & 2 deletions packages/ui/src/components/bible-card.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ const meta = {
title: 'Components/BibleCard',
component: BibleCard,
parameters: {
layout: 'centered',
layout: 'fullscreen',
},
render: (args) => (
<div className="yv:h-screen yv:w-screen yv:flex yv:justify-center yv:items-center">
<div className="yv:w-full">
<BibleCard {...args} />
</div>
),
Expand Down Expand Up @@ -44,6 +44,50 @@ export const Default: Story = {
versionId: 111,
},
};

export const WideContainer: Story = {
args: {
reference: 'LUK.1.39-45',
versionId: 111,
},
tags: ['integration'],
parameters: {
layout: 'fullscreen',
},
render: (args) => (
<div className="yv:p-8" style={{ width: 900 }}>
<BibleCard {...args} />
</div>
),
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

await waitFor(async () => {
await expect(canvas.getByText(/at that time mary got ready/i)).toBeInTheDocument();
});

const card = canvasElement.querySelector('section[data-yv-sdk][data-yv-theme]');
const contentGroup = canvasElement.querySelector('section[data-yv-sdk][data-yv-theme] > div');
const bibleText = canvasElement.querySelector('[data-slot="yv-bible-renderer"]');

await expect(card).not.toBeNull();
await expect(contentGroup).not.toBeNull();
await expect(bibleText).not.toBeNull();

const cardWidth = card?.getBoundingClientRect().width ?? 0;
const contentGroupRect = contentGroup?.getBoundingClientRect();
const cardRect = card?.getBoundingClientRect();
const leftWhitespace = (contentGroupRect?.left ?? 0) - (cardRect?.left ?? 0);
const rightWhitespace = (cardRect?.right ?? 0) - (contentGroupRect?.right ?? 0);
const bibleTextWidth = bibleText?.getBoundingClientRect().width ?? 0;

await expect(cardWidth).toBeGreaterThan(800);
await expect(contentGroupRect?.width ?? 0).toBeLessThanOrEqual(600);
await expect(bibleTextWidth).toBeLessThanOrEqual(600);
await expect(Math.abs(leftWhitespace - rightWhitespace)).toBeLessThanOrEqual(1);
},
};

export const DarkMode: Story = {
args: {
reference: 'LUK.1.39-45',
Expand Down
21 changes: 21 additions & 0 deletions packages/ui/src/components/bible-card.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,25 @@ describe('BibleCard - Delayed spinner', () => {
0,
);
});

it('should let the card fill its container while centering the content group', () => {
vi.mocked(usePassage).mockReturnValue({
passage: mockPassage,
loading: false,
error: null,
refetch: vi.fn(),
});

const { container } = render(<BibleCard reference="JHN.3.16" versionId={3034} />);
const card = container.querySelector('section');
const contentGroup = container.querySelector('section > div[style*="max-width"]');
const bibleTextView = container.querySelector('[data-slot="yv-bible-renderer"]')?.parentElement;

expect(card).toHaveClass('yv:w-full');
expect(card).not.toHaveClass('yv:max-w-md');
expect(card!.style.boxSizing).toBe('border-box');
expect((contentGroup as HTMLElement).style.maxWidth).toBe('600px');
expect((contentGroup as HTMLElement).style.marginInline).toBe('auto');
expect(bibleTextView).not.toHaveClass('yv:max-w-[600px]');
});
});
79 changes: 41 additions & 38 deletions packages/ui/src/components/bible-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,48 +160,51 @@ export function BibleCard({
<section
data-yv-sdk
data-yv-theme={theme}
className="yv:w-full yv:flex yv:flex-col yv:grow yv:bg-card yv:p-6 yv:max-w-md yv:rounded-2xl"
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' }}>
Comment on lines +163 to +166
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

<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}
theme={theme}
onVersionPickerPress={onVersionPickerPress}
/>
) : null}
</div>

<AnimatedHeight>
<BibleTextView
theme={theme}
onVersionPickerPress={onVersionPickerPress}
fontSize={16}
fontFamily={SOURCE_SERIF_FONT}
reference={reference}
versionId={versionNum}
passageState={{
passage,
loading: passageLoading,
error: passageError,
}}
/>
) : null}
</div>
</AnimatedHeight>

<AnimatedHeight>
<BibleTextView
theme={theme}
fontSize={16}
fontFamily={SOURCE_SERIF_FONT}
reference={reference}
versionId={versionNum}
passageState={{
passage,
loading: passageLoading,
error: passageError,
}}
/>
</AnimatedHeight>

<BibleCardFooter copyright={!passageError ? version?.copyright : null} />
<BibleCardFooter copyright={!passageError ? version?.copyright : null} />
</div>
</section>
);
}
51 changes: 50 additions & 1 deletion packages/ui/src/components/verse-of-the-day.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@ const meta = {
title: 'Components/VerseOfTheDay',
component: VerseOfTheDay,
parameters: {
layout: 'centered',
layout: 'fullscreen',
},
render: (args) => (
<div className="yv:w-full">
<VerseOfTheDay {...args} />
</div>
),
tags: ['autodocs'],
argTypes: {
background: {
Expand Down Expand Up @@ -85,6 +90,50 @@ export const Default: Story = {
},
};

export const WideContainer: Story = {
args: {
versionId: 111,
showSunIcon: true,
showBibleAppAttribution: true,
showShareButton: true,
size: 'default',
},
tags: ['integration'],
parameters: {
layout: 'fullscreen',
},
render: (args) => (
<div className="yv:p-8" style={{ width: 900 }}>
<VerseOfTheDay {...args} />
</div>
),
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

await expect(
await canvas.findByText(/for I am about to do something new/i),
).toBeInTheDocument();

const card = canvasElement.querySelector('section[data-yv-sdk][data-yv-theme]');
const contentGroup = canvasElement.querySelector('section[data-yv-sdk][data-yv-theme] > div');
const bibleText = canvasElement.querySelector('[data-slot="yv-bible-renderer"]');

await expect(card).not.toBeNull();
await expect(contentGroup).not.toBeNull();
await expect(bibleText).not.toBeNull();

const cardRect = card?.getBoundingClientRect();
const contentGroupRect = contentGroup?.getBoundingClientRect();
const leftWhitespace = (contentGroupRect?.left ?? 0) - (cardRect?.left ?? 0);
const rightWhitespace = (cardRect?.right ?? 0) - (contentGroupRect?.right ?? 0);

await expect(cardRect?.width ?? 0).toBeGreaterThan(800);
await expect(contentGroupRect?.width ?? 0).toBeLessThanOrEqual(600);
await expect(bibleText?.getBoundingClientRect().width ?? 0).toBeLessThanOrEqual(600);
await expect(Math.abs(leftWhitespace - rightWhitespace)).toBeLessThanOrEqual(1);
},
};

export const Large: Story = {
args: {
size: 'lg',
Expand Down
12 changes: 12 additions & 0 deletions packages/ui/src/components/verse-of-the-day.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,16 @@ describe('VerseOfTheDay i18n integration', () => {
render(<VerseOfTheDay />);
expect(screen.getByText(en.verseOfTheDay)).toBeInTheDocument();
});

it('lets the card fill its container while centering the content group', () => {
const { container } = render(<VerseOfTheDay />);
const card = container.querySelector('section');
const contentGroup = container.querySelector('section > div[style*="max-width"]');

expect(card).toHaveClass('yv:w-full');
expect(card).not.toHaveClass('yv:max-w-md');
expect(card!.style.boxSizing).toBe('border-box');
expect((contentGroup as HTMLElement).style.maxWidth).toBe('600px');
expect((contentGroup as HTMLElement).style.marginInline).toBe('auto');
});
});
Loading
Loading