Minor optimizations and changes to improve clarity

- Use `merge` tag for message_details_actions to avoid nesting ConstraintLayout
- Rename DismissActionSheet action and it's field shallDismissBackingActivity
- Return whe messageActionView is null instead of treating it as optional

MAILAND-1533
This commit is contained in:
Marino Meneghel 2021-07-14 15:08:29 +02:00
parent 6d8ca288f2
commit 5ac6d000ab
8 changed files with 37 additions and 29 deletions

View File

@ -255,8 +255,9 @@ internal class MessageDetailsAdapter(
messageHtmlWithQuotedHistory: String?,
webView: WebView
) {
val messageActionsView: MessageDetailsActionsView? =
itemView.messageWebViewContainer.findViewById(ITEM_MESSAGE_ACTIONS_LAYOUT_ID)
val messageActionsView: MessageDetailsActionsView =
itemView.messageWebViewContainer.findViewById(ITEM_MESSAGE_ACTIONS_LAYOUT_ID) ?: return
val replyMode = if (message.toList.size + message.ccList.size > 1) {
MessageDetailsActionsView.ReplyMode.REPLY_ALL
} else {
@ -267,14 +268,14 @@ internal class MessageDetailsAdapter(
messageHtmlWithQuotedHistory.isNullOrEmpty(),
message.isDraft()
)
messageActionsView?.bind(uiModel)
messageActionsView.bind(uiModel)
messageActionsView?.onShowHistoryClicked { showHistoryButton ->
messageActionsView.onShowHistoryClicked { showHistoryButton ->
loadHtmlDataIntoWebView(webView, messageHtmlWithQuotedHistory)
showHistoryButton.isVisible = false
}
messageActionsView?.onReplyClicked { onReplyMessageClicked(message) }
messageActionsView?.onMoreActionsClicked { onMoreMessageActionsClicked(message) }
messageActionsView.onReplyClicked { onReplyMessageClicked(message) }
messageActionsView.onMoreActionsClicked { onMoreMessageActionsClicked(message) }
}
private fun loadHtmlDataIntoWebView(webView: WebView, htmlContent: String?) {

View File

@ -45,8 +45,7 @@ class MessageDetailsActionsView @JvmOverloads constructor(
init {
val binding = MessageDetailsActionsBinding.inflate(
LayoutInflater.from(context),
this,
true
this
)
showHistoryButton = binding.detailsButtonShowHistory
replyButton = binding.detailsButtonReply
@ -71,7 +70,7 @@ class MessageDetailsActionsView @JvmOverloads constructor(
moreActionsButton.setOnClickListener { callback(it) }
}
class UiModel(
data class UiModel(
val replyMode: ReplyMode,
val hideShowHistory: Boolean,
val hideAllActions: Boolean
@ -81,4 +80,4 @@ class MessageDetailsActionsView @JvmOverloads constructor(
REPLY(R.drawable.ic_reply),
REPLY_ALL(R.drawable.ic_reply_all),
}
}
}

View File

@ -340,7 +340,8 @@ class MessageActionSheet : BottomSheetDialogFragment() {
is MessageActionSheetAction.ShowMessageHeaders -> showMessageHeaders(sheetAction.messageHeaders)
is MessageActionSheetAction.ChangeStarredStatus -> dismiss()
is MessageActionSheetAction.Delete -> dismiss()
is MessageActionSheetAction.ShouldDismiss -> handleDismissBehavior(sheetAction.dismissBackingActivity)
is MessageActionSheetAction.DismissActionSheet ->
handleDismissBehavior(sheetAction.shallDismissBackingActivity)
else -> Timber.v("unhandled action $sheetAction")
}
}

View File

@ -41,5 +41,5 @@ sealed class MessageActionSheetAction {
object Delete : MessageActionSheetAction()
data class ShouldDismiss(val dismissBackingActivity: Boolean) : MessageActionSheetAction()
data class DismissActionSheet(val shallDismissBackingActivity: Boolean) : MessageActionSheetAction()
}

View File

@ -207,7 +207,7 @@ class MessageActionSheetViewModel @Inject constructor(
}
}.invokeOnCompletion {
val dismissBackingActivity = !isApplyingActionToMessageWithinAConversation()
actionsMutableFlow.value = MessageActionSheetAction.ShouldDismiss(dismissBackingActivity)
actionsMutableFlow.value = MessageActionSheetAction.DismissActionSheet(dismissBackingActivity)
}
}
@ -233,7 +233,7 @@ class MessageActionSheetViewModel @Inject constructor(
}
}.invokeOnCompletion {
val dismissBackingActivity = !isApplyingActionToMessageWithinAConversation()
actionsMutableFlow.value = MessageActionSheetAction.ShouldDismiss(dismissBackingActivity)
actionsMutableFlow.value = MessageActionSheetAction.DismissActionSheet(dismissBackingActivity)
}
}
@ -269,7 +269,7 @@ class MessageActionSheetViewModel @Inject constructor(
}
}.invokeOnCompletion {
val dismissBackingActivity = !isApplyingActionToMessageWithinAConversation()
actionsMutableFlow.value = MessageActionSheetAction.ShouldDismiss(dismissBackingActivity)
actionsMutableFlow.value = MessageActionSheetAction.DismissActionSheet(dismissBackingActivity)
}
}

View File

@ -37,4 +37,4 @@ enum class ActionSheetTarget {
* to act on one specific message within a conversation (that has more than one message)
*/
MESSAGE_ITEM_WITHIN_CONVERSATION_DETAIL_SCREEN
}
}

View File

@ -17,8 +17,10 @@
~ along with ProtonMail. If not, see https://www.gnu.org/licenses/.
-->
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
<merge xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools"
tools:parentTag="androidx.constraintlayout.widget.ConstraintLayout"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:padding="@dimen/padding_m">
@ -26,8 +28,10 @@
<Button
android:id="@+id/details_button_show_history"
style="@style/ProtonButton.Borderless.Text"
android:layout_width="0dp"
android:layout_height="@dimen/message_details_actions_button_size"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:padding="@dimen/padding_m"
android:contentDescription="@string/details_show_history"
android:text="@string/details_show_history"
android:visibility="gone"
app:layout_constraintStart_toStartOf="parent"
@ -38,8 +42,10 @@
<ImageButton
android:id="@+id/details_button_reply"
style="@style/ProtonImage.ImageButton"
android:layout_width="@dimen/message_details_actions_button_size"
android:layout_height="@dimen/message_details_actions_button_size"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:padding="@dimen/padding_m"
android:contentDescription="@string/reply"
android:background="?selectableItemBackgroundBorderless"
android:src="@drawable/ic_reply"
app:layout_constraintEnd_toStartOf="@id/details_button_more_actions"
@ -50,8 +56,10 @@
<ImageButton
android:id="@+id/details_button_more_actions"
style="@style/ProtonImage.ImageButton"
android:layout_width="@dimen/message_details_actions_button_size"
android:layout_height="@dimen/message_details_actions_button_size"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:padding="@dimen/padding_m"
android:contentDescription="@string/more"
android:background="?selectableItemBackgroundBorderless"
android:src="@drawable/ic_three_dots_horizontal"
app:layout_constraintEnd_toEndOf="parent"
@ -59,5 +67,4 @@
app:layout_constraintBottom_toBottomOf="parent"
app:tint="@color/cornflower_blue" />
</androidx.constraintlayout.widget.ConstraintLayout>
</merge>

View File

@ -203,7 +203,7 @@ class MessageActionSheetViewModelTest : ArchTest, CoroutinesTest {
fun verifyMoveToInboxEmitsShouldDismissActionThatDoesNotDismissBackingActivityWhenBottomActionSheetTargetIsConversationDetails() {
// given
val messageId = "messageId1"
val expected = MessageActionSheetAction.ShouldDismiss(false)
val expected = MessageActionSheetAction.DismissActionSheet(false)
every { conversationModeEnabled(any()) } returns true
every { moveMessagesToFolder.invoke(any(), any(), any()) } just Runs
every {
@ -225,7 +225,7 @@ class MessageActionSheetViewModelTest : ArchTest, CoroutinesTest {
fun verifyMoveToInboxEmitsShouldDismissActionThatDismissesBackingActivityWhenBottomActionSheetTargetIsMessageDetails() {
// given
val messageId = "messageId2"
val expected = MessageActionSheetAction.ShouldDismiss(true)
val expected = MessageActionSheetAction.DismissActionSheet(true)
every { conversationModeEnabled(any()) } returns false
every { moveMessagesToFolder.invoke(any(), any(), any()) } just Runs
every {
@ -272,7 +272,7 @@ class MessageActionSheetViewModelTest : ArchTest, CoroutinesTest {
fun verifyMarkReadCallsChangeConversationReadStatusWhenConversationModeIsEnabledAndAConversationIsBeingMarkedAsRead() {
// given
val conversationId = "conversationId02"
val expected = MessageActionSheetAction.ShouldDismiss(true)
val expected = MessageActionSheetAction.DismissActionSheet(true)
val userId = UserId("userId02")
val markReadAction = ChangeConversationsReadStatus.Action.ACTION_MARK_READ
val location = Constants.MessageLocationType.ALL_MAIL
@ -321,7 +321,7 @@ class MessageActionSheetViewModelTest : ArchTest, CoroutinesTest {
fun verifyMarkUnreadCallsMessageRepositoryToMarkUnreadWhenConversationIsEnabledAndActionIsBeingAppliedToASpecificMessageInAConversation() {
// given
val messageId = "messageId4"
val expected = MessageActionSheetAction.ShouldDismiss(false)
val expected = MessageActionSheetAction.DismissActionSheet(false)
every { conversationModeEnabled(any()) } returns true
every { messageRepository.markUnRead(listOf(messageId)) } just Runs
every {