Skip to content

Conversation

@cameronapak
Copy link
Collaborator

Here's a quick video overview.

CleanShot 2026-01-05 at 14 01 06@2x

As you can see in here, the Bible abbreviations and the years fit within the boundaries of the box and do not overflow.

@changeset-bot
Copy link

changeset-bot bot commented Jan 5, 2026

🦋 Changeset detected

Latest commit: 122fbff

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 Patch
@youversion/platform-core Patch
@youversion/platform-react-hooks Patch
nextjs Patch

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

@chatgpt-codex-connector
Copy link

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@cameronapak cameronapak self-assigned this Jan 5, 2026
@cameronapak cameronapak requested a review from bmanquen January 5, 2026 20:02
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 5, 2026

Greptile Summary

This PR fixes Bible version abbreviation text overflow by implementing dynamic font scaling. The solution extracts the inline abbreviation rendering logic into a dedicated VersionAbbreviationIcon component that uses ResizeObserver to automatically scale text to fit within the icon boundaries.

Key changes:

  • Created VersionAbbreviationIcon component with iterative font size calculation algorithm
  • Removed yv:p-1 padding from ItemMedia to maximize available space for text
  • Added yv:items-center class to center content in the icon box
  • Added STORYBOOK_AUTH_REDIRECT_URL environment variable to .env.example
  • Fixed trailing whitespace in .env.example comment
  • Added type="button" to language selector button

The implementation uses a 5-iteration convergence loop to calculate optimal font sizes for both the prefix and digit portions of version abbreviations, ensuring text fits within 70% of container width and 40% of container height.

Confidence Score: 4/5

  • This PR is safe to merge with minor considerations about initial render performance
  • The implementation correctly solves the overflow issue with a well-structured component. Score of 4 (instead of 5) due to: (1) ResizeObserver triggering multiple calculations on mount which could impact performance when rendering many versions, and (2) the iterative size calculation modifying DOM during measurement could be optimized by calculating initial sizes synchronously
  • No files require special attention - the changes are focused and well-implemented

Important Files Changed

Filename Overview
packages/ui/.env.example Added STORYBOOK_AUTH_REDIRECT_URL environment variable and fixed trailing whitespace in comment
packages/ui/src/components/bible-version-picker.tsx Extracted dynamic text sizing logic into VersionAbbreviationIcon component with ResizeObserver-based scaling to prevent overflow

Sequence Diagram

sequenceDiagram
    participant User
    participant VersionPicker as BibleVersionPicker
    participant VersionIcon as VersionAbbreviationIcon
    participant ResizeObserver
    participant DOM

    User->>VersionPicker: Opens picker
    VersionPicker->>DOM: Renders version list
    
    loop For each Bible version
        DOM->>VersionIcon: Mount component with abbreviation text
        VersionIcon->>VersionIcon: Parse text into prefix + digits
        VersionIcon->>ResizeObserver: Create observer on container
        ResizeObserver->>VersionIcon: Observe container element
        
        ResizeObserver->>VersionIcon: Trigger resize event
        VersionIcon->>VersionIcon: Calculate prefix size (5 iterations)
        VersionIcon->>DOM: Set fontSize on prefix element
        VersionIcon->>DOM: Measure scrollWidth & offsetHeight
        VersionIcon->>VersionIcon: Calculate digits size (5 iterations)
        VersionIcon->>DOM: Set fontSize on digits element
        VersionIcon->>DOM: Measure scrollWidth & offsetHeight
        VersionIcon->>VersionIcon: Update state with final sizes
        VersionIcon->>DOM: Re-render with calculated font sizes
    end
    
    User->>VersionPicker: Select version
    VersionPicker->>User: Close picker & update selection
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@cameronapak
Copy link
Collaborator Author

I imagine this will require a changeset update, and that's something I don't know how to do yet. We'll need to learn from Brenden

bmanquen
bmanquen previously approved these changes Jan 6, 2026
overflow

Adjust text scaling to account for container height, preventing
overflow. Also, ensure text does not wrap by adding
`yv:whitespace-nowrap` class.
Removes unused digitsRef and digitsSize state. The font size for digits
is now derived from prefixSize, ensuring consistent scaling.
@cameronapak cameronapak force-pushed the cp/YPE-1002-versions-fit-ui branch from 72d7c96 to 4459560 Compare January 6, 2026 17:28
@cameronapak
Copy link
Collaborator Author

Okay, so I ran through and made a small change where the font size of the text is just one font size and not two different font sizes.

And then the second thing that I did was create a change set.

And the third thing that I did was create or to rebase.

And that's it. This is ready to be merged upon your approval @bmanquen

@cameronapak cameronapak merged commit d0d596c into main Jan 6, 2026
5 checks passed
@cameronapak cameronapak deleted the cp/YPE-1002-versions-fit-ui branch January 6, 2026 19:25
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.

3 participants