Skip to content

Commit

Permalink
fix: block sending empty messages, also empty markdowns [WPB-10179] (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
saleniuk authored and github-actions[bot] committed Jul 25, 2024
1 parent 5af0fdd commit 60a9e35
Show file tree
Hide file tree
Showing 5 changed files with 270 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import com.wire.android.R
import com.wire.android.ui.common.colorsScheme
import com.wire.android.ui.common.textfield.WireTextFieldColors
import com.wire.android.ui.common.textfield.wireTextFieldColors
import com.wire.android.util.isNotMarkdownBlank
import com.wire.android.util.ui.KeyboardHeight

@Stable
Expand Down Expand Up @@ -69,11 +70,11 @@ class MessageCompositionInputStateHolder(val messageTextState: TextFieldState) {
val inputType: InputType by derivedStateOf {
when (val state = compositionState) {
is CompositionState.Composing -> InputType.Composing(
isSendButtonEnabled = messageTextState.text.isNotEmpty()
isSendButtonEnabled = messageTextState.text.isNotMarkdownBlank()
)

is CompositionState.Editing -> InputType.Editing(
isEditButtonEnabled = messageTextState.text.toString() != state.originalMessageText
isEditButtonEnabled = messageTextState.text != state.originalMessageText && messageTextState.text.isNotMarkdownBlank()
)
}
}
Expand Down
31 changes: 31 additions & 0 deletions app/src/main/kotlin/com/wire/android/ui/markdown/MarkdownHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,37 @@ fun MarkdownNode.getFirstInlines(): MarkdownPreview? {
}
}

private fun List<MarkdownNode>.isNotBlank(): Boolean = this.any {
when (it) {
is MarkdownNode.Document -> it.children.isNotBlank()
is MarkdownNode.Block.BlockQuote -> it.children.isNotBlank()
is MarkdownNode.Block.FencedCode -> it.literal.isNotBlank()
is MarkdownNode.Block.Heading -> it.children.isNotBlank()
is MarkdownNode.Block.IntendedCode -> it.literal.isNotBlank()
is MarkdownNode.Block.ListBlock.Bullet -> it.children.isNotBlank()
is MarkdownNode.Block.ListBlock.Ordered -> it.children.isNotBlank()
is MarkdownNode.Block.ListItem -> it.children.isNotBlank()
is MarkdownNode.Block.Paragraph -> it.children.isNotBlank()
is MarkdownNode.Block.Table -> it.children.isNotBlank()
is MarkdownNode.Block.TableContent.Body -> it.children.isNotBlank()
is MarkdownNode.Block.TableContent.Head -> it.children.isNotBlank()
is MarkdownNode.Block.ThematicBreak -> true
is MarkdownNode.Inline.Break -> true
is MarkdownNode.Inline.Code -> it.literal.isNotBlank()
is MarkdownNode.Inline.Emphasis -> it.children.isNotBlank()
is MarkdownNode.Inline.Image -> it.destination.isNotBlank()
is MarkdownNode.Inline.Link -> it.destination.isNotBlank()
is MarkdownNode.Inline.Strikethrough -> it.children.isNotBlank()
is MarkdownNode.Inline.StrongEmphasis -> it.children.isNotBlank()
is MarkdownNode.Inline.Text -> it.literal.isNotBlank()
is MarkdownNode.TableCell -> it.children.isNotBlank()
is MarkdownNode.TableRow -> it.children.isNotBlank()
is MarkdownNode.Unsupported -> false
}
}

fun MarkdownNode.Document.isNotBlank(): Boolean = children.isNotBlank()

private fun List<MarkdownNode.Inline>.toPreview(): MarkdownPreview {
return MarkdownPreview(this.toPersistentList())
}
Expand Down
6 changes: 5 additions & 1 deletion app/src/main/kotlin/com/wire/android/util/StringUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package com.wire.android.util

import com.wire.android.ui.markdown.isNotBlank
import com.wire.android.ui.markdown.toMarkdownDocument
import java.math.BigInteger
import java.security.MessageDigest

Expand All @@ -31,7 +33,7 @@ val String.Companion.NEW_LINE_SYMBOL get() = "\n"

fun String?.orDefault(default: String) = this ?: default

public inline fun String.ifNotEmpty(transform: () -> String): String = if (!isEmpty()) transform() else this
inline fun String.ifNotEmpty(transform: () -> String): String = if (!isEmpty()) transform() else this

@Suppress("MagicNumber")
fun String.sha256(): String {
Expand All @@ -47,3 +49,5 @@ fun String.toTitleCase(delimiter: String = " ", separator: String = " "): String
fun String.capitalizeFirstLetter(): String = lowercase().replaceFirstChar(Char::titlecaseChar)

fun String.normalizeFileName(): String = this.replace("/", "")

fun CharSequence.isNotMarkdownBlank(): Boolean = this.isNotBlank() && this.toString().toMarkdownDocument().isNotBlank()
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,25 @@
package com.wire.android.ui.home.messagecomposer.state

import androidx.compose.foundation.text.input.TextFieldState
import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd
import androidx.compose.ui.unit.dp
import com.wire.android.config.CoroutineTestExtension
import com.wire.android.config.SnapshotExtension
import com.wire.android.util.EMPTY
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.amshove.kluent.shouldBeEqualTo
import org.junit.jupiter.api.BeforeEach
import org.amshove.kluent.shouldBeInstanceOf
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith

@OptIn(ExperimentalCoroutinesApi::class)
@ExtendWith(CoroutineTestExtension::class, SnapshotExtension::class)
class MessageCompositionInputStateHolderTest {

private lateinit var messageTextState: TextFieldState

private lateinit var state: MessageCompositionInputStateHolder

@BeforeEach
fun before() {
messageTextState = TextFieldState()
state = MessageCompositionInputStateHolder(messageTextState = messageTextState)
}

@Test
fun `when offset increases and is bigger than previous and options height, options height is updated`() = runTest {

val (state, _) = Arrangement().arrange()
// When
state.handleImeOffsetChange(
50.dp,
Expand All @@ -61,6 +53,7 @@ class MessageCompositionInputStateHolderTest {
@Test
fun `when offset decreases and showSubOptions is false, options height is updated`() = runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(previousOffset = 50.dp)

// When
Expand All @@ -78,6 +71,7 @@ class MessageCompositionInputStateHolderTest {
@Test
fun `when offset decreases to zero, showOptions and isTextExpanded are set to false`() = runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(previousOffset = 50.dp)

// When
Expand All @@ -96,6 +90,7 @@ class MessageCompositionInputStateHolderTest {
@Test
fun `when offset equals keyboard height, showSubOptions is set to false`() = runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(keyboardHeight = 30.dp)

// When
Expand All @@ -113,6 +108,7 @@ class MessageCompositionInputStateHolderTest {
@Test
fun `when offset is greater than keyboard height, keyboardHeight is updated`() = runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(keyboardHeight = 20.dp)

// When
Expand All @@ -130,6 +126,7 @@ class MessageCompositionInputStateHolderTest {
@Test
fun `when offset increases and is greater than keyboardHeight but is less than previousOffset, keyboardHeight is updated`() = runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(previousOffset = 50.dp, keyboardHeight = 20.dp)

// When
Expand All @@ -149,6 +146,7 @@ class MessageCompositionInputStateHolderTest {
fun `when offset decreases, showSubOptions is true, and actualOffset is greater than optionsHeight, values remain unchanged`() =
runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(
previousOffset = 50.dp,
keyboardHeight = 20.dp,
Expand All @@ -172,6 +170,7 @@ class MessageCompositionInputStateHolderTest {
fun `when offset decreases, showSubOptions is false, and actualOffset is greater than optionsHeight, optionsHeight is updated`() =
runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(
previousOffset = 50.dp,
keyboardHeight = 20.dp,
Expand All @@ -194,6 +193,7 @@ class MessageCompositionInputStateHolderTest {
@Test
fun `when offset is the same as previousOffset and greater than current keyboardHeight, keyboardHeight is updated`() = runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(previousOffset = 40.dp, keyboardHeight = 20.dp)

// When
Expand All @@ -212,6 +212,7 @@ class MessageCompositionInputStateHolderTest {
@Test
fun `given first keyboard appear when source equals target, then initialKeyboardHeight is set`() = runTest {
// Given
val (state, _) = Arrangement().arrange()
val imeValue = 50.dp
state.updateValuesForTesting(initialKeyboardHeight = 0.dp)

Expand All @@ -226,6 +227,7 @@ class MessageCompositionInputStateHolderTest {
fun `given extended keyboard height when attachment button is clicked, then keyboardHeight is set to initialKeyboardHeight`() =
runTest {
// Given
val (state, _) = Arrangement().arrange()
val initialKeyboardHeight = 10.dp
state.updateValuesForTesting(previousOffset = 40.dp, keyboardHeight = 20.dp, initialKeyboardHeight = initialKeyboardHeight)

Expand All @@ -241,6 +243,7 @@ class MessageCompositionInputStateHolderTest {
@Test
fun `when offset decreases but is not zero, only optionsHeight is updated`() = runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(previousOffset = 50.dp)

// When
Expand All @@ -261,6 +264,7 @@ class MessageCompositionInputStateHolderTest {
fun `when keyboard is still in a process of hiding from the previous screen after navigating, options should not be visible`() =
runTest {
// Given
val (state, _) = Arrangement().arrange()
state.updateValuesForTesting(previousOffset = 0.dp)

// When
Expand All @@ -277,6 +281,140 @@ class MessageCompositionInputStateHolderTest {
state.isTextExpanded shouldBeEqualTo false
}

@Test
fun `given empty text, when composing, then send button should be disabled`() = runTest {
// Given
val messageText = String.EMPTY
val (state, _) = Arrangement()
.withText(messageText)
.arrange()

// When
state.toComposing()

// Then
state.inputType.shouldBeInstanceOf<InputType.Composing>().let {
it.isSendButtonEnabled shouldBeEqualTo false
}
}

@Test
fun `given non-empty text but with only empty markdown, when composing, then send button should be disabled`() = runTest {
// Given
val messageText = "# " // just an example, more combinations are tested in StringUtilTest
val (state, _) = Arrangement()
.withText(messageText)
.arrange()

// When
state.toComposing()

// Then
state.inputType.shouldBeInstanceOf<InputType.Composing>().let {
it.isSendButtonEnabled shouldBeEqualTo false
}
}

@Test
fun `given non-empty text, when composing, then send button should be enabled`() = runTest {
// Given
val messageText = "text"
val (state, _) = Arrangement()
.withText(messageText)
.arrange()

// When
state.toComposing()

// Then
state.inputType.shouldBeInstanceOf<InputType.Composing>().let {
it.isSendButtonEnabled shouldBeEqualTo true
}
}

@Test
fun `given empty text, when editing, then send button should be disabled`() = runTest {
// Given
val editMessageText = "edit"
val messageText = String.EMPTY
val (state, _) = Arrangement()
.withText(messageText)
.arrange()

// When
state.toEdit(editMessageText)

// Then
state.inputType.shouldBeInstanceOf<InputType.Editing>().let {
it.isEditButtonEnabled shouldBeEqualTo false
}
}

@Test
fun `given non-empty text bit with only empty markdown, when editing, then send button should be disabled`() = runTest {
// Given
val editMessageText = "edit"
val messageText = "# " // just an example, more combinations are tested in StringUtilTest
val (state, _) = Arrangement()
.withText(messageText)
.arrange()

// When
state.toEdit(editMessageText)

// Then
state.inputType.shouldBeInstanceOf<InputType.Editing>().let {
it.isEditButtonEnabled shouldBeEqualTo false
}
}

@Test
fun `given the same text as edit message text, when editing, then send button should be disabled`() = runTest {
// Given
val editMessageText = "edit"
val (state, _) = Arrangement()
.withText(editMessageText)
.arrange()

// When
state.toEdit(editMessageText)

// Then
state.inputType.shouldBeInstanceOf<InputType.Editing>().let {
it.isEditButtonEnabled shouldBeEqualTo false
}
}

@Test
fun `given different text than edit message text, when editing, then send button should be enabled`() = runTest {
// Given
val editMessageText = "edit"
val messageText = "$editMessageText new"
val (state, _) = Arrangement()
.withText(messageText)
.arrange()

// When
state.toEdit(editMessageText)

// Then
state.inputType.shouldBeInstanceOf<InputType.Editing>().let {
it.isEditButtonEnabled shouldBeEqualTo true
}
}

class Arrangement {

private val textFieldState = TextFieldState()
private val state = MessageCompositionInputStateHolder(textFieldState)

fun withText(text: String) = apply {
textFieldState.setTextAndPlaceCursorAtEnd(text)
}

fun arrange() = state to this
}

companion object {
// I set it 0 to make tests more straight forward
val NAVIGATION_BAR_HEIGHT = 0.dp
Expand Down
Loading

0 comments on commit 60a9e35

Please sign in to comment.