-
Notifications
You must be signed in to change notification settings - Fork 3
feat(ui): update Bible version picker to match Figma #267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
jaredhightower-youversion
merged 8 commits into
main
from
fix/update-bible-version-abbreviation-title-to-match-figma
Jun 26, 2026
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
07735ed
feat(ui): update Bible version abbreviation title to match Figma
jaredhightower-youversion 62f7d36
feat(core,hooks): add Organizations client and useOrganization hook
jaredhightower-youversion 4f38cce
feat(hooks): add useOrganizations multi-org resolver hook
jaredhightower-youversion 427364c
feat(ui): apply YouVersion brand fonts to BibleVersionPicker to match…
jaredhightower-youversion 45033df
revert(ui,core): revert brand fonts to Inter/Source Serif 4 pending l…
jaredhightower-youversion c036797
fix: resolved PR comments
jaredhightower-youversion 479c0b2
fix(ui): set Bible version abbreviation tile radius to 8px
jaredhightower-youversion 6ed6e3c
fix(ui): seed recent versions via localStorage mock in tile styling test
jaredhightower-youversion File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| --- | ||
| "@youversion/platform-core": minor | ||
| "@youversion/platform-react-hooks": minor | ||
| "@youversion/platform-react-ui": minor | ||
| --- | ||
|
|
||
| Update the Bible Version picker to match the latest Reader SDK Figma design, adding publisher names and refreshing the abbreviation tile. | ||
|
|
||
| - `@youversion/platform-core`: New `OrganizationsClient` with `getOrganization(organizationId)` for fetching an organization by its UUID (`GET /v1/organizations/{id}`), validated against the existing `OrganizationSchema`. Design tokens use Inter (`--yv-font-sans`) and Source Serif 4 (`--yv-font-serif`); the YouVersion brand fonts (Aktiv Grotesk App / Untitled Serif) are reverted pending licensing — see `docs/adr/0001-revert-brand-fonts-pending-licensing.md`. | ||
| - `@youversion/platform-react-hooks`: New `useOrganization(organizationId)` hook (plus `useOrganizationsClient`) following the standard `useApiData` pattern. Fetching is skipped when the id is empty. Also adds `useOrganizations(organizationIds)`, which resolves many organizations at once, deduplicated by id, so a list of versions sharing publishers only fetches each organization once. | ||
| - `@youversion/platform-react-ui`: `BibleVersionPicker` now renders the publisher name above the version title for versions that have an `organization_id` (rows without an associated organization render the title only), and recently used versions persist `organization_id` so they display the publisher too. Publisher names are resolved once at the list level via `useOrganizations` instead of per row, avoiding N+1 requests when many versions share a publisher. The `VersionAbbreviationIcon` tile now renders as a 64px square with a 6px radius, warm-neutral (`secondary`) fill, themed border, and serif typography (Source Serif 4) using the foreground text color; recent-version and all-version rows share the same tile styling, and long or trailing-digit abbreviations (e.g. `NASB1995` → `NASB` / `1995`) stay readable without overflowing. Brand fonts (Aktiv Grotesk App / Untitled Serif) are reverted to Inter / Source Serif 4 pending licensing; the brand-font implementation is parked on branch `feat/youversion-brand-fonts`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| # 1. Revert brand fonts to Inter / Source Serif 4 pending licensing | ||
|
|
||
| Date: 2026-06-24 | ||
|
|
||
| ## Status | ||
|
|
||
| Accepted | ||
|
|
||
| ## Context | ||
|
|
||
| The SDK had begun shipping YouVersion brand fonts to consumer apps: | ||
|
|
||
| - **Aktiv Grotesk App** (Dalton Maag) as the sans default (`--yv-font-sans`), | ||
| loaded via a hardcoded `@font-face` pointing at a public CDN woff2. | ||
| - **Untitled Serif** (Klim Type Foundry) as the serif default (`--yv-font-serif`) | ||
| and the Bible Version picker abbreviation tile, same hardcoded `@font-face` pattern. | ||
|
|
||
| Both create the same exposure: the SDK's purpose is to render fonts inside | ||
| **third-party developer apps**, so the font files are delivered to, and | ||
| downloadable by, third parties. | ||
|
|
||
| - **Aktiv Grotesk (Dalton Maag):** the licence is breached the moment a | ||
| third-party developer uses their app key and gains access to the actual font | ||
| file (`.woff`/`.woff2`/`.otf`/`.ttf`). No licence tier we hold covers serving | ||
| this font to arbitrary third parties. CORS / file-level protection is | ||
| enforced server-side (YouVersion API gateway + CDN), not in the SDK — the SDK | ||
| cannot make the file un-downloadable. | ||
| - **Untitled Serif (Klim):** an Enterprise licence may permit third-party use | ||
| if developers qualify as a "partner" (the licence enumerates affiliates, | ||
| agencies, partners, vendors, contractors, freelancers). Whether a Platform | ||
| developer is a "partner" is an **open legal question**. | ||
|
|
||
| A "browser-consumable stylesheet" endpoint exists | ||
| (`GET /v1/fonts/{font_id}/stylesheet`, accepts `app_key`, gateway injects the | ||
| app-id header). It is the correct future consumption pattern, but it does **not** | ||
| by itself resolve licensing: the woff2 it references still sits at a public CDN | ||
| URL, so switching to it does not make the font file un-downloadable. | ||
|
|
||
| ## Decision | ||
|
|
||
| Revert **both** brand fonts to the prior fallbacks for the shipping PR: | ||
|
|
||
| - `--yv-font-sans` → `'Inter', sans-serif` | ||
| - `--yv-font-serif` → `'Source Serif 4', serif` | ||
|
|
||
| Remove both brand `@font-face` blocks, the `--font-aktiv` / `--font-untitled-serif` | ||
| aliases, the `yv:font-aktiv` / `yv:font-untitled-serif` usages, and the brand | ||
| options in the Bible Reader font picker. The abbreviation-tile redesign and all | ||
| other Figma layout/typography work, the `useOrganizations` hooks, and publisher | ||
| names are retained — only the font **family** is reverted. | ||
|
|
||
| The brand-font implementation is parked on branch `feat/youversion-brand-fonts` | ||
| (snapshot at the pre-revert HEAD) for re-application once licensing clears. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - The SDK ships no licence-restricted font files to third parties. Defensible | ||
| legal state. | ||
| - The abbreviation tile and serif body text render in **Source Serif 4** (the | ||
| serif fallback) rather than Untitled Serif — closest legal match to the Figma | ||
| serif intent; exact brand match is deferred. | ||
| - Re-introducing brand fonts requires: (1) legal sign-off on Untitled Serif's | ||
| "partner" classification and/or a resolved Aktiv licence path, and (2) loading | ||
| via the gated `/v1/fonts/{font_id}/stylesheet` endpoint rather than hardcoded | ||
| `@font-face`. Untitled Serif is `font_id` 1 / slug `untitled-serif`. | ||
| - Re-application path: cherry-pick the font hunks from `feat/youversion-brand-fonts` | ||
| onto then-current `main`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import type { Organization } from '../types'; | ||
|
|
||
| export const mockLockmanOrganization: Organization = { | ||
| id: '798d8fa4-f640-4155-8cfb-fa91d1d8a06c', | ||
| name: 'The Lockman Foundation', | ||
| primary_language: 'en', | ||
| website_url: 'https://www.lockman.org', | ||
| }; | ||
|
|
||
| export const mockBiblicaOrganization: Organization = { | ||
| id: '05a9aa40-37b6-4e34-b9f1-a443fa4b1fff', | ||
| name: 'Biblica', | ||
| primary_language: 'en', | ||
| website_url: 'https://www.biblica.com', | ||
| }; | ||
|
|
||
| export const mockOrganizations: Organization[] = [mockLockmanOrganization, mockBiblicaOrganization]; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import { describe, it, expect, beforeEach, vi } from 'vitest'; | ||
| import { ApiClient } from '../client'; | ||
| import { OrganizationsClient } from '../organizations'; | ||
| import { OrganizationSchema } from '../schemas'; | ||
|
|
||
| describe('OrganizationsClient', () => { | ||
| let apiClient: ApiClient; | ||
| let organizationsClient: OrganizationsClient; | ||
|
|
||
| beforeEach(() => { | ||
| apiClient = new ApiClient({ | ||
| apiHost: process.env.YVP_API_HOST || '', | ||
| appKey: process.env.YVP_APP_KEY || '', | ||
| installationId: 'test-installation', | ||
| }); | ||
| organizationsClient = new OrganizationsClient(apiClient); | ||
| }); | ||
|
|
||
| describe('getOrganization', () => { | ||
| it('should fetch an organization by ID', async () => { | ||
| const organization = await organizationsClient.getOrganization( | ||
| '798d8fa4-f640-4155-8cfb-fa91d1d8a06c', | ||
| ); | ||
|
|
||
| const { success } = OrganizationSchema.safeParse(organization); | ||
| expect(success).toBe(true); | ||
| expect(organization.id).toBe('798d8fa4-f640-4155-8cfb-fa91d1d8a06c'); | ||
| expect(organization.name).toBe('The Lockman Foundation'); | ||
| }); | ||
|
|
||
| it('should request the organization endpoint with the provided ID', async () => { | ||
| const getSpy = vi.spyOn(apiClient, 'get'); | ||
|
|
||
| await organizationsClient.getOrganization('05a9aa40-37b6-4e34-b9f1-a443fa4b1fff'); | ||
|
|
||
| expect(getSpy).toHaveBeenCalledWith('/v1/organizations/05a9aa40-37b6-4e34-b9f1-a443fa4b1fff'); | ||
| getSpy.mockRestore(); | ||
| }); | ||
|
|
||
| it('should throw an error for invalid organization ID', async () => { | ||
| await expect(organizationsClient.getOrganization('not-a-uuid')).rejects.toThrow( | ||
| 'Organization ID must be a valid UUID', | ||
| ); | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { z } from 'zod'; | ||
| import type { ApiClient } from './client'; | ||
| import { OrganizationSchema } from './schemas'; | ||
| import type { Organization } from './types'; | ||
|
|
||
| /** Client for interacting with Organization API endpoints. */ | ||
| export class OrganizationsClient { | ||
| private client: ApiClient; | ||
|
|
||
| private static readonly organizationIdSchema = z | ||
| .string() | ||
| .trim() | ||
| .uuid('Organization ID must be a valid UUID'); | ||
|
|
||
| /** Creates a new OrganizationsClient instance. */ | ||
| constructor(client: ApiClient) { | ||
| this.client = client; | ||
| } | ||
|
|
||
| /** | ||
| * Fetches an organization by its ID. | ||
| * @param organizationId The organization UUID. | ||
| * @returns The requested Organization object. | ||
| */ | ||
| async getOrganization(organizationId: string): Promise<Organization> { | ||
| const parsedOrganizationId = OrganizationsClient.organizationIdSchema.parse(organizationId); | ||
| const organization = await this.client.get<Organization>( | ||
| `/v1/organizations/${parsedOrganizationId}`, | ||
| ); | ||
|
|
||
| return OrganizationSchema.parse(organization); | ||
|
bmanquen marked this conversation as resolved.
|
||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| import { renderHook, waitFor, act } from '@testing-library/react'; | ||
| import { describe, expect, vi, beforeEach, it } from 'vitest'; | ||
| import { useOrganization } from './useOrganization'; | ||
| import { type Organization, type OrganizationsClient } from '@youversion/platform-core'; | ||
| import { useOrganizationsClient } from './useOrganizationsClient'; | ||
| import { createYVWrapper } from './test/utils'; | ||
|
|
||
| vi.mock('./useOrganizationsClient'); | ||
|
|
||
| describe('useOrganization', () => { | ||
| const mockGetOrganization = vi.fn(); | ||
|
|
||
| const mockOrganization: Organization = { | ||
| id: '798d8fa4-f640-4155-8cfb-fa91d1d8a06c', | ||
| name: 'The Lockman Foundation', | ||
| primary_language: 'en', | ||
| website_url: 'https://www.lockman.org', | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| mockGetOrganization.mockResolvedValue(mockOrganization); | ||
|
|
||
| const mockClient: Partial<OrganizationsClient> = { getOrganization: mockGetOrganization }; | ||
| vi.mocked(useOrganizationsClient).mockReturnValue(mockClient as OrganizationsClient); | ||
| }); | ||
|
|
||
| describe('fetching organization', () => { | ||
| it('should fetch organization by ID', async () => { | ||
| const wrapper = createYVWrapper(); | ||
| const { result } = renderHook(() => useOrganization('798d8fa4-f640-4155-8cfb-fa91d1d8a06c'), { | ||
| wrapper, | ||
| }); | ||
|
|
||
| expect(result.current.loading).toBe(true); | ||
| expect(result.current.organization).toBe(null); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current.loading).toBe(false); | ||
| }); | ||
|
|
||
| expect.soft(mockGetOrganization).toHaveBeenCalledWith('798d8fa4-f640-4155-8cfb-fa91d1d8a06c'); | ||
| expect.soft(result.current.organization).toEqual(mockOrganization); | ||
| }); | ||
|
|
||
| it('should refetch when organizationId changes', async () => { | ||
| const wrapper = createYVWrapper(); | ||
| const { rerender } = renderHook(({ organizationId }) => useOrganization(organizationId), { | ||
| wrapper, | ||
| initialProps: { organizationId: '798d8fa4-f640-4155-8cfb-fa91d1d8a06c' }, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockGetOrganization).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| rerender({ organizationId: '05a9aa40-37b6-4e34-b9f1-a443fa4b1fff' }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockGetOrganization).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| expect(mockGetOrganization).toHaveBeenNthCalledWith( | ||
| 2, | ||
| '05a9aa40-37b6-4e34-b9f1-a443fa4b1fff', | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('enabled option', () => { | ||
| it('should not fetch when enabled is false', async () => { | ||
| const wrapper = createYVWrapper(); | ||
| const { result } = renderHook( | ||
| () => useOrganization('798d8fa4-f640-4155-8cfb-fa91d1d8a06c', { enabled: false }), | ||
| { wrapper }, | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current.loading).toBe(false); | ||
| }); | ||
|
|
||
| expect.soft(mockGetOrganization).not.toHaveBeenCalled(); | ||
| expect.soft(result.current.organization).toBe(null); | ||
| }); | ||
|
|
||
| it('should not fetch when organizationId is empty', async () => { | ||
| const wrapper = createYVWrapper(); | ||
| const { result } = renderHook(() => useOrganization(''), { wrapper }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current.loading).toBe(false); | ||
| }); | ||
|
|
||
| expect.soft(mockGetOrganization).not.toHaveBeenCalled(); | ||
| expect.soft(result.current.organization).toBe(null); | ||
| }); | ||
| }); | ||
|
|
||
| describe('error handling', () => { | ||
| it('should handle fetch errors', async () => { | ||
| const wrapper = createYVWrapper(); | ||
| const error = new Error('Failed to fetch organization'); | ||
| mockGetOrganization.mockRejectedValueOnce(error); | ||
|
|
||
| const { result } = renderHook(() => useOrganization('798d8fa4-f640-4155-8cfb-fa91d1d8a06c'), { | ||
| wrapper, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current.loading).toBe(false); | ||
| }); | ||
|
|
||
| expect.soft(result.current.error).toEqual(error); | ||
| expect.soft(result.current.organization).toBe(null); | ||
| }); | ||
| }); | ||
|
|
||
| describe('manual refetch', () => { | ||
| it('should support manual refetch', async () => { | ||
| const wrapper = createYVWrapper(); | ||
| const { result } = renderHook(() => useOrganization('798d8fa4-f640-4155-8cfb-fa91d1d8a06c'), { | ||
| wrapper, | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current.loading).toBe(false); | ||
| }); | ||
|
|
||
| expect.soft(mockGetOrganization).toHaveBeenCalledTimes(1); | ||
|
|
||
| act(() => { | ||
| result.current.refetch(); | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockGetOrganization).toHaveBeenCalledTimes(2); | ||
| }); | ||
| }); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.