ronantakizawa/codereview-bench huggingface dataset:
Repository: android/compose-samples
File: Jetcaster/app/src/main/java/com/example/jetcaster/ui/JetcasterAppState.kt (Kotlin)

Code before:
        fun createRoute(podcastUri: String) = "player/$podcastUri"
    }
}

@Composable
fun rememberJetcasterAppState(
    navController: NavHostController = rememberNavController(),
    context: Context = LocalContext.current
) = remember(navController, context) {
    JetcasterAppState(navController, context)
}

class JetcasterAppState(
    val navController: NavHostController,
    private val context: Context
) {
    var isOnline by mutableStateOf(checkIfOnline())
        private set

    fun refreshOnline() {
        isOnline = checkIfOnline()
    }

    @Suppress("DEPRECATION")
    private fun checkIfOnline(): Boolean {
        val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager

        return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
            val capabilities = cm.getNetworkCapabilities(cm.activeNetwork) ?: return false
            capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) &&
                capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
        } else {
            cm.activeNetworkInfo?.isConnectedOrConnecting == true
        }
    }
}


Review comment (suggestion):
Small nit: I like the `androidx.core` extension here, which avoids having to know the right `Context` constant:
```suggestion
        val cm = context.getSystemService<ConnectivityManager>()!!
```

Code after:
    object Player : Screen("player/{podcastUri}") {
        fun createRoute(podcastUri: String) = "player/$podcastUri"
    }
}

@Composable
fun rememberJetcasterAppState(
    navController: NavHostController = rememberNavController(),
    context: Context = LocalContext.current
) = remember(navController, context) {
    JetcasterAppState(navController, context)
}

class JetcasterAppState(
    val navController: NavHostController,
    private val context: Context
) {
    var isOnline by mutableStateOf(checkIfOnline())
        private set

    fun refreshOnline() {
        isOnline = checkIfOnline()
    }

    @Suppress("DEPRECATION")
    private fun checkIfOnline(): Boolean {
        val cm = getSystemService(context, ConnectivityManager::class.java)

        return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
            val capabilities = cm?.getNetworkCapabilities(cm.activeNetwork) ?: return false
            capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) &&
                capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
        } else {
            cm?.activeNetworkInfo?.isConnectedOrConnecting == true
        }
    }
}
Repository: android/compose-samples
File: JetNews/app/src/main/java/com/example/jetnews/ui/modifiers/KeyEvents.kt (Kotlin)

Code before:
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.example.jetnews.ui.modifiers

import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.input.key.Key
import androidx.compose.ui.input.key.KeyEventType.Companion.KeyUp
import androidx.compose.ui.input.key.key
import androidx.compose.ui.input.key.onPreviewKeyEvent
import androidx.compose.ui.input.key.type

/**
 * Note: this method intercepts the key event rather than passing it on to children
 */
fun Modifier.interceptKey(key: Key, onKeyEvent: () -> Unit): Modifier {
    return this.onPreviewKeyEvent {
        @OptIn(ExperimentalComposeUiApi::class)
        if (it.key == key && it.type == KeyUp) {
            onKeyEvent()
            true
        } else {
            false
        }
    }
}


Review comment (suggestion):
No need to say this is a note as it's the documentation for this function

```suggestion
 * Intercept a key event rather than passing it on to children
```

Code after:
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.example.jetnews.ui.modifiers

import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.input.key.Key
import androidx.compose.ui.input.key.KeyEventType.Companion.KeyUp
import androidx.compose.ui.input.key.key
import androidx.compose.ui.input.key.onPreviewKeyEvent
import androidx.compose.ui.input.key.type

/**
 * Intercepts a key event rather than passing it on to children
 */
@OptIn(ExperimentalComposeUiApi::class)
fun Modifier.interceptKey(key: Key, onKeyEvent: () -> Unit): Modifier {
    return this.onPreviewKeyEvent {
        if (it.key == key && it.type == KeyUp) { // fire onKeyEvent on KeyUp to prevent duplicates
            onKeyEvent()
            true
        } else it.key == key // only pass the key event to children if it's not the chosen key
    }
}
Repository: android/compose-samples
File: Reply/app/src/main/java/com/example/reply/ui/theme/Color.kt (Kotlin)

Code before:
package com.example.reply.ui.theme

import androidx.compose.ui.graphics.Color

// Generate them via theme builder
// https://material-foundation.github.io/material-theme-builder/#/custom

val reply_light_primary= Color(0xFF825500)
val reply_light_on_primary = Color(0xFFFFFFFF)
val reply_light_primary_container = Color(0xFFFFDDAE)
val reply_light_on_primary_container = Color(0xFF2A1800)
val reply_light_secondary = Color(0xFF6F5B40)
val reply_light_on_secondary = Color(0xFFFFFFFF)
val reply_light_secondary_container = Color(0xFFFADEBC)
val reply_light_on_secondary_container = Color(0xFF271904)
val reply_light_tertiary = Color(0xFF516440)
val reply_light_on_tertiary = Color(0xFFFFFFFF)
val reply_light_tertiary_container = Color(0xFFD3EABC)
val reply_light_on_tertiary_container = Color(0xFF102004)
val reply_light_error = Color(0xFFBA1B1B)
val reply_light_error_container = Color(0xFFFFDAD4)
val reply_light_on_error = Color(0xFFFFFFFF)
val reply_light_on_error_container = Color(0xFF410001)
val reply_light_background = Color(0xFFFCFCFC)
val reply_light_on_background = Color(0xFF1F1B16)
val reply_light_surface = Color(0xFFFCFCFC)
val reply_light_on_surface = Color(0xFF1F1B16)
val reply_light_surface_variant = Color(0xFFF0E0CF)
val reply_light_on_surface_variant = Color(0xFF4F4539)
val reply_light_outline = Color(0xFF817567)
val reply_light_inverse_on_surface = Color(0xFFF9EFE6)
val reply_light_inverse_surface = Color(0xFF34302A)
val reply_light_primary_inverse = Color(0xFFFFB945)

Review comment (bug):
I filed a bug for the theme builder about this, but... We should change these from snake_case to PascalCase

Code after:
package com.example.reply.ui.theme

import androidx.compose.ui.graphics.Color

// Generate them via theme builder
// https://material-foundation.github.io/material-theme-builder/#/custom

val replyLightPrimary= Color(0xFF825500)
val replyLightOnPrimary = Color(0xFFFFFFFF)
val replyLightPrimaryContainer = Color(0xFFFFDDAE)
val replyLightOnPrimaryContainer = Color(0xFF2A1800)
val replyLightSecondary = Color(0xFF6F5B40)
val replyLightOnSecondary = Color(0xFFFFFFFF)
val replyLightSecondaryContainer = Color(0xFFFADEBC)
val replyLightOnSecondaryContainer = Color(0xFF271904)
val replyLightTertiary = Color(0xFF516440)
val replyLightOnTertiary = Color(0xFFFFFFFF)
val replyLightTertiaryContainer = Color(0xFFD3EABC)
val replyLightOnTertiaryContainer = Color(0xFF102004)
val replyLightError = Color(0xFFBA1B1B)
val replyLightErrorContainer = Color(0xFFFFDAD4)
val replyLightOnError = Color(0xFFFFFFFF)
val replyLightOnErrorContainer = Color(0xFF410001)
val replyLightBackground = Color(0xFFFCFCFC)
val replyLightOnBackground = Color(0xFF1F1B16)
val replyLightSurface = Color(0xFFFCFCFC)
val replyLightOnSurface = Color(0xFF1F1B16)
val replyLightSurfaceVariant = Color(0xFFF0E0CF)
val replyLightOnSurfaceVariant = Color(0xFF4F4539)
val replyLightOutline = Color(0xFF817567)
val replyLightInverseOnSurface = Color(0xFFF9EFE6)
val replyLightInverseSurface = Color(0xFF34302A)
val replyLightPrimaryInverse = Color(0xFFFFB945)
Repository: android/compose-samples
File: Jetcaster/app/src/main/java/com/example/jetcaster/util/WindowInfoUtil.kt (Kotlin)

Code before:
    ) : DevicePosture

    data class Separating(
        val hingePosition: Rect,
        var orientation: FoldingFeature.Orientation
    ) : DevicePosture
}

@OptIn(ExperimentalContracts::class)
fun isTableTopPosture(foldFeature: FoldingFeature?): Boolean {
    contract { returns(true) implies (foldFeature != null) }
    return foldFeature?.state == FoldingFeature.State.HALF_OPENED &&
        foldFeature.orientation == FoldingFeature.Orientation.HORIZONTAL
}

@OptIn(ExperimentalContracts::class)
fun isBookPosture(foldFeature: FoldingFeature?): Boolean {
    contract { returns(true) implies (foldFeature != null) }
    return foldFeature?.state == FoldingFeature.State.HALF_OPENED &&
        foldFeature.orientation == FoldingFeature.Orientation.VERTICAL
}

@OptIn(ExperimentalContracts::class)
fun isSeparating(foldFeature: FoldingFeature?): Boolean {
    contract { returns(true) implies (foldFeature != null) }
    return foldFeature?.state == FoldingFeature.State.FLAT && foldFeature.isSeparating
}


Review comment (suggestion):
Nothing to do with this PR but `isSeparating` isn't the best of the names 😅  I don't know what it means!

Code after:
    ) : DevicePosture

    data class SeparatingPosture(
        val hingePosition: Rect,
        var orientation: FoldingFeature.Orientation
    ) : DevicePosture
}

@OptIn(ExperimentalContracts::class)
fun isTableTopPosture(foldFeature: FoldingFeature?): Boolean {
    contract { returns(true) implies (foldFeature != null) }
    return foldFeature?.state == FoldingFeature.State.HALF_OPENED &&
        foldFeature.orientation == FoldingFeature.Orientation.HORIZONTAL
}

@OptIn(ExperimentalContracts::class)
fun isBookPosture(foldFeature: FoldingFeature?): Boolean {
    contract { returns(true) implies (foldFeature != null) }
    return foldFeature?.state == FoldingFeature.State.HALF_OPENED &&
        foldFeature.orientation == FoldingFeature.Orientation.VERTICAL
}

@OptIn(ExperimentalContracts::class)
fun isSeparatingPosture(foldFeature: FoldingFeature?): Boolean {
    contract { returns(true) implies (foldFeature != null) }
    return foldFeature?.state == FoldingFeature.State.FLAT && foldFeature.isSeparating
}
Repository: android/compose-samples
File: Jetcaster/core/src/main/java/com/example/jetcaster/core/player/EpisodePlayer.kt (Kotlin)

Code before:
     * Stops the currently played episode
     */
    fun stop()

    /**
     * Plays another episode in the queue (if available)
     */
    fun next()

    /**
     * Plays the previous episode in the queue (if available). Or if an episode is currently
     * playing this will start the episode from the beginning
     */
    fun previous()

    /**
     * Advances a currently played episode by a given time interval specified in [duration].
     */
    fun advanceBy(duration: Duration)

    /**
     * Rewinds a currently played episode by a given time interval specified in [duration].
     */
    fun rewindBy(duration: Duration)

    fun increaseSpeed()

    fun decreaseSpeed()
}


Review comment (suggestion):
Maybe this should accept a `Duration`? It can default to half a second. Same comment for `decreaseSpeed`

Code after:
     * Stops the currently played episode
     */
    fun stop()

    /**
     * Plays another episode in the queue (if available)
     */
    fun next()

    /**
     * Plays the previous episode in the queue (if available). Or if an episode is currently
     * playing this will start the episode from the beginning
     */
    fun previous()

    /**
     * Advances a currently played episode by a given time interval specified in [duration].
     */
    fun advanceBy(duration: Duration)

    /**
     * Rewinds a currently played episode by a given time interval specified in [duration].
     */
    fun rewindBy(duration: Duration)

    /**
     * Increases the speed of Player playback by a given time specified in [duration].
     */
    fun increaseSpeed(speed: Duration = Duration.ofMillis(500))

    /**
     * Decreases the speed of Player playback by a given time specified in [duration].
     */
    fun decreaseSpeed(speed: Duration = Duration.ofMillis(500))
}
Repository: android/compose-samples
File: Jetcaster/core/src/main/java/com/example/jetcaster/core/player/MockEpisodePlayer.kt (Kotlin)

Code before:
        timerJob = null
    }

    override fun stop() {
        isPlaying.value = false
        timeElapsed.value = Duration.ZERO

        timerJob?.cancel()
        timerJob = null
    }

    override fun advanceBy(duration: Duration) {
        val currentEpisodeDuration = _currentEpisode.value?.duration ?: return
        timeElapsed.update {
            (it + duration).coerceAtMost(currentEpisodeDuration)
        }
    }

    override fun rewindBy(duration: Duration) {
        timeElapsed.update {
            (it - duration).coerceAtLeast(Duration.ZERO)
        }
    }

    override fun increaseSpeed(speed: Duration) {
        _playerSpeed.value += Duration.ofMillis(500)
    }

    override fun decreaseSpeed(speed: Duration) {
        _playerSpeed.value -= Duration.ofMillis(500)
    }

    override fun next() {
        val q = queue.value
        if (q.isEmpty()) {
            return
        }

        timeElapsed.value = Duration.ZERO
        val nextEpisode = q[0]
        currentEpisode = nextEpisode
        queue.value = q - nextEpisode
        play()
    }

    override fun previous() {
        timeElapsed.value = Duration.ZERO
        isPlaying.value = false
        timerJob?.cancel()
        timerJob = null
    }

Review comment (suggestion):
```suggestion
        _playerSpeed.value += speed
```

Code after:
        timerJob = null
    }

    override fun stop() {
        isPlaying.value = false
        timeElapsed.value = Duration.ZERO

        timerJob?.cancel()
        timerJob = null
    }

    override fun advanceBy(duration: Duration) {
        val currentEpisodeDuration = _currentEpisode.value?.duration ?: return
        timeElapsed.update {
            (it + duration).coerceAtMost(currentEpisodeDuration)
        }
    }

    override fun rewindBy(duration: Duration) {
        timeElapsed.update {
            (it - duration).coerceAtLeast(Duration.ZERO)
        }
    }

    override fun increaseSpeed(speed: Duration) {
        _playerSpeed.value += speed
    }

    override fun decreaseSpeed(speed: Duration) {
        _playerSpeed.value -= speed
    }

    override fun next() {
        val q = queue.value
        if (q.isEmpty()) {
            return
        }

        timeElapsed.value = Duration.ZERO
        val nextEpisode = q[0]
        currentEpisode = nextEpisode
        queue.value = q - nextEpisode
        play()
    }

    override fun previous() {
        timeElapsed.value = Duration.ZERO
        isPlaying.value = false
        timerJob?.cancel()
        timerJob = null
    }
Repository: android/compose-samples
File: Jetcaster/core/src/main/java/com/example/jetcaster/core/player/MockEpisodePlayer.kt (Kotlin)

Code before:
        isPlaying.value = false
        timeElapsed.value = Duration.ZERO

        timerJob?.cancel()
        timerJob = null
    }

    override fun advanceBy(duration: Duration) {
        val currentEpisodeDuration = _currentEpisode.value?.duration ?: return
        timeElapsed.update {
            (it + duration).coerceAtMost(currentEpisodeDuration)
        }
    }

    override fun rewindBy(duration: Duration) {
        timeElapsed.update {
            (it - duration).coerceAtLeast(Duration.ZERO)
        }
    }

    override fun increaseSpeed(speed: Duration) {
        _playerSpeed.value += Duration.ofMillis(500)
    }

    override fun decreaseSpeed(speed: Duration) {
        _playerSpeed.value -= Duration.ofMillis(500)
    }

    override fun next() {
        val q = queue.value
        if (q.isEmpty()) {
            return
        }

        timeElapsed.value = Duration.ZERO
        val nextEpisode = q[0]
        currentEpisode = nextEpisode
        queue.value = q - nextEpisode
        play()
    }

    override fun previous() {
        timeElapsed.value = Duration.ZERO
        isPlaying.value = false
        timerJob?.cancel()
        timerJob = null
    }

    private fun hasNext(): Boolean {
        return queue.value.isNotEmpty()
    }

Review comment (suggestion):
```suggestion
        _playerSpeed.value -= speed
```

Code after:
        isPlaying.value = false
        timeElapsed.value = Duration.ZERO

        timerJob?.cancel()
        timerJob = null
    }

    override fun advanceBy(duration: Duration) {
        val currentEpisodeDuration = _currentEpisode.value?.duration ?: return
        timeElapsed.update {
            (it + duration).coerceAtMost(currentEpisodeDuration)
        }
    }

    override fun rewindBy(duration: Duration) {
        timeElapsed.update {
            (it - duration).coerceAtLeast(Duration.ZERO)
        }
    }

    override fun increaseSpeed(speed: Duration) {
        _playerSpeed.value += speed
    }

    override fun decreaseSpeed(speed: Duration) {
        _playerSpeed.value -= speed
    }

    override fun next() {
        val q = queue.value
        if (q.isEmpty()) {
            return
        }

        timeElapsed.value = Duration.ZERO
        val nextEpisode = q[0]
        currentEpisode = nextEpisode
        queue.value = q - nextEpisode
        play()
    }

    override fun previous() {
        timeElapsed.value = Duration.ZERO
        isPlaying.value = false
        timerJob?.cancel()
        timerJob = null
    }

    private fun hasNext(): Boolean {
        return queue.value.isNotEmpty()
    }
Repository: android/compose-samples
File: Jetchat/app/src/main/java/com/example/compose/jetchat/components/JetchatDrawer.kt (Kotlin)

Code before:
    val context = LocalContext.current
    Row(
        modifier = Modifier
            .height(56.dp)
            .fillMaxWidth()
            .padding(horizontal = 12.dp)
            .clip(CircleShape)
            .clickable(onClick = {
                addWidgetToHomeScreen(context)
            }),
        verticalAlignment = CenterVertically
    ) {
        Text(
            stringResource(id = R.string.add_widget_to_home_page),
            style = MaterialTheme.typography.bodyMedium,
            color = MaterialTheme.colorScheme.onSurface,
            modifier = Modifier.padding(start = 12.dp)
        )
    }
}

private fun addWidgetToHomeScreen(context: Context) {
    val appWidgetManager = AppWidgetManager.getInstance(context)
    val myProvider = ComponentName(context, WidgetReceiver::class.java)

    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
        if (appWidgetManager.isRequestPinAppWidgetSupported) {
            appWidgetManager.requestPinAppWidget(myProvider, null, null)
        } else {
            Toast.makeText(
                context,
                "Unable to add widget, please add from home screen",
                Toast.LENGTH_LONG
            ).show()
        }
    } else {
        Toast.makeText(
            context,
            "Unable to add widget, please add from home screen",
            Toast.LENGTH_LONG
        ).show()
    }
}


Review comment (refactor):
Instead of showing the error only when user clicks the button, can we prevent showing the drawer item at all? 
Maybe you can abstract the method to something like `widgetAddingIsSupported()` that uses the SDK check and uses the `isRequestPinAppWidgetSupported` and check for that before showing the UI.

Code after:
@Composable
private fun WidgetDiscoverability() {
    val context = LocalContext.current
    Row(
        modifier = Modifier
            .height(56.dp)
            .fillMaxWidth()
            .padding(horizontal = 12.dp)
            .clip(CircleShape)
            .clickable(onClick = {
                addWidgetToHomeScreen(context)
            }),
        verticalAlignment = CenterVertically
    ) {
        Text(
            stringResource(id = R.string.add_widget_to_home_page),
            style = MaterialTheme.typography.bodyMedium,
            color = MaterialTheme.colorScheme.onSurface,
            modifier = Modifier.padding(start = 12.dp)
        )
    }
}

@RequiresApi(Build.VERSION_CODES.O)
private fun addWidgetToHomeScreen(context: Context) {
    val appWidgetManager = AppWidgetManager.getInstance(context)
    val myProvider = ComponentName(context, WidgetReceiver::class.java)
    if (widgetAddingIsSupported(context)) {
        appWidgetManager.requestPinAppWidget(myProvider, null, null)
    }
}

@ChecksSdkIntAtLeast(api = Build.VERSION_CODES.O)
private fun widgetAddingIsSupported(context: Context): Boolean {
    return Build.VERSION.SDK_INT >= Build.VERSION_CODES.O &&
        AppWidgetManager.getInstance(context).isRequestPinAppWidgetSupported
}