[Device management] Update the unknown verification status icon (PSG-824) #7361

mnaturel · 2022-10-13T15:54:01Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Update the UI to handle verification status of other sessions when the current one is not verified.

Motivation and context

Closes #7327

Screenshots / GIFs

Tests

Signin to a new session
Enable the lab flag for the new session manager
Go to Settings -> Security & Privacy -> Show all sessions
Check the unverified sessions section does not show in security recommendations
Check the other session list items display a correct UI
Go to the session overview of another session
Check the UI is correct
Verify the current session
Check the unverified sessions section shows in security recommendations if there are any unverified sessions
Check the other session list items display a correct UI
Go to the session overview of another session
Check the UI is correct

Tested devices

Physical
Emulator
OS version(s): Android 11

Checklist

Changes has been tested on an Android device or Android emulator with API 21
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR
If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()

bmarty

LGTM, just 2 remarks.

bmarty · 2022-10-14T08:12:25Z

...or/src/main/java/im/vector/app/features/settings/devices/v2/VectorSettingsDevicesFragment.kt

        if (unverifiedSessionsCount == 0 && inactiveSessionsCount == 0) {
            hideSecurityRecommendations()
        } else {
            views.deviceListHeaderSectionSecurityRecommendations.isVisible = true
            views.deviceListSecurityRecommendationsDivider.isVisible = true
-            views.deviceListUnverifiedSessionsRecommendation.isVisible = unverifiedSessionsCount > 0
+
+            views.deviceListUnverifiedSessionsRecommendation.isVisible = unverifiedSessionsCount > 0 && isCurrentSessionVerified


Could be nice to start using Paparazzi to test the different state of the View.
There is an example in PaparazziExampleScreenshotTest and a doc screenshot_testing.md is the project.

Yes, we will plan it. For now, the screens are still subject to change. Once the feature is stable, we will add some tests.

bmarty · 2022-10-14T08:17:57Z

vector/src/main/res/drawable/ic_shield_unknown.xml

+  <path
+      android:strokeWidth="1"
+      android:pathData="M12.008,23.487C12.005,23.487 12.002,23.488 12,23.489C11.998,23.488 11.995,23.487 11.992,23.487C11.92,23.471 11.813,23.445 11.675,23.409C11.399,23.337 11.002,23.223 10.523,23.058C9.565,22.725 8.292,22.186 7.022,21.361C4.49,19.715 2,16.954 2,12.405V3.45L12,0.521L22,3.45V12.405C22,16.954 19.51,19.715 16.978,21.361C15.708,22.186 14.435,22.725 13.477,23.058C12.998,23.223 12.601,23.337 12.325,23.409C12.187,23.445 12.08,23.471 12.008,23.487Z"
+      android:fillColor="#737D8C"


Maybe define and use a new color for this new shield as per https://github.com/vector-im/element-android/blob/main/library/ui-styles/src/main/res/values/colors.xml#L142
Also update line 14 and file ic_shield_unknown_no_border.xml

I have added the new color resource but I had to enable backward compatibility for API < 24 (see https://developer.android.com/develop/ui/views/graphics/vector-drawable-resources#vector-drawables-backward-solution). The update: cf25b81

mnaturel · 2022-10-14T12:44:39Z

vector/src/main/res/drawable/ic_shield_unknown.xml

+    android:viewportWidth="24"
+    android:viewportHeight="24">
+    <path
+        android:fillColor="@color/shield_color_unknown"


Backward lib compatibility has been enabled in vector module to avoid that, we should not have a warning here.

ElementBot · 2022-10-14T13:54:46Z

vector/src/main/res/drawable/ic_shield_unknown.xml

+    android:viewportWidth="24"
+    android:viewportHeight="24">
+    <path
+        android:fillColor="@color/shield_color_unknown"


⚠️ Resource references will not work correctly in images generated for this vector icon for API < 24; check generated icon to make sure it looks acceptable

⚠️ Resource references will not work correctly in images generated for this vector icon for API < 24; check generated icon to make sure it looks acceptable

ElementBot · 2022-10-14T13:54:46Z

vector/src/main/res/drawable/ic_shield_unknown.xml

+        android:strokeWidth="1"
+        android:strokeColor="#ffffff" />
+    <path
+        android:fillColor="@color/shield_color_unknown"


⚠️ Resource references will not work correctly in images generated for this vector icon for API < 24; check generated icon to make sure it looks acceptable

⚠️ Resource references will not work correctly in images generated for this vector icon for API < 24; check generated icon to make sure it looks acceptable

ElementBot · 2022-10-14T13:54:46Z

vector/src/main/res/drawable/ic_shield_unknown_no_border.xml

+    android:viewportWidth="24"
+    android:viewportHeight="24">
+    <path
+        android:fillColor="@color/shield_color_unknown"


⚠️ Resource references will not work correctly in images generated for this vector icon for API < 24; check generated icon to make sure it looks acceptable

⚠️ Resource references will not work correctly in images generated for this vector icon for API < 24; check generated icon to make sure it looks acceptable

ElementBot · 2022-10-14T13:54:46Z

vector/src/main/res/drawable/ic_shield_unknown_no_border.xml

+        android:strokeWidth="1"
+        android:strokeColor="#ffffff" />
+    <path
+        android:fillColor="@color/shield_color_unknown"


⚠️ Resource references will not work correctly in images generated for this vector icon for API < 24; check generated icon to make sure it looks acceptable

⚠️ Resource references will not work correctly in images generated for this vector icon for API < 24; check generated icon to make sure it looks acceptable

bmarty

Thanks!

sonarqubecloud · 2022-10-14T14:07:17Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

42.1% Coverage
0.0% Duplication

Maxime NATUREL added 10 commits October 13, 2022 10:13

Adding changelog entry

893008a

Adding new unknown shield icon

b18b95d

Updating descriptions when unknown verification status

ea0c990

Removing duplication of rendering session info

a48131b

Removing learn more in case of unknown verification status

7be958e

Replacing error prone takeIf structure

7ce56ce

Hide unverified sessions section when current session is not verified

f77bceb

Checking current session status in the filter use case

28ce915

Updating unit tests

316f1ef

Fixing code style issue

f72ab9b

mnaturel added the Z-NextRelease For issues and PRs which should be included in the NextRelease. label Oct 13, 2022

mnaturel marked this pull request as ready for review October 14, 2022 07:40

mnaturel requested review from a team and fedrunov and removed request for a team October 14, 2022 07:56

bmarty requested review from bmarty and removed request for fedrunov October 14, 2022 08:08

bmarty reviewed Oct 14, 2022

View reviewed changes

Adding new shield color into resources

cf25b81

mnaturel requested a review from bmarty October 14, 2022 12:02

mnaturel commented Oct 14, 2022

View reviewed changes

Fixing security section recommendation visibility

81038bd

ElementBot reviewed Oct 14, 2022

View reviewed changes

bmarty approved these changes Oct 14, 2022

View reviewed changes

mnaturel merged commit 612d2e5 into develop Oct 14, 2022

mnaturel deleted the feature/mna/device-manager-unknown-verification-status branch October 14, 2022 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Device management] Update the unknown verification status icon (PSG-824) #7361

[Device management] Update the unknown verification status icon (PSG-824) #7361

mnaturel commented Oct 13, 2022 •

edited

Loading

bmarty left a comment

bmarty Oct 14, 2022

mnaturel Oct 14, 2022

bmarty Oct 14, 2022

mnaturel Oct 14, 2022

mnaturel Oct 14, 2022

ElementBot Oct 14, 2022

ElementBot Oct 14, 2022

ElementBot Oct 14, 2022

ElementBot Oct 14, 2022

bmarty left a comment

sonarqubecloud bot commented Oct 14, 2022

[Device management] Update the unknown verification status icon (PSG-824) #7361

[Device management] Update the unknown verification status icon (PSG-824) #7361

Conversation

mnaturel commented Oct 13, 2022 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Oct 14, 2022

mnaturel commented Oct 13, 2022 •

edited

Loading