Skip to content

Commit

Permalink
fix: attachment options visible when navigating after searching [WPB-…
Browse files Browse the repository at this point in the history 
…2425] (#2818)
  • Loading branch information
@saleniuk
saleniuk authored Mar 26, 2024
1 parent 7e37bcb commit e79d182
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 89 deletions.
53 changes: 27 additions & 26 deletions app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/EnabledMessageComposer.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ package com.wire.android.ui.home.messagecomposer

import android.net.Uri
import androidx.activity.compose.BackHandler
import androidx.compose.animation.animateContentSize
import androidx.compose.animation.fadeIn
import androidx.compose.animation.fadeOut
import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
Expand Down Expand Up @@ -94,7 +95,7 @@ fun EnabledMessageComposer(

LaunchedEffect(offsetY) {
with(density) {
inputStateHolder.handleOffsetChange(
inputStateHolder.handleImeOffsetChange(
offsetY.toDp(),
navBarHeight,
imeAnimationSource.toDp(),
Expand All @@ -103,10 +104,6 @@ fun EnabledMessageComposer(
}
}

LaunchedEffect(isImeVisible) {
inputStateHolder.handleIMEVisibility(isImeVisible)
}

LaunchedEffect(modalBottomSheetState.isVisible) {
if (modalBottomSheetState.isVisible) {
messageCompositionInputStateHolder.clearFocus()
Expand Down Expand Up @@ -262,31 +259,35 @@ fun EnabledMessageComposer(
onCloseRichEditingButtonClicked = additionalOptionStateHolder::toAttachmentAndAdditionalOptionsMenu,
)
}

AdditionalOptionSubMenu(
isFileSharingEnabled = messageComposerViewState.value.isFileSharingEnabled,
additionalOptionsState = additionalOptionStateHolder.additionalOptionsSubMenuState,
onRecordAudioMessageClicked = ::toAudioRecording,
onCloseAdditionalAttachment = ::toInitialAttachmentOptions,
onLocationPickerClicked = ::toLocationPicker,
onAttachmentPicked = onAttachmentPicked,
onAudioRecorded = onAudioRecorded,
onLocationPicked = onLocationPicked,
onCaptureVideoPermissionPermanentlyDenied = onCaptureVideoPermissionPermanentlyDenied,
tempWritableImageUri = tempWritableImageUri,
tempWritableVideoUri = tempWritableVideoUri,
Box(
modifier = Modifier
.height(
inputStateHolder.calculateOptionsMenuHeight(
additionalOptionStateHolder.additionalOptionsSubMenuState
)
inputStateHolder.calculateOptionsMenuHeight(additionalOptionStateHolder.additionalOptionsSubMenuState)
)
.fillMaxWidth()
.background(
colorsScheme().messageComposerBackgroundColor
.background(colorsScheme().messageComposerBackgroundColor)
) {
androidx.compose.animation.AnimatedVisibility(
visible = inputStateHolder.subOptionsVisible,
enter = fadeIn(),
exit = fadeOut(),
) {
AdditionalOptionSubMenu(
isFileSharingEnabled = messageComposerViewState.value.isFileSharingEnabled,
additionalOptionsState = additionalOptionStateHolder.additionalOptionsSubMenuState,
onRecordAudioMessageClicked = ::toAudioRecording,
onCloseAdditionalAttachment = ::toInitialAttachmentOptions,
onLocationPickerClicked = ::toLocationPicker,
onAttachmentPicked = onAttachmentPicked,
onAudioRecorded = onAudioRecorded,
onLocationPicked = onLocationPicked,
onCaptureVideoPermissionPermanentlyDenied = onCaptureVideoPermissionPermanentlyDenied,
tempWritableImageUri = tempWritableImageUri,
tempWritableVideoUri = tempWritableVideoUri,
modifier = Modifier.fillMaxSize()
)
.animateContentSize()
)
}
}
}
}
}
Expand Down
24 changes: 11 additions & 13 deletions ...tlin/com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,7 @@ class MessageCompositionInputStateHolder(
)
)

fun handleIMEVisibility(isImeVisible: Boolean) {
if (isImeVisible) {
optionsVisible = true
} else if (!subOptionsVisible) {
optionsVisible = false
}
}

fun handleOffsetChange(offset: Dp, navBarHeight: Dp, source: Dp, target: Dp) {
fun handleImeOffsetChange(offset: Dp, navBarHeight: Dp, source: Dp, target: Dp) {
val actualOffset = max(offset - navBarHeight, 0.dp)

// this check secures that if some additional space will be added to keyboard
Expand All @@ -104,10 +96,16 @@ class MessageCompositionInputStateHolder(
}

if (previousOffset < actualOffset) {
optionsVisible = true
if (!subOptionsVisible || optionsHeight <= actualOffset) {
optionsHeight = actualOffset
subOptionsVisible = false

// only if the real goal of this ime offset increase is to really open the keyboard
// otherwise it can mean the keyboard is still in a process of hiding from the previous screen and ultimately won't be shown
// in this case we don't want to show and hide the options for a short time as it will only make unwanted blink effect
if (target > 0.dp) {
optionsVisible = true
if (!subOptionsVisible || optionsHeight <= actualOffset) {
optionsHeight = actualOffset
subOptionsVisible = false
}
}
} else if (previousOffset > actualOffset) {
if (!subOptionsVisible) {
Expand Down
81 changes: 31 additions & 50 deletions .../com/wire/android/ui/home/messagecomposer/state/MessageCompositionInputStateHolderTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,48 +46,10 @@ class MessageCompositionInputStateHolderTest {
)
}

@Test
fun `when IME is visible, showOptions should be set to true`() {
// Given
val isImeVisible = true

// When
state.handleIMEVisibility(isImeVisible)

// Then
state.optionsVisible shouldBeEqualTo true
}

@Test
fun `when IME is hidden and showSubOptions is true, showOptions remains unchanged`() {
// Given
val isImeVisible = false
state.updateValuesForTesting(showSubOptions = true, showOptions = false)

// When
state.handleIMEVisibility(isImeVisible)

// Then
state.optionsVisible shouldBeEqualTo false
}

@Test
fun `when IME is hidden and showSubOptions is false, showOptions should be set to false`() {
// Given
val isImeVisible = false
state.updateValuesForTesting(showSubOptions = false)

// When
state.handleIMEVisibility(isImeVisible)

// Then
state.optionsVisible shouldBeEqualTo false
}

@Test
fun `when offset increases and is bigger than previous and options height, options height is updated`() {
// When
state.handleOffsetChange(
state.handleImeOffsetChange(
50.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -105,7 +67,7 @@ class MessageCompositionInputStateHolderTest {
state.updateValuesForTesting(previousOffset = 50.dp)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
20.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -122,7 +84,7 @@ class MessageCompositionInputStateHolderTest {
state.updateValuesForTesting(previousOffset = 50.dp)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
0.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -140,7 +102,7 @@ class MessageCompositionInputStateHolderTest {
state.updateValuesForTesting(keyboardHeight = 30.dp)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
30.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -157,7 +119,7 @@ class MessageCompositionInputStateHolderTest {
state.updateValuesForTesting(keyboardHeight = 20.dp)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
30.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -174,7 +136,7 @@ class MessageCompositionInputStateHolderTest {
state.updateValuesForTesting(previousOffset = 50.dp, keyboardHeight = 20.dp)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
30.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -197,7 +159,7 @@ class MessageCompositionInputStateHolderTest {
)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
30.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -219,7 +181,7 @@ class MessageCompositionInputStateHolderTest {
)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
30.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -236,7 +198,7 @@ class MessageCompositionInputStateHolderTest {
state.updateValuesForTesting(previousOffset = 40.dp, keyboardHeight = 20.dp)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
40.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -255,7 +217,7 @@ class MessageCompositionInputStateHolderTest {
state.updateValuesForTesting(initialKeyboardHeight = 0.dp)

// When
state.handleOffsetChange(20.dp, NAVIGATION_BAR_HEIGHT, source = imeValue, target = imeValue)
state.handleImeOffsetChange(20.dp, NAVIGATION_BAR_HEIGHT, source = imeValue, target = imeValue)

// Then
state.initialKeyboardHeight shouldBeEqualTo imeValue
Expand All @@ -269,7 +231,7 @@ class MessageCompositionInputStateHolderTest {

// When
state.showOptions()
state.handleOffsetChange(0.dp, NAVIGATION_BAR_HEIGHT, source = TARGET, target = SOURCE)
state.handleImeOffsetChange(0.dp, NAVIGATION_BAR_HEIGHT, source = TARGET, target = SOURCE)

// Then
state.keyboardHeight shouldBeEqualTo 20.dp
Expand All @@ -282,7 +244,7 @@ class MessageCompositionInputStateHolderTest {
state.updateValuesForTesting(previousOffset = 50.dp)

// When
state.handleOffsetChange(
state.handleImeOffsetChange(
10.dp,
NAVIGATION_BAR_HEIGHT,
SOURCE,
Expand All @@ -295,6 +257,25 @@ class MessageCompositionInputStateHolderTest {
state.isTextExpanded shouldBeEqualTo false
}

@Test
fun `when keyboard is still in a process of hiding from the previous screen after navigating, options should not be visible`() {
// Given
state.updateValuesForTesting(previousOffset = 0.dp)

// When
state.handleImeOffsetChange(
offset = 40.dp,
navBarHeight = NAVIGATION_BAR_HEIGHT,
source = 50.dp,
target = 0.dp
)

// Then
state.optionsHeight shouldBeEqualTo 0.dp
state.optionsVisible shouldBeEqualTo false
state.isTextExpanded shouldBeEqualTo false
}

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

0 comments on commit e79d182

Please sign in to comment.