Fix QR icon background and changes for code review

2025-02-21 19:28:27 +00:00 · 2024-06-07 15:28:12 +09:30 · 2024-06-07 15:28:12 +09:30 · f66fbef0ad
commit f66fbef0ad
parent 79c35b0e3b
10 changed files with 88 additions and 84 deletions
--- a/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/Callbacks.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/Callbacks.kt
@ -3,5 +3,5 @@ package org.thoughtcrime.securesms.conversation.newmessage
 interface Callbacks {
    fun onChange(value: String) {}
    fun onContinue() {}
-    fun onScan(value: String) {}
+    fun onScanQrCode(value: String) {}
 }
--- a/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageFragment.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageFragment.kt
@ -46,7 +46,6 @@ import org.thoughtcrime.securesms.conversation.v2.ConversationActivityV2
 import org.thoughtcrime.securesms.dependencies.DatabaseComponent
 import org.thoughtcrime.securesms.showOpenUrlDialog
 import org.thoughtcrime.securesms.ui.AppTheme
-import org.thoughtcrime.securesms.ui.GetString
 import org.thoughtcrime.securesms.ui.LoadingArcOr
 import org.thoughtcrime.securesms.ui.PreviewTheme
 import org.thoughtcrime.securesms.ui.ThemeResPreviewParameterProvider
@ -134,7 +133,7 @@ private fun NewMessage(
        HorizontalPager(pagerState) {
            when (TITLES[it]) {
                R.string.enter_account_id -> EnterAccountId(state, callbacks, onHelp)
-                R.string.qrScan -> MaybeScanQrCode(errors, onScan = callbacks::onScan)
+                R.string.qrScan -> MaybeScanQrCode(errors, onScan = callbacks::onScanQrCode)
            }
        }
    }
--- a/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageViewModel.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageViewModel.kt
@ -45,7 +45,7 @@ class NewMessageViewModel @Inject constructor(
        createPrivateChatIfPossible(state.value.newMessageIdOrOns)
    }

-    override fun onScan(value: String) {
+    override fun onScanQrCode(value: String) {
        if (PublicKeyValidation.isValid(value, isPrefixRequired = false) && PublicKeyValidation.hasValidPrefix(value)) {
            onPublicKey(value)
        } else {
--- a/app/src/main/java/org/thoughtcrime/securesms/conversation/start/NewConversationHomeFragment.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/start/NewConversationHomeFragment.kt
@ -27,6 +27,7 @@ import org.thoughtcrime.securesms.ui.ItemButton
 import org.thoughtcrime.securesms.ui.classicDarkColors
 import org.thoughtcrime.securesms.ui.components.AppBar
 import org.thoughtcrime.securesms.ui.components.QrImage
+import org.thoughtcrime.securesms.ui.contentDescription
 import org.thoughtcrime.securesms.ui.medium
 import org.thoughtcrime.securesms.ui.small
 import javax.inject.Inject
@ -70,7 +71,7 @@ class NewConversationHomeFragment : Fragment() {
                Spacer(modifier = Modifier.height(4.dp))
                Text(text = stringResource(R.string.qrYoursDescription), color = classicDarkColors[5], style = MaterialTheme.typography.small)
                Spacer(modifier = Modifier.height(20.dp))
-                QrImage(string = TextSecurePreferences.getLocalNumber(requireContext())!!, contentDescription = stringResource(R.string.AccessibilityId_qr_code))
+                QrImage(string = TextSecurePreferences.getLocalNumber(requireContext())!!, Modifier.contentDescription(R.string.AccessibilityId_qr_code))
            }
        }
    }
--- a/app/src/main/java/org/thoughtcrime/securesms/home/HomeActivity.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/home/HomeActivity.kt
@ -103,7 +103,6 @@ import org.thoughtcrime.securesms.ui.SessionShieldIcon
 import org.thoughtcrime.securesms.ui.ThemeResPreviewParameterProvider
 import org.thoughtcrime.securesms.ui.components.OutlineButton
 import org.thoughtcrime.securesms.ui.contentDescription
-import org.thoughtcrime.securesms.ui.contentDescription
 import org.thoughtcrime.securesms.ui.h8
 import org.thoughtcrime.securesms.ui.small
 import org.thoughtcrime.securesms.util.ConfigurationMessageUtilities
--- a/app/src/main/java/org/thoughtcrime/securesms/onboarding/LinkDeviceViewModel.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/onboarding/LinkDeviceViewModel.kt
@ -4,19 +4,12 @@ import android.app.Application
 import androidx.lifecycle.AndroidViewModel
 import androidx.lifecycle.viewModelScope
 import dagger.hilt.android.lifecycle.HiltViewModel
-import kotlinx.coroutines.ExperimentalCoroutinesApi
-import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.asStateFlow
-import kotlinx.coroutines.flow.collect
-import kotlinx.coroutines.flow.consumeAsFlow
-import kotlinx.coroutines.flow.debounce
-import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapNotNull
 import kotlinx.coroutines.flow.receiveAsFlow
 import kotlinx.coroutines.flow.take
-import kotlinx.coroutines.flow.takeWhile
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import network.loki.messenger.R
@ -26,7 +19,6 @@ import org.session.libsignal.crypto.MnemonicCodec.DecodingError.InvalidWord
 import org.session.libsignal.utilities.Hex
 import org.thoughtcrime.securesms.crypto.MnemonicUtilities
 import javax.inject.Inject
-import kotlin.time.Duration.Companion.seconds

 class LinkDeviceEvent(val mnemonic: ByteArray)

@ -59,7 +51,7 @@ class LinkDeviceViewModel @Inject constructor(
        viewModelScope.launch {
            runDecodeCatching(string)
                .onSuccess(::onSuccess)
-                .onFailure(::onScanFailure)
+                .onFailure(::onQrCodeScanFailure)
        }
    }

@ -82,7 +74,7 @@ class LinkDeviceViewModel @Inject constructor(
        }
    }

-    private fun onScanFailure(error: Throwable) {
+    private fun onQrCodeScanFailure(error: Throwable) {
        viewModelScope.launch { qrErrors.send(error) }
    }

--- a/app/src/main/java/org/thoughtcrime/securesms/onboarding/recoverypassword/RecoveryPasswordActivity.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/onboarding/recoverypassword/RecoveryPasswordActivity.kt
@ -163,8 +163,8 @@ fun RecoveryPasswordCell(seed: String, copySeed:() -> Unit = {}) {
            ) {
                QrImage(
                    seed,
-                    modifier = Modifier.padding(vertical = 24.dp),
-                    contentDescription = stringResource(R.string.AccessibilityId_qr_code),
+                    modifier = Modifier.padding(vertical = 24.dp)
+                        .contentDescription(R.string.AccessibilityId_qr_code),
                    icon = R.drawable.session_shield
                )
            }
--- a/app/src/main/java/org/thoughtcrime/securesms/preferences/QRCodeActivity.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/preferences/QRCodeActivity.kt
@ -2,6 +2,7 @@ package org.thoughtcrime.securesms.preferences

 import android.os.Bundle
 import androidx.compose.foundation.ExperimentalFoundationApi
+import androidx.compose.foundation.background
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.padding
@ -28,6 +29,7 @@ import org.thoughtcrime.securesms.database.threadDatabase
 import org.thoughtcrime.securesms.ui.components.MaybeScanQrCode
 import org.thoughtcrime.securesms.ui.components.QrImage
 import org.thoughtcrime.securesms.ui.components.SessionTabRow
+import org.thoughtcrime.securesms.ui.contentDescription
 import org.thoughtcrime.securesms.ui.setComposeContent
 import org.thoughtcrime.securesms.ui.small
 import org.thoughtcrime.securesms.util.start
@ -43,7 +45,11 @@ class QRCodeActivity : PassphraseRequiredActionBarActivity() {
        supportActionBar!!.title = resources.getString(R.string.activity_qr_code_title)

        setComposeContent {
-            Tabs(TextSecurePreferences.getLocalNumber(this)!!, errors.receiveAsFlow(), onScan = ::onScan)
+            Tabs(
+                TextSecurePreferences.getLocalNumber(this)!!,
+                errors.receiveAsFlow(),
+                onScan = ::onScan
+            )
        }
    }

@ -86,13 +92,15 @@ private fun Tabs(sessionId: String, errors: Flow<String>, onScan: (String) -> Un
 fun QrPage(string: String) {
    Column(
        modifier = Modifier
+            .background(MaterialTheme.colors.surface)
            .padding(horizontal = 32.dp)
            .fillMaxSize()
    ) {
        QrImage(
            string = string,
-            contentDescription = stringResource(R.string.AccessibilityId_qr_code),
-            modifier = Modifier.padding(top = 32.dp, bottom = 12.dp),
+            modifier = Modifier
+                .padding(top = 32.dp, bottom = 12.dp)
+                .contentDescription(R.string.AccessibilityId_qr_code),
            icon = R.drawable.session
        )

--- a/app/src/main/java/org/thoughtcrime/securesms/ui/components/QrImage.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/ui/components/QrImage.kt
@ -4,9 +4,11 @@ import android.graphics.Bitmap
 import androidx.compose.animation.AnimatedVisibility
 import androidx.compose.animation.fadeIn
 import androidx.compose.foundation.Image
+import androidx.compose.foundation.background
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.aspectRatio
 import androidx.compose.foundation.layout.fillMaxWidth
+import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
 import androidx.compose.material.Card
 import androidx.compose.material.Icon
@ -20,8 +22,11 @@ import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.graphics.ColorFilter
+import androidx.compose.ui.graphics.FilterQuality
 import androidx.compose.ui.graphics.asImageBitmap
+import androidx.compose.ui.layout.ContentScale
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.unit.dp
 import kotlinx.coroutines.Dispatchers
@ -34,7 +39,6 @@ import org.thoughtcrime.securesms.util.QRCodeUtilities
@Composable
 fun QrImage(
    string: String,
-    contentDescription: String,
    modifier: Modifier = Modifier,
    icon: Int = R.drawable.session_shield
 ) {
@ -45,60 +49,63 @@ fun QrImage(
    val scope = rememberCoroutineScope()
    LaunchedEffect(string) {
        scope.launch(Dispatchers.IO) {
-            val c = 100
-            val w = c * 2
-            bitmap = QRCodeUtilities.encode(string, w).also {
-                val hw = 30
-                for (y in c - hw until c + hw) {
-                    for (x in c - hw until c + hw) {
-                        it.setPixel(x, y, 0x00000000)
-                    }
-                }
+            bitmap = (300..500 step 100).firstNotNullOf {
+                runCatching { QRCodeUtilities.encode(string, it) }.getOrNull()
            }
        }
    }

-    @Composable
-    fun content(modifier: Modifier = Modifier) {
-        Box(
-            modifier = modifier
-                .fillMaxWidth()
-                .aspectRatio(1f)
-        ) {
-            AnimatedVisibility(
-                visible = bitmap != null,
-                enter = fadeIn(),
-            ) {
-                bitmap?.let {
-                    Image(
-                        bitmap = it.asImageBitmap(),
-                        modifier = Modifier
-                            .fillMaxWidth()
-                            .aspectRatio(1f),
-                        contentDescription = contentDescription,
-                        colorFilter = ColorFilter.tint(LocalOnLightCell.current)
-                    )
-                }
-            }
-
-            Icon(
-                painter = painterResource(id = icon),
-                contentDescription = "",
-                tint = LocalOnLightCell.current,
-                modifier = Modifier
-                    .align(Alignment.Center)
-                    .size(60.dp)
-            )
-        }
-    }
-
    if (MaterialTheme.colors.isLight) {
-        content(modifier)
+        Content(bitmap, icon, modifier = modifier, backgroundColor = MaterialTheme.colors.surface)
    } else {
        Card(
            backgroundColor = LocalLightCell.current,
            elevation = 0.dp,
            modifier = modifier
-        ) { content() }
+        ) { Content(bitmap, icon, modifier = Modifier.padding(16.dp), backgroundColor = LocalLightCell.current) }
    }
 }
+
+@Composable
+private fun Content(
+    bitmap: Bitmap?,
+    icon: Int,
+    modifier: Modifier = Modifier,
+    qrColor: Color = LocalOnLightCell.current,
+    backgroundColor: Color,
+) {
+    Box(
+        modifier = modifier
+            .fillMaxWidth()
+            .aspectRatio(1f)
+    ) {
+        AnimatedVisibility(
+            visible = bitmap != null,
+            enter = fadeIn(),
+        ) {
+            bitmap?.let {
+                Image(
+                    bitmap = it.asImageBitmap(),
+                    contentDescription = "",
+                    contentScale = ContentScale.Crop,
+                    modifier = Modifier
+                        .fillMaxWidth()
+                        .aspectRatio(1f),
+                    colorFilter = ColorFilter.tint(qrColor),
+                    // Use FilterQuality.None to keep QR edges sharp
+                    filterQuality = FilterQuality.None
+                )
+            }
+        }
+
+        Icon(
+            painter = painterResource(id = icon),
+            contentDescription = "",
+            tint = LocalOnLightCell.current,
+            modifier = Modifier
+                .align(Alignment.Center)
+                .size(60.dp)
+                .background(color = backgroundColor)
+        )
+    }
+}
--- a/app/src/main/java/org/thoughtcrime/securesms/util/QRCodeUtilities.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/util/QRCodeUtilities.kt
@ -4,8 +4,8 @@ import android.graphics.Bitmap
 import android.graphics.Color
 import com.google.zxing.BarcodeFormat
 import com.google.zxing.EncodeHintType
-import com.google.zxing.WriterException
 import com.google.zxing.qrcode.QRCodeWriter
+import com.google.zxing.qrcode.decoder.ErrorCorrectionLevel

 object QRCodeUtilities {

@ -16,25 +16,23 @@ object QRCodeUtilities {
        hasTransparentBackground: Boolean = true,
        dark: Int = Color.BLACK,
        light: Int = Color.WHITE,
-    ): Bitmap {
-        try {
-            val hints = hashMapOf( EncodeHintType.MARGIN to 1 )
-            val result = QRCodeWriter().encode(data, BarcodeFormat.QR_CODE, size, size, hints)
-            val bitmap = Bitmap.createBitmap(result.width, result.height, Bitmap.Config.ARGB_8888)
-            val color = if (isInverted) light else dark
-            val background = if (isInverted) dark else light
+    ): Bitmap? = runCatching {
+        val hints = hashMapOf(
+            EncodeHintType.MARGIN to 0,
+            EncodeHintType.ERROR_CORRECTION to ErrorCorrectionLevel.H
+        )
+        val color = if (isInverted) light else dark
+        val background = if (isInverted) dark else light
+        val result = QRCodeWriter().encode(data, BarcodeFormat.QR_CODE, size, size, hints)
+        Bitmap.createBitmap(result.width, result.height, Bitmap.Config.ARGB_8888).apply {
            for (y in 0 until result.height) {
                for (x in 0 until result.width) {
-                    if (result.get(x, y)) {
-                        bitmap.setPixel(x, y, color)
-                    } else if (!hasTransparentBackground) {
-                        bitmap.setPixel(x, y, background)
+                    when {
+                        result.get(x, y) -> setPixel(x, y, color)
+                        !hasTransparentBackground -> setPixel(x, y, background)
                    }
                }
            }
-            return bitmap
-        } catch (e: WriterException) {
-            return Bitmap.createBitmap(512, 512, Bitmap.Config.ARGB_8888)
        }
-    }
-}
+    }.getOrNull()
+}