FTUE - Choose a display picture #5323

ouchadam · 2022-02-23T15:50:41Z

Type of change

WIP Feature
Bugfix
Technical
Other :

Content

Depends on #5211

Introduces a profile picture personalisation screen to the onboarding flow

Also extracts and tests the uri filename resolver since I was in the area

Motivation and context

To allow users to change their profile picture during the account creation
Part of #5200

Screenshots / GIFs

TINY	TALL	TABLET	GIF

Tests

With the personalisation feature flag enabled
Create a new account

Tested devices

Physical
Emulator
OS version(s):

github-actions · 2022-02-23T16:10:24Z

Unit Test Results

  96 files +  4   96 suites +4 1m 18s ⏱️ +12s
172 tests +10 172 ✔️ +10 0 💤 ±0 0 ❌ ±0
564 runs +40 564 ✔️ +40 0 💤 ±0 0 ❌ ±0

Results for commit 3a2dea2. ± Comparison against base commit dd0d2e8.

This pull request removes 2 and adds 12 tests. Note that renamed tests count towards both.

im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given failure when handling display name updates action then emits failure event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ when handling display name updates action then updates user display name and emits name updated event

im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given a selected picture when handling save selected profile picture then updates upstream avatar and completes personalization
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given no selected picture when saving selected profile picture then emits failure event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given upstream failure when handling display name update then emits failure event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given upstream update avatar fails when saving selected profile picture then emits failure event
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ when handling display name update then updates upstream user display name
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ when handling profile picture selected then updates selected picture state
im.vector.app.features.onboarding.OnboardingViewModelTest ‑ when handling profile picture skipped then completes personalization
im.vector.app.features.onboarding.UriFilenameResolverTest ‑ given a non content schema Uri when querying file name then returns last segment
im.vector.app.features.onboarding.UriFilenameResolverTest ‑ given a non hierarchical Uri when querying file name then is null
im.vector.app.features.onboarding.UriFilenameResolverTest ‑ given content schema Uri with backing content when querying file name then returns display name column
…

♻️ This comment has been updated with latest results.

github-actions · 2022-02-23T16:17:56Z

Matrix SDK

Integration Tests Results:

[org.matrix.android.sdk.session]
passed=
[org.matrix.android.sdk.account]
passed=
[org.matrix.android.sdk.internal]
passed=
[org.matrix.android.sdk.ordering]
passed=
[org.matrix.android.sdk.PermalinkParserTest]
passed=

- cases for the personalisation and display name actions

…er avatar

- includes tests

- moves the personalisation state to a dedicated model to allow for back and forth state restoration

… loading

- this toolbar will fade in with the fragment as it sits at the fragment level, probably worth revisiting once more pages have a toolbar

…image

- also makes the avatar itself clicking, including a foreground ripple

bmarty

Thanks for adding all those tests!
A few remarks, but that's great work! Thanks!

bmarty · 2022-03-03T21:45:13Z

library/ui-styles/src/main/res/values-h480dp/dimens.xml

+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+    <item name="ftue_auth_profile_picture_height" format="float" type="dimen">0.15</item>
+    <item name="ftue_auth_profile_picture_icon_height" format="float" type="dimen">0.05</item>


Those 2 values are the same than in default resource, is it a mistake?

ah great catch 👀 ! they can be deleted, they were separated whilst checking with design about the different screen sizes, we chose to maintain the same % of screen for small screens and allow the entire avatar to be clickable instead of making the image bigger

bmarty · 2022-03-03T21:47:49Z

vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt

-                setState { copy(asyncDisplayName = Success(Unit)) }
+                setState {
+                    copy(
+                            asyncDisplayName = Success(Unit),


Can't it be asyncDisplayName = Success(displayName) instead of storing the value in personalizationState. The remark is also applicable to selectedPictureUri I guess.

this is a good point, at the moment those asyncs are only being used for showing/hiding the loading view, in Login2 they're replaced with a single isLoading: Boolean, I was thinking of doing the same but wanted to do all the asyncs at the same time

WDYT?

That sounds like a plan! OK for me. What I want to avoid is having multiple sources of truth.

will raise a PR for it 💯

bmarty · 2022-03-03T21:52:59Z

vector/src/main/res/layout/fragment_ftue_profile_picture.xml

+                app:layout_constraintTop_toTopOf="@id/pos"
+                app:tint="?vctr_content_secondary">
+
+            </ImageView>


Could use /> for empty tag block :)

will do 👍

bmarty · 2022-03-03T21:55:22Z

vector/src/test/java/im/vector/app/test/fakes/FakeSession.kt

@@ -34,10 +35,13 @@ class FakeSession(
        mockkStatic("im.vector.app.core.extensions.SessionKt")
    }

+    override val myUserId: String = "a-user-id"


I am wondering if we should have a valid userId here like @fake:server.fake. WDYT?

sounds great to me! makes me wonder if a dedicated type for UserId or MatrixId could be helpful (might raise a ticket for a value class PoC)

ah it already exists #3633

deec923 for the value update

bmarty

Thanks for the update. I let you merge it!

* develop: (392 commits) Missing import of at-Ignore annotation. FTUE - Choose a display picture (#5323) Ignore ThreadMessagingTest as it seems to cause other integration tests to fail. Maybe the file is here? I give up for the weekend Frustration at artifact handling vs what's in docs. Update the top bar in a room (#5213) Tweak upload/download of codecov xml file Address review points from adam Remove unneeded code, retaining a comment for how to exclude certain projects Merge pull request #5405 from vector-im/cgizard/ISSUE-5402 Remove the printing of file name to the log as it's doubling up information. Remove exclusions (for now). Fix typo in name of action giving avatar/display name error dialogs human readable error messages - reuses the ErrorDialog logic which translates exceptions to human readable strings Run codecoverage and pass to sonarqube upload for processing. Better codecov based on ouchadam's suggestion Correct name of environment variable Use environment variable that is tied to project property Merge pull request #4498 from vector-im/yostyle/fix_strandhogg ... # Conflicts: # vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt

ouchadam added the Z-FTUE Issue is relevant to the first time use project or experience label Feb 23, 2022

ouchadam mentioned this pull request Feb 23, 2022

Mobile FTUE: Profile Personalisation screens (Android) #5200

Closed

10 tasks

ouchadam mentioned this pull request Feb 28, 2022

FTUE - Capability based personalisation flow #5375

Merged

6 tasks

Base automatically changed from feature/adm/choose-display-name to develop March 2, 2022 17:59

ouchadam added 17 commits March 3, 2022 10:24

adding tests around the onboarding view model

6b62b5b

- cases for the personalisation and display name actions

adding base choose name fragment with UI

53e9e8f

add click handling for the display name actions

281b65c

adding tests around the onboarding view model

8172589

- cases for the personalisation and display name actions

adding barebones profile picture fragment with ability to select a us…

c4dc874

…er avatar

extracting uri filename resolving to a class which can be injected

05394a9

- includes tests

updating upstream avatar on profile picture save and continue step

dcb16dd

- moves the personalisation state to a dedicated model to allow for back and forth state restoration

adding test case for skipping profile picture setting

bcf84b8

taking the profile loading into account when rendering the onboarding…

d3589fb

… loading

extracting method for the handling of the profile picture selection

479bff3

adding dedicated camera icon for choosing profile picture

efb08b1

adding toolbar to back to profile picture page

146ad6f

- this toolbar will fade in with the fragment as it sits at the fragment level, probably worth revisiting once more pages have a toolbar

changing edit/add picture icon based on if we're already selected an …

9518581

…image

making use of debounced clicks to avoid potential extra clicks

989e762

making the avatar height and camera icon relative percentage based

c934fdc

- also makes the avatar itself clicking, including a foreground ripple

fixing formatting

aa3f1b0

making use of fake session id for user id assertion

6b04efd

ouchadam force-pushed the feature/adm/choose-display-picture branch from 2ed1491 to 6b04efd Compare March 3, 2022 10:58

ouchadam marked this pull request as ready for review March 3, 2022 10:58

bmarty approved these changes Mar 3, 2022

View reviewed changes

ouchadam added 3 commits March 4, 2022 11:59

using a real matrix id syntax for the fake session user id

deec923

removing duplicated dimens

1fc1935

using self closing imageview tag

3a2dea2

bmarty self-requested a review March 7, 2022 14:05

bmarty approved these changes Mar 7, 2022

View reviewed changes

ouchadam enabled auto-merge (squash) March 7, 2022 14:07

ouchadam disabled auto-merge March 7, 2022 14:07

ouchadam merged commit 9a02543 into develop Mar 7, 2022

ouchadam deleted the feature/adm/choose-display-picture branch March 7, 2022 14:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FTUE - Choose a display picture #5323

FTUE - Choose a display picture #5323

ouchadam commented Feb 23, 2022 •

edited

Loading

github-actions bot commented Feb 23, 2022 •

edited

Loading

github-actions bot commented Feb 23, 2022

bmarty left a comment

bmarty Mar 3, 2022

ouchadam Mar 4, 2022

ouchadam Mar 4, 2022

bmarty Mar 3, 2022

ouchadam Mar 4, 2022

bmarty Mar 7, 2022

ouchadam Mar 7, 2022

bmarty Mar 3, 2022

ouchadam Mar 4, 2022

ouchadam Mar 4, 2022

bmarty Mar 3, 2022

ouchadam Mar 4, 2022

ouchadam Mar 4, 2022

bmarty left a comment

FTUE - Choose a display picture #5323

FTUE - Choose a display picture #5323

Conversation

ouchadam commented Feb 23, 2022 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

github-actions bot commented Feb 23, 2022 • edited Loading

Unit Test Results

github-actions bot commented Feb 23, 2022

Matrix SDK

Integration Tests Results:

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

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

ouchadam commented Feb 23, 2022 •

edited

Loading

github-actions bot commented Feb 23, 2022 •

edited

Loading