feat: RN Expo Bible version filter (YPE-4658) - #142
Conversation
Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>
Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>
Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>
Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>
Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>
Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>
Why Bible components do not take the version lists as public propsPartners set The WebView is a second React tree. It is also a second JavaScript runtime. React Native context cannot cross that boundary. As a result, each native wrapper (still in React Native) reads the lists from context. Then the wrapper copies the lists into the props of the DOM component. This copy is SDK plumbing. It is not a second partner API. If a partner must pass |
cameronapak
left a comment
There was a problem hiding this comment.
Review (YPE-4658) @ 284f2db
Code Reviewer bot, sent on behalf of Cam.
This review uses Matt Pocock code-review (Standards + Spec). Format: https://conventionalcomments.org
Counts
- Standards: 1 must-fix, 3 worth, 1 nit. Worst: the picker WebView does not get the three lists.
- Spec: 1 must-fix, 2 worth, 1 nit. Worst: the same picker miss. A host cannot keep versions out of the picker.
Visual
YouVersionProvider (core)
permittedVersionIds / excludedVersionIds / permittedLanguageTags
BibleCard / Reader / TextView / VOTD
DOM YouVersionProvider lists reach WebView
BibleVersionPickerSheet
bible-version-picker-content
YouVersionProvider(appKey only) lists do not reach WebView
Standards
issue (must-fix): BibleVersionPickerSheet and bible-version-picker-content.tsx do not forward permittedVersionIds, excludedVersionIds, or permittedLanguageTags. Native context does not cross the WebView. The picker mounts its own web YouVersionProvider with appKey only. See CONTEXT.md Expo DOM, Native-Owned State, and Version Picker Sheet.
suggestion (worth): Only the BibleCard native-to-DOM test asserts latestDomProps for the lists. Reader, text view, VOTD, and picker have no equivalent (AGENTS.md test layers).
suggestion (worth): Version-filter terms are in the README only. CONTEXT.md does not name them.
suggestion (worth): The three arrays copy through many files (Shotgun Surgery / Duplicated Code / Data Clumps). VersionFilterProps exists for DOM. Native sites still name each field.
nitpick: Primitive Obsession. The sister web API uses three arrays.
Spec
issue (must-fix): The picker WebView never gets the three lists. The spec says a host cannot keep unwanted versions out of the picker, recents, reader, card, text view, and Verse of the Day. Lists on RN core context alone do not reach the WebView client. Recents will not hide. If the user selects an excluded version, native onSelect writes it through setVersionId (MMKV).
suggestion (worth): Locked seam 2 is incomplete. Tests cover the web-yv-provider shim and BibleCard primary DOM only. They do not cover the picker web provider.
suggestion (worth): BibleCardProps, BibleReaderProps, BibleTextViewProps, and VerseOfTheDayProps inherit VersionFilterProps and do not omit the three keys. The spec puts the three lists on core YouVersionProvider only. No per-component lists. Runtime still uses context. Types advertise an API that does not work.
nitpick: Unrelated prettier edits on CONTRIBUTING.md, ADRs 0013-0015, apps/example/app/(tabs)/profile.tsx, and unrelated tests. No second ADR (good).
Locks that hold
Same three names on core context. Unset and [] stay distinct and are tested. UI wrapper forwards. No configure() and no YouVersionPlatformConfiguration. Pin stays @youversion/platform-react-ui@2.6.2 with a local widen. No second native usability predicate. Refuse tests pass stored or host versionId through and leave MMKV blobs. Terms stay 4657 (version filter, version refuse).
- Forward permittedVersionIds/excludedVersionIds/permittedLanguageTags through BibleVersionPickerSheet into bible-version-picker-content web provider - Split InternalVersionFilterProps from exported DOM bridge props so hosts cannot set filter lists on BibleReader/Card/TextView/VOTD - Extend seam-2 tests for reader, text, VOTD, picker (native→DOM→web shim) - Add version filter and version refuse terms to CONTEXT.md Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>
There was a problem hiding this comment.
Correctness pass over the diff, using conventional comments. Two findings inline: one blocking, one non-blocking.
praise: full suite (377 ui + 555 core) and typecheck are green on 38a14e0, and prop names and enforcement semantics match platform-sdk-react#340 exactly — permittedVersionIds / excludedVersionIds / permittedLanguageTags line up, and the applySDKConfig vs. render-time config write ordering is correct.
note (non-blocking): two things I checked and cleared, recorded so nobody re-derives them. The array props in the core provider's useMemo deps churn identity on inline literals, but Expo's webview-wrapper rebuilds smartActions and re-emits $$props on every render regardless, so it costs nothing extra. And the DOM bridge replaces rather than merges marshalled props, so clearing a list at runtime does propagate correctly.
… tests - Forward filter lists through BibleChapterPickerSheet into chapter-picker-content - Replace tautological webProviderPropsFromDomBridge with source-text asserts on each DOM entry (highlights-bridge pattern) so dropped JSX props fail seam 2 - Cover native→DOM forward for chapter picker (unset vs empty) Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>
Summary
Implements YPE-4658: optional Bible version filter lists on the existing core
YouVersionProvider, forwarded by the UI provider through native wrappers into each DOM webYouVersionProvider. Aligns with sister policy YPE-4657 (not published yet).AI agent working on behalf of Cam.
Public API (core + UI
YouVersionProvider)permittedVersionIds?: number[]— unset = no restriction;[]= permit nothingexcludedVersionIds?: number[]— exclusion winspermittedLanguageTags?: string[]— BCP 47 (en,zh-Hans)Lists are provider-only (not on
BibleReader/BibleCard/BibleTextView/VerseOfTheDay). Native stores and forwards only. No native usability predicate, no auto-pick, no silent 3034 swap when a stored/hostversionIdexists, no MMKV rewrites on version refuse.Forwarding path
YouVersionProvider→ core context → native wrappers (BibleReader / BibleCard / BibleTextView / VerseOfTheDay / BibleVersionPickerSheet / BibleChapterPickerSheet) → DOM →web-yv-provider→ webYouVersionProvider.TDD seams (three locked)
packages/core/src/__tests__/youversion-provider.test.tsx,packages/ui/src/native/__tests__/youversion-provider.test.tsxpackages/ui/src/lib/__tests__/web-yv-provider.test.tsxshim + source-text asserts that every DOM entry passes the three lists onto webYouVersionProvider(BibleCard, BibleReader, BibleTextView, VerseOfTheDay, version picker, chapter picker) + native wrapperlatestDomPropsforward testspackages/ui/src/native/__tests__/bible-reader-location.test.tsx,packages/ui/src/native/__tests__/bible-card-version-persistence.test.tsxMerge gate
Do not merge until a 4657-shaped
@youversion/platform-react-uiis published and pinned inpackages/ui/package.json. This PR keeps the current2.6.2pin and widensweb-yv-providerprop types locally (same pattern asadditionalHeaders). Version filtering in the WebView will not be live until that publish + pin bump.Verification
Greptile Summary
Adds provider-level Bible version and language filters and forwards them across the native-to-DOM bridge.
Confidence Score: 5/5
The PR appears safe to merge because no blocking failure remains in the reviewed follow-up scope.
The previously reported auxiliary-picker gap is closed: the built-in BibleCard picker reads the same core context and forwards all three filter lists through its DOM provider.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR App["Consumer YouVersionProvider"] --> Core["Core provider context"] Core --> Native["Native Bible wrappers"] Native --> MainDOM["Bible content DOM WebViews"] Native --> Picker["Chapter and version picker WebViews"] MainDOM --> WebProvider["Web YouVersionProvider"] Picker --> WebProvider WebProvider --> Filter["Web SDK version filtering"]Reviews (3): Last reviewed commit: "fix(ui): forward version filter through ..." | Re-trigger Greptile
Context used (4)