Merge pull request #7798 from vector-im/feature/ons/fix_device_manager_ui

Device Manager UI review fixes (PSG-1102)
This commit is contained in:
Onuray Sahin 2022-12-19 18:03:12 +03:00 committed by GitHub
commit e95380ac9e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 36 additions and 37 deletions

1
changelog.d/7798.bugfix Normal file
View File

@ -0,0 +1 @@
Device Manager UI review fixes

View File

@ -3338,7 +3338,7 @@
<item quantity="one">Consider signing out from old sessions (%1$d day or more) that you dont use anymore.</item> <item quantity="one">Consider signing out from old sessions (%1$d day or more) that you dont use anymore.</item>
<item quantity="other">Consider signing out from old sessions (%1$d days or more) that you dont use anymore.</item> <item quantity="other">Consider signing out from old sessions (%1$d days or more) that you dont use anymore.</item>
</plurals> </plurals>
<string name="device_manager_current_session_title">Current Session</string> <string name="device_manager_current_session_title">Current session</string>
<string name="device_manager_session_title">Session</string> <string name="device_manager_session_title">Session</string>
<string name="device_manager_device_title">Device</string> <string name="device_manager_device_title">Device</string>
<!-- Examples: Last activity Yesterday at 6PM, Last activity Aug 31 at 5:47PM --> <!-- Examples: Last activity Yesterday at 6PM, Last activity Aug 31 at 5:47PM -->

View File

@ -223,7 +223,6 @@ class VectorSettingsDevicesFragment :
override fun onViewAllClicked() { override fun onViewAllClicked() {
viewNavigator.navigateToOtherSessions( viewNavigator.navigateToOtherSessions(
requireActivity(), requireActivity(),
R.string.device_manager_header_section_security_recommendations_title,
DeviceManagerFilterType.UNVERIFIED, DeviceManagerFilterType.UNVERIFIED,
excludeCurrentDevice = true excludeCurrentDevice = true
) )
@ -233,7 +232,6 @@ class VectorSettingsDevicesFragment :
override fun onViewAllClicked() { override fun onViewAllClicked() {
viewNavigator.navigateToOtherSessions( viewNavigator.navigateToOtherSessions(
requireActivity(), requireActivity(),
R.string.device_manager_header_section_security_recommendations_title,
DeviceManagerFilterType.INACTIVE, DeviceManagerFilterType.INACTIVE,
excludeCurrentDevice = true excludeCurrentDevice = true
) )
@ -447,7 +445,6 @@ class VectorSettingsDevicesFragment :
override fun onViewAllOtherSessionsClicked() { override fun onViewAllOtherSessionsClicked() {
viewNavigator.navigateToOtherSessions( viewNavigator.navigateToOtherSessions(
context = requireActivity(), context = requireActivity(),
titleResourceId = R.string.device_manager_sessions_other_title,
defaultFilter = DeviceManagerFilterType.ALL_SESSIONS, defaultFilter = DeviceManagerFilterType.ALL_SESSIONS,
excludeCurrentDevice = true excludeCurrentDevice = true
) )

View File

@ -31,12 +31,11 @@ class VectorSettingsDevicesViewNavigator @Inject constructor() {
fun navigateToOtherSessions( fun navigateToOtherSessions(
context: Context, context: Context,
titleResourceId: Int,
defaultFilter: DeviceManagerFilterType, defaultFilter: DeviceManagerFilterType,
excludeCurrentDevice: Boolean, excludeCurrentDevice: Boolean,
) { ) {
context.startActivity( context.startActivity(
OtherSessionsActivity.newIntent(context, titleResourceId, defaultFilter, excludeCurrentDevice) OtherSessionsActivity.newIntent(context, defaultFilter, excludeCurrentDevice)
) )
} }

View File

@ -27,7 +27,7 @@ import im.vector.app.core.epoxy.VectorEpoxyHolder
import im.vector.app.core.epoxy.VectorEpoxyModel import im.vector.app.core.epoxy.VectorEpoxyModel
import im.vector.app.core.utils.DimensionConverter import im.vector.app.core.utils.DimensionConverter
private const val EXTRA_TOP_MARGIN_DP = 48 private const val EXTRA_TOP_MARGIN_DP = 32
@EpoxyModelClass @EpoxyModelClass
abstract class SessionDetailsHeaderItem : VectorEpoxyModel<SessionDetailsHeaderItem.Holder>(R.layout.item_session_details_header) { abstract class SessionDetailsHeaderItem : VectorEpoxyModel<SessionDetailsHeaderItem.Holder>(R.layout.item_session_details_header) {

View File

@ -20,7 +20,6 @@ import android.content.Context
import android.content.Intent import android.content.Intent
import android.os.Bundle import android.os.Bundle
import android.view.View import android.view.View
import androidx.annotation.StringRes
import com.airbnb.mvrx.Mavericks import com.airbnb.mvrx.Mavericks
import dagger.hilt.android.AndroidEntryPoint import dagger.hilt.android.AndroidEntryPoint
import im.vector.app.core.extensions.addFragment import im.vector.app.core.extensions.addFragment
@ -48,13 +47,11 @@ class OtherSessionsActivity : SimpleFragmentActivity() {
companion object { companion object {
fun newIntent( fun newIntent(
context: Context, context: Context,
@StringRes
titleResourceId: Int,
defaultFilter: DeviceManagerFilterType, defaultFilter: DeviceManagerFilterType,
excludeCurrentDevice: Boolean, excludeCurrentDevice: Boolean,
): Intent { ): Intent {
return Intent(context, OtherSessionsActivity::class.java).apply { return Intent(context, OtherSessionsActivity::class.java).apply {
putExtra(Mavericks.KEY_ARG, OtherSessionsArgs(titleResourceId, defaultFilter, excludeCurrentDevice)) putExtra(Mavericks.KEY_ARG, OtherSessionsArgs(defaultFilter, excludeCurrentDevice))
} }
} }
} }

View File

@ -17,14 +17,11 @@
package im.vector.app.features.settings.devices.v2.othersessions package im.vector.app.features.settings.devices.v2.othersessions
import android.os.Parcelable import android.os.Parcelable
import androidx.annotation.StringRes
import im.vector.app.features.settings.devices.v2.filter.DeviceManagerFilterType import im.vector.app.features.settings.devices.v2.filter.DeviceManagerFilterType
import kotlinx.parcelize.Parcelize import kotlinx.parcelize.Parcelize
@Parcelize @Parcelize
data class OtherSessionsArgs( data class OtherSessionsArgs(
@StringRes
val titleResourceId: Int,
val defaultFilter: DeviceManagerFilterType, val defaultFilter: DeviceManagerFilterType,
val excludeCurrentDevice: Boolean, val excludeCurrentDevice: Boolean,
) : Parcelable ) : Parcelable

View File

@ -182,7 +182,9 @@ class OtherSessionsFragment :
override fun onViewCreated(view: View, savedInstanceState: Bundle?) { override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState) super.onViewCreated(view, savedInstanceState)
setupToolbar(views.otherSessionsToolbar).setTitle(args.titleResourceId).allowBack() setupToolbar(views.otherSessionsToolbar)
.setTitle(R.string.device_manager_sessions_other_title)
.allowBack()
observeViewEvents() observeViewEvents()
initFilterView() initFilterView()
} }
@ -251,7 +253,7 @@ class OtherSessionsFragment :
val selection = devices.count { it.isSelected } val selection = devices.count { it.isSelected }
stringProvider.getQuantityString(R.plurals.x_selected, selection, selection) stringProvider.getQuantityString(R.plurals.x_selected, selection, selection)
} else { } else {
getString(args.titleResourceId) getString(R.string.device_manager_sessions_other_title)
} }
toolbar?.title = title toolbar?.title = title
} }

View File

@ -69,7 +69,7 @@
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginTop="?attr/actionBarSize" android:layout_marginTop="?attr/actionBarSize"
android:layout_marginBottom="32dp" android:layout_marginBottom="16dp"
app:layout_collapseMode="parallax" app:layout_collapseMode="parallax"
app:sessionsListHeaderDescription="@string/device_manager_sessions_other_description" app:sessionsListHeaderDescription="@string/device_manager_sessions_other_description"
app:sessionsListHeaderHasLearnMoreLink="false" app:sessionsListHeaderHasLearnMoreLink="false"
@ -81,7 +81,7 @@
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginStart="16dp" android:layout_marginStart="16dp"
android:layout_marginTop="?attr/actionBarSize" android:layout_marginTop="?attr/actionBarSize"
android:layout_marginBottom="32dp" android:layout_marginBottom="16dp"
android:paddingTop="20dp" android:paddingTop="20dp"
android:visibility="gone" android:visibility="gone"
app:layout_collapseMode="parallax" app:layout_collapseMode="parallax"

View File

@ -47,6 +47,7 @@
android:layout_width="0dp" android:layout_width="0dp"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginHorizontal="8dp" android:layout_marginHorizontal="8dp"
android:layout_marginTop="4dp"
android:text="@string/device_manager_session_overview_signout" android:text="@string/device_manager_session_overview_signout"
app:layout_constraintEnd_toEndOf="parent" app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintHorizontal_bias="0" app:layout_constraintHorizontal_bias="0"

View File

@ -75,7 +75,7 @@
android:layout_width="0dp" android:layout_width="0dp"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginHorizontal="16dp" android:layout_marginHorizontal="16dp"
android:layout_marginVertical="16dp" android:layout_marginVertical="4dp"
app:layout_constraintEnd_toEndOf="parent" app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent" app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/deviceListHeaderCurrentSession" /> app:layout_constraintTop_toBottomOf="@id/deviceListHeaderCurrentSession" />

View File

@ -5,9 +5,15 @@
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:foreground="?selectableItemBackground" android:foreground="?selectableItemBackground"
android:paddingHorizontal="8dp"
android:paddingTop="8dp"> android:paddingTop="8dp">
<androidx.constraintlayout.widget.Guideline
android:id="@+id/startGuideline"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:orientation="vertical"
app:layout_constraintGuide_begin="8dp" />
<View <View
android:id="@+id/otherSessionItemBackground" android:id="@+id/otherSessionItemBackground"
android:layout_width="0dp" android:layout_width="0dp"
@ -29,7 +35,7 @@
android:contentDescription="@string/a11y_device_manager_device_type_mobile" android:contentDescription="@string/a11y_device_manager_device_type_mobile"
android:padding="8dp" android:padding="8dp"
app:layout_constraintBottom_toBottomOf="@id/otherSessionItemBackground" app:layout_constraintBottom_toBottomOf="@id/otherSessionItemBackground"
app:layout_constraintStart_toStartOf="@id/otherSessionItemBackground" app:layout_constraintStart_toStartOf="@id/startGuideline"
app:layout_constraintTop_toTopOf="@id/otherSessionItemBackground" app:layout_constraintTop_toTopOf="@id/otherSessionItemBackground"
tools:src="@drawable/ic_device_type_mobile" /> tools:src="@drawable/ic_device_type_mobile" />
@ -52,8 +58,8 @@
android:layout_width="0dp" android:layout_width="0dp"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginStart="16dp" android:layout_marginStart="16dp"
android:layout_marginEnd="8dp"
android:layout_marginTop="8dp" android:layout_marginTop="8dp"
android:layout_marginEnd="8dp"
android:ellipsize="end" android:ellipsize="end"
android:lines="1" android:lines="1"
app:layout_constraintEnd_toEndOf="parent" app:layout_constraintEnd_toEndOf="parent"
@ -89,7 +95,7 @@
android:id="@+id/otherSessionSeparator" android:id="@+id/otherSessionSeparator"
android:layout_width="0dp" android:layout_width="0dp"
android:layout_height="1dp" android:layout_height="1dp"
android:layout_marginTop="8dp" android:layout_marginTop="16dp"
android:background="?vctr_content_quinary" android:background="?vctr_content_quinary"
app:layout_constraintEnd_toEndOf="parent" app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="@id/otherSessionNameTextView" app:layout_constraintStart_toStartOf="@id/otherSessionNameTextView"

View File

@ -9,7 +9,7 @@
android:id="@+id/sessionDetailsContentTitle" android:id="@+id/sessionDetailsContentTitle"
style="@style/TextAppearance.Vector.Body.DevicesManagement" style="@style/TextAppearance.Vector.Body.DevicesManagement"
android:layout_width="0dp" android:layout_width="0dp"
android:layout_height="wrap_content" android:layout_height="0dp"
android:layout_marginStart="@dimen/layout_horizontal_margin" android:layout_marginStart="@dimen/layout_horizontal_margin"
app:layout_constraintBottom_toTopOf="@id/sessionDetailsContentDivider" app:layout_constraintBottom_toTopOf="@id/sessionDetailsContentDivider"
app:layout_constraintEnd_toStartOf="@id/sessionDetailsContentDescription" app:layout_constraintEnd_toStartOf="@id/sessionDetailsContentDescription"
@ -22,14 +22,14 @@
style="@style/TextAppearance.Vector.Body" style="@style/TextAppearance.Vector.Body"
android:layout_width="0dp" android:layout_width="0dp"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginStart="8dp" android:layout_marginStart="12dp"
android:layout_marginEnd="@dimen/layout_horizontal_margin" android:layout_marginEnd="@dimen/layout_horizontal_margin"
android:gravity="end" android:gravity="end"
app:layout_constraintBottom_toTopOf="@id/sessionDetailsContentDivider" app:layout_constraintBottom_toTopOf="@id/sessionDetailsContentDivider"
app:layout_constraintEnd_toEndOf="parent" app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@id/sessionDetailsContentTitle" app:layout_constraintStart_toEndOf="@id/sessionDetailsContentTitle"
app:layout_constraintTop_toTopOf="@id/sessionDetailsContentTop" app:layout_constraintTop_toTopOf="@id/sessionDetailsContentTop"
tools:text="Element Web: Firefox" /> tools:text="app.element.io: Firefox on macOS" />
<View <View
android:id="@+id/sessionDetailsContentDivider" android:id="@+id/sessionDetailsContentDivider"

View File

@ -3,7 +3,8 @@
xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools" xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="match_parent"> android:layout_height="match_parent"
android:paddingBottom="8dp">
<androidx.recyclerview.widget.RecyclerView <androidx.recyclerview.widget.RecyclerView
android:id="@+id/otherSessionsRecyclerView" android:id="@+id/otherSessionsRecyclerView"
@ -19,8 +20,9 @@
style="@style/Widget.Vector.Button.Text" style="@style/Widget.Vector.Button.Text"
android:layout_width="wrap_content" android:layout_width="wrap_content"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginStart="72dp"
android:layout_marginTop="4dp"
android:padding="0dp" android:padding="0dp"
android:layout_marginStart="16dp"
app:layout_constraintStart_toStartOf="@id/otherSessionsRecyclerView" app:layout_constraintStart_toStartOf="@id/otherSessionsRecyclerView"
app:layout_constraintTop_toBottomOf="@id/otherSessionsRecyclerView" app:layout_constraintTop_toBottomOf="@id/otherSessionsRecyclerView"
tools:text="@string/device_manager_other_sessions_view_all" /> tools:text="@string/device_manager_other_sessions_view_all" />

View File

@ -6,7 +6,7 @@
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:background="@drawable/bg_current_session" android:background="@drawable/bg_current_session"
android:paddingHorizontal="24dp" android:paddingHorizontal="24dp"
android:paddingBottom="16dp"> android:paddingBottom="8dp">
<ImageView <ImageView
android:id="@+id/sessionInfoDeviceTypeImageView" android:id="@+id/sessionInfoDeviceTypeImageView"

View File

@ -24,7 +24,7 @@
android:layout_width="0dp" android:layout_width="0dp"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginHorizontal="@dimen/layout_horizontal_margin" android:layout_marginHorizontal="@dimen/layout_horizontal_margin"
android:layout_marginTop="18.5dp" android:layout_marginTop="@dimen/layout_vertical_margin"
app:layout_constraintEnd_toEndOf="parent" app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent" app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/sessions_list_header_title" app:layout_constraintTop_toBottomOf="@id/sessions_list_header_title"

View File

@ -31,7 +31,6 @@ import org.junit.Before
import org.junit.Test import org.junit.Test
private const val A_SESSION_ID = "session_id" private const val A_SESSION_ID = "session_id"
private const val A_TITLE_RESOURCE_ID = 1234
private val A_DEFAULT_FILTER = DeviceManagerFilterType.INACTIVE private val A_DEFAULT_FILTER = DeviceManagerFilterType.INACTIVE
class VectorSettingsDevicesViewNavigatorTest { class VectorSettingsDevicesViewNavigatorTest {
@ -67,11 +66,11 @@ class VectorSettingsDevicesViewNavigatorTest {
@Test @Test
fun `given an intent when navigating to other sessions list then it starts the correct activity`() { fun `given an intent when navigating to other sessions list then it starts the correct activity`() {
// Given // Given
val intent = givenIntentForOtherSessions(A_TITLE_RESOURCE_ID, A_DEFAULT_FILTER, true) val intent = givenIntentForOtherSessions(A_DEFAULT_FILTER, true)
context.givenStartActivity(intent) context.givenStartActivity(intent)
// When // When
vectorSettingsDevicesViewNavigator.navigateToOtherSessions(context.instance, A_TITLE_RESOURCE_ID, A_DEFAULT_FILTER, true) vectorSettingsDevicesViewNavigator.navigateToOtherSessions(context.instance, A_DEFAULT_FILTER, true)
// Then // Then
context.verifyStartActivity(intent) context.verifyStartActivity(intent)
@ -96,9 +95,9 @@ class VectorSettingsDevicesViewNavigatorTest {
return intent return intent
} }
private fun givenIntentForOtherSessions(titleResourceId: Int, defaultFilter: DeviceManagerFilterType, excludeCurrentDevice: Boolean): Intent { private fun givenIntentForOtherSessions(defaultFilter: DeviceManagerFilterType, excludeCurrentDevice: Boolean): Intent {
val intent = mockk<Intent>() val intent = mockk<Intent>()
every { OtherSessionsActivity.newIntent(context.instance, titleResourceId, defaultFilter, excludeCurrentDevice) } returns intent every { OtherSessionsActivity.newIntent(context.instance, defaultFilter, excludeCurrentDevice) } returns intent
return intent return intent
} }

View File

@ -47,7 +47,6 @@ import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.matrix.android.sdk.api.session.uia.DefaultBaseAuth import org.matrix.android.sdk.api.session.uia.DefaultBaseAuth
private const val A_TITLE_RES_ID = 1
private const val A_DEVICE_ID_1 = "device-id-1" private const val A_DEVICE_ID_1 = "device-id-1"
private const val A_DEVICE_ID_2 = "device-id-2" private const val A_DEVICE_ID_2 = "device-id-2"
private const val A_PASSWORD = "password" private const val A_PASSWORD = "password"
@ -58,7 +57,6 @@ class OtherSessionsViewModelTest {
val mavericksTestRule = MavericksTestRule(testDispatcher = testDispatcher) val mavericksTestRule = MavericksTestRule(testDispatcher = testDispatcher)
private val defaultArgs = OtherSessionsArgs( private val defaultArgs = OtherSessionsArgs(
titleResourceId = A_TITLE_RES_ID,
defaultFilter = DeviceManagerFilterType.ALL_SESSIONS, defaultFilter = DeviceManagerFilterType.ALL_SESSIONS,
excludeCurrentDevice = false, excludeCurrentDevice = false,
) )