feat(ui, hooks): add sign in/out to the bible reader

[bmanquen]

• add ability to sign in/out in the bible reader component
• refactor the auth in the hooks package so that the user doesn't
  have to pass a redirectUri. We are now able to get a redirectUri
  from the provider.
• add new icons for the settings and sign in/out feature in the
  reader.

Light mode:

Dark mode:

[changeset-bot]

🦋 Changeset detected

Latest commit: b57d5c4

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-hooks	Minor
@youversion/platform-core	Minor
@youversion/platform-react-ui	Minor
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]

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.

[greptile-apps]

Greptile Summary

This PR adds sign in/out functionality to the bible reader component and refactors the authentication hooks to make redirectUri optional by allowing it to be configured at the provider level.

Key Changes:

• Refactored useYVAuth.signIn() to accept optional redirectUrl parameter, falling back to provider's redirectUri if not provided
• Added user menu to bible reader toolbar with avatar display for authenticated users and person icon for unauthenticated users
• Implemented sign in/out popover UI integrated with useYVAuth hook
• Created new GearIcon and PersonIcon components for the toolbar
• Added showHeader prop to Popover component to support headerless popovers
• Increased default button SVG size from 4 to 6 for better visibility
• Added comprehensive tests for auth flows in both unit tests and Storybook stories
• Created test utility setupAuthenticatedUser for mocking authenticated state in stories

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The changes are well-structured with comprehensive test coverage including unit tests and Storybook integration tests. The auth refactoring maintains backward compatibility by making redirectUrl optional while providing a clear error message when neither source is available. The UI changes are isolated to the bible reader component and properly integrate with existing patterns.
• No files require special attention

Important Files Changed

Filename	Overview
packages/hooks/src/useYVAuth.ts	Refactored signIn to make redirectUrl optional, falling back to provider's redirectUri. Added redirectUri to return value and tests verify both paths work correctly.
packages/hooks/src/useYVAuth.test.tsx	Added comprehensive tests for the new optional redirectUrl parameter, verifying signIn works both with explicit URL and provider fallback.
packages/ui/src/components/bible-reader.tsx	Added user authentication UI to toolbar with avatar display, sign in/out popover, and settings gear icon. Grid layout adjusted to accommodate new elements.
packages/ui/src/components/ui/popover.tsx	Added optional showHeader prop to conditionally render popover header, supporting headerless popovers like the sign in/out menu.

Sequence Diagram

sequenceDiagram
    participant User
    participant BibleReader
    participant useYVAuth
    participant AuthProvider
    participant YouVersionAPI

    Note over User,YouVersionAPI: Sign In Flow
    User->>BibleReader: Click person icon
    BibleReader->>BibleReader: Open popover
    User->>BibleReader: Click Sign In button
    BibleReader->>useYVAuth: Call signIn with no parameters
    useYVAuth->>useYVAuth: Check if redirectUrl in params
    alt redirectUrl provided
        useYVAuth->>YouVersionAPI: signIn with provided value
    else no redirectUrl
        useYVAuth->>AuthProvider: Get redirectUri from context
        alt redirectUri exists
            useYVAuth->>YouVersionAPI: signIn with context value
        else no redirectUri
            useYVAuth-->>BibleReader: Throw error
        end
    end
    YouVersionAPI-->>User: Redirect to OAuth flow
    
    Note over User,YouVersionAPI: Post-Authentication
    User->>AuthProvider: Return from OAuth
    AuthProvider->>YouVersionAPI: handleAuthCallback
    YouVersionAPI-->>AuthProvider: Return user data
    AuthProvider->>AuthProvider: Update userInfo state
    BibleReader->>useYVAuth: Get auth state
    useYVAuth->>AuthProvider: Get userInfo from context
    useYVAuth-->>BibleReader: Return auth and userInfo
    BibleReader->>BibleReader: Display avatar or person icon
    
    Note over User,YouVersionAPI: Sign Out Flow
    User->>BibleReader: Click avatar
    BibleReader->>BibleReader: Open popover
    User->>BibleReader: Click Sign Out button
    BibleReader->>useYVAuth: Call signOut
    useYVAuth->>YouVersionAPI: clearAuthTokens
    useYVAuth->>AuthProvider: setUserInfo to null
    BibleReader->>BibleReader: Display person icon

Loading

[cameronapak]

This PR looks good to me. I had one question. I'm gonna ask greptile a question, and then this should be good to go.

And, please do add a changeset!

[cameronapak]

@greptileai, can you help me understand a little more about packages/hooks/src/useYVAuth.test.tsx

I saw there were two failed tests and want to understand more about why that might be happening.

× should use redirectUri from provider when redirectUrl is not passed 213ms
× should use redirectUri from provider with scopes when redirectUrl is not passed 205ms

[greptile-apps]

Based on my analysis of the code and diff, I can now explain what's happening with the failing tests:

Analysis of the Failing Tests

The two failing tests are:

1. should use redirectUri from provider when redirectUrl is not passed
2. should use redirectUri from provider with scopes when redirectUrl is not passed

The Problem:

Looking at the test expectations, both tests expect YouVersionAPIUsers.signIn to be called with 'http://test.example.com', which suggests that's the redirectUri that should be provided by the test's auth provider configuration.

However, the tests are likely failing because:

1. Missing Test Setup: The createAuthProviderWrapper function (which is imported but the file doesn't exist in the repository structure I can see) needs to provide a config with redirectUri: 'http://test.example.com' to the YouVersionAuthProvider.

2. Context Value Missing: From the changes in the diff, I can see that redirectUri was added to the AuthContextValue and is now being passed from the provider to the context. The test might be failing because the test wrapper isn't properly providing this value.

The Expected Flow:

1. The YouVersionAuthProvider receives a config prop with redirectUri
2. It passes this to the context: redirectUri: config.redirectUri
3. The useYVAuth hook gets this from context: const { userInfo, setUserInfo, isLoading, error, redirectUri } = useYouVersionAuthContext();
4. When signIn() is called without parameters, it should use redirectUri as the fallback

The Fix:

The test is expecting the createAuthProviderWrapper to provide a configuration like:

const TestWrapper = createAuthProviderWrapper({
  appKey: 'test-key',
  redirectUri: 'http://test.example.com'  // This is what the test expects
});

But it seems like either:

1. The createAuthProviderWrapper function doesn't exist or isn't properly implemented
2. The wrapper isn't providing the correct redirectUri value
3. There's an issue with how the redirectUri is being passed through the context

To fix these tests, you would need to ensure that the test wrapper provides the expected redirectUri value in the configuration passed to YouVersionAuthProvider.

[cameronapak]

suggestion: Can you add photos of the new sign in and sign out UI? When I see a pull request that is a UI update, then it would help a lot to see the UI so that I don't have to pull it locally to review it

[cameronapak]

LGTM