Skip to content

feat: implement Bible reader view for Android #39

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.

Sign up for GitHub

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

Open
sidorchukandrew wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: implement Bible reader view for Android #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions android/src/main/java/com/youversion/reactnativesdk/RNBibleReaderViewModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.youversion.reactnativesdk

import com.youversion.reactnativesdk.views.YVPBibleReaderView
import com.youversion.reactnativesdk.views.YVPBibleWidgetView
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 18, 2026

Choose a reason for hiding this comment

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

Unused import of YVPBibleWidgetView

YVPBibleWidgetView is imported but never referenced in this module — only YVPBibleReaderView is registered. YVPBibleWidgetView is already imported in its own module (RNBibleWidgetViewModule.kt). This appears to be accidentally added.

Suggested change
import com.youversion.reactnativesdk.views.YVPBibleWidgetView

import expo.modules.kotlin.modules.Module
import expo.modules.kotlin.modules.ModuleDefinition

Expand Down
59 changes: 36 additions & 23 deletions android/src/main/java/com/youversion/reactnativesdk/views/YVPBibleReaderView.kt
View file
Open in desktop
Copy link
Collaborator

@bmanquen bmanquen Feb 19, 2026

Choose a reason for hiding this comment

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

I agree with Greptile that we should not use !! since null is allowed and this can cause runtime crashes.

Original file line number Diff line number Diff line change
@@ -1,55 +1,68 @@
package com.youversion.reactnativesdk.views

import android.content.Context
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import com.youversion.platform.core.bibles.domain.BibleReference
import com.youversion.platform.reader.BibleReader
import expo.modules.kotlin.AppContext
import expo.modules.kotlin.views.AutoSizingComposable
import expo.modules.kotlin.views.ComposeProps
import expo.modules.kotlin.views.Direction
import expo.modules.kotlin.views.ExpoComposeView
import java.util.EnumSet
Comment on lines +11 to +15
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 18, 2026

Choose a reason for hiding this comment

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

Unused imports

AutoSizingComposable, Direction, and EnumSet are imported but never used in this file. They appear to have been carried over from YVPSignInWithYouVersionButton.kt where they are used for auto-sizing.

Suggested change
import expo.modules.kotlin.views.AutoSizingComposable
import expo.modules.kotlin.views.ComposeProps
import expo.modules.kotlin.views.Direction
import expo.modules.kotlin.views.ExpoComposeView
import java.util.EnumSet

Copy link
Collaborator

@bmanquen bmanquen Feb 19, 2026

Choose a reason for hiding this comment

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

This makes sense unless I am missing context.



data class BibleReaderViewProps(
// Bible reference
val versionId: MutableState<Int?> = mutableStateOf(null),
val bookUSFM: MutableState<String?> = mutableStateOf(null),
val chapter: MutableState<Int?> = mutableStateOf(null),
val verse: MutableState<Int?> = mutableStateOf(null),
val verseStart: MutableState<Int?> = mutableStateOf(null),
val verseEnd: MutableState<Int?> = mutableStateOf(null),

val appName: MutableState<String?> = mutableStateOf(null),
val signInMessage: MutableState<String?> = mutableStateOf(null),
val hasReference: MutableState<Boolean?> = mutableStateOf(false),
val hasReference: MutableState<Boolean?> = mutableStateOf(null)
) : ComposeProps

class YVPBibleReaderView(context: Context, appContext: AppContext) :
ExpoComposeView<BibleReaderViewProps>(context, appContext, withHostingView = true) {

override val props = BibleReaderViewProps()

@Composable
override fun Content(modifier: Modifier) {
// TODO: Replace with actual BibleReaderView composable when Kotlin SDK is ready
Box(
modifier = modifier
.fillMaxSize()
.padding(16.dp),
contentAlignment = Alignment.Center
) {
Text(
text = "BibleReaderView placeholder\n" +
"App: ${props.appName.value}\n" +
"Reference: ${props.bookUSFM.value} ${props.chapter.value}",
color = Color.Gray
BibleReader(
appName = props.appName.value!!,
appSignInMessage = props.signInMessage.value!!,
Comment on lines +36 to +38
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 18, 2026

Choose a reason for hiding this comment

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

Force-unwrapping nullable props will crash on initial render

props.appName.value and props.signInMessage.value are MutableState<String?> initialized to null. The !! operator will throw a NullPointerException when Content() is composed before React Native has sent the prop values. Unlike the iOS counterpart where @Field var appName: String provides a non-optional default, Android's MutableState<String?> starts as null.

Consider adding a guard to return early if the required props are not yet set:

Suggested change
BibleReader(
appName = props.appName.value!!,
appSignInMessage = props.signInMessage.value!!,
val appName = props.appName.value ?: return
val signInMessage = props.signInMessage.value ?: return
BibleReader(
appName = appName,
appSignInMessage = signInMessage,

Context Used: Rule from dashboard - Avoid unnecessary negations and else blocks to reduce nesting and improve code readability. Use earl... (source)

bibleReference = bibleReference(),
)
}

fun bibleReference(): BibleReference? {
val start = props.verseStart.value
val end = props.verseEnd.value

if (start != null && end != null) {
return BibleReference(
versionId = props.versionId.value!!,
bookUSFM = props.bookUSFM.value!!,
chapter = props.chapter.value!!,
verseStart = start,
verseEnd = end
)
}
Comment on lines +47 to 55
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 18, 2026

Choose a reason for hiding this comment

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

Force-unwraps on versionId, bookUSFM, and chapter risk NullPointerException

Similar to the Content() method, the !! operators on lines 49-51 will crash if these props haven't been set yet. Since bibleReference() is called from Content() during composition, the same timing issue applies — props may still be null on the first composition pass.

Consider using null-safe access and returning null if required fields are missing:

Suggested change
if (start != null && end != null) {
return BibleReference(
versionId = props.versionId.value!!,
bookUSFM = props.bookUSFM.value!!,
chapter = props.chapter.value!!,
verseStart = start,
verseEnd = end
)
}
if (start != null && end != null) {
val versionId = props.versionId.value ?: return null
val bookUSFM = props.bookUSFM.value ?: return null
val chapter = props.chapter.value ?: return null
return BibleReference(
versionId = versionId,
bookUSFM = bookUSFM,
chapter = chapter,
verseStart = start,
verseEnd = end
)
}

Context Used: Rule from dashboard - Avoid unnecessary negations and else blocks to reduce nesting and improve code readability. Use earl... (source)


if (props.hasReference.value == true) {
return BibleReference(
versionId = props.versionId.value!!,
bookUSFM = props.bookUSFM.value!!,
chapter = props.chapter.value!!,
verse = props.verse.value
)
}

return null
}
}