Feature/aris/threads post release improvements #5959

ariskotsomitopoulos · 2022-05-06T09:13:32Z

This PR contains some threads improvements. Reference link here

Clicking on room search root message summary now navigates user within the thread
Enhance thread list filtering (threads that the user is participating) icon
Open keyboard when user taps reply in thread
Fix thread room display name
Other small ui improvements

Light

Before	After

Dark

Before	After

…d action

github-actions · 2022-05-06T09:53:15Z

Unit Test Results

122 files ±0 122 suites ±0 2m 26s ⏱️ +6s
205 tests ±0 205 ✔️ ±0 0 💤 ±0 0 ❌ ±0
690 runs ±0 690 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 324856d. ± Comparison against base commit 3674ae7.

♻️ This comment has been updated with latest results.

Florian14

Code seems good to me, just some small "bugs":

if I select "reply in thread" on a message which already has a thread, it will open the thread but the keyboard is not opened
I don't have the green dot upon the filter button of the threads list:

ariskotsomitopoulos · 2022-05-09T13:41:43Z

@Florian14

If you long press -> and select reply in thread the keyboard should be opened. I have tested it again and it works in both emulator & device. Can you check it again, and if the problem exists maybe tell me what device you use. (if you simply press thread summary instead of long press -> reply in thread keyboard will/should not be opened)
The green dot will appear when you select Filter -> My Threads. The purpose of that dot is to make visible to the user that this option is selected.

Florian14 · 2022-05-09T13:52:42Z

@ariskotsomitopoulos

Did you try on a message which already has a thread? In that case, it only displays the thread timeline without the keyboard. I confirm that I did a long press -> reply in thread on the original message. If I do it on a single message (without an existing thread yet), there is no pb, the keyboard is correctly opened.
I succeed to have the dot on the second device by filtering on "My threads", but not on the first device (the user who had posted the first message and created the thread), I don't understand why.

ariskotsomitopoulos · 2022-05-09T15:54:41Z

@Florian14

Are you sure you have the correct feature version installed?

In my builds everything is working as expected, I cannot replicate it. Check the video below:

Screen_Recording_20220509-184704_Element.dbg_20220509185530.1.mp4

Florian14 · 2022-05-10T14:35:43Z

@ariskotsomitopoulos

I just tested on a fresh install and it worked fine... so it's good for me!

…ovements # Conflicts: # vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt # vector/src/main/java/im/vector/app/features/home/room/detail/search/SearchResultController.kt

bmarty

Thanks for hte PR.
Only small remarks, nothing blocking for me (static review only)

library/ui-styles/src/main/res/values/colors.xml

bmarty · 2022-05-16T09:39:41Z

library/ui-styles/src/main/res/values/colors.xml

@@ -136,4 +136,9 @@
    <color name="shield_color_black">#17191C</color>
    <color name="shield_color_warning">#FF4B55</color>

+    <!-- Badge Colors -->
+    <attr name="vctr_badge_color_border" format="color" />
+    <color name="vctr_badge_color_border_light">#FFFFFF</color>


Use palette_white instead of hard-coded color?

bmarty · 2022-05-16T09:40:31Z

vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

@@ -2443,7 +2443,7 @@ class TimelineFragment @Inject constructor(

    private fun onReplyInThreadClicked(action: EventSharedAction.ReplyInThread) {
        if (vectorPreferences.areThreadMessagesEnabled()) {
-            navigateToThreadTimeline(action.eventId, action.startsThread)
+            navigateToThreadTimeline(action.eventId, action.startsThread, true)


Add named parameter? Useful when reviewing on GH.

bmarty · 2022-05-16T09:43:05Z

vector/src/main/java/im/vector/app/features/home/room/detail/search/SearchResultItem.kt

@@ -66,6 +67,7 @@ abstract class SearchResultItem : VectorEpoxyModel<SearchResultItem.Holder>() {
                    val displayName = it.threadSummarySenderInfo?.displayName
                    val avatarUrl = it.threadSummarySenderInfo?.avatarUrl
                    avatarRenderer.render(MatrixItem.UserItem(userId, displayName, avatarUrl), holder.threadSummaryAvatarImageView)
+                    holder.threadSummaryConstraintLayout.onClick(threadSummaryListener)


This is not new, but the id threadSummaryConstraintLayout should be renamed to something more generic like threadSummaryLayout or threadSummaryContainer. Applicable to some other view ids, and not a blocker.

I updated searchThreadSummaryConstraintLayout to searchThreadSummaryContainer, here might be good to have something generic for future changes. However, sometimes it is useful to have the exact type (ConstraintLayout, TextView, ImageView, etc ) in the id while the developer knows at once the type.

…ovements # Conflicts: # vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

github-actions · 2022-05-17T10:55:24Z

Matrix SDK

Integration Tests Results:

[org.matrix.android.sdk.session]
= passed=7 failures=13 errors=0 skipped=3
[org.matrix.android.sdk.account]
= passed=1 failures=2 errors=0 skipped=2
[org.matrix.android.sdk.internal]
= passed=88 failures=59 errors=0 skipped=13
[org.matrix.android.sdk.ordering]
= passed=16 failures=0 errors=0 skipped=0
[org.matrix.android.sdk.PermalinkParserTest]
= passed=2 failures=0 errors=0 skipped=0

ariskotsomitopoulos added 7 commits May 4, 2022 13:55

Reduce thread list menu icon size

4b746ee

Show room name instead of root message author in thread timeline toolbar

88babbb

Open thread timeline keyboard when user navigates from reply in threa…

f4b1e06

…d action

Add navigate to thread functionality on search results thread summary

ad50c22

Remove thread summary counter minEms

faf5fc0

Add badge in thread list filtering

c5fc1e5

Add changelog

87c4289

ariskotsomitopoulos requested review from a team, bmarty and Florian14 and removed request for a team May 6, 2022 09:20

Florian14 reviewed May 9, 2022

View reviewed changes

Florian14 approved these changes May 10, 2022

View reviewed changes

Merge branch 'develop' into feature/aris/threads_post_release_ui_impr…

40de685

…ovements # Conflicts: # vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt # vector/src/main/java/im/vector/app/features/home/room/detail/search/SearchResultController.kt

bmarty approved these changes May 16, 2022

View reviewed changes

ariskotsomitopoulos added 2 commits May 16, 2022 13:32

PR remarks

72a945a

Merge branch 'develop' into feature/aris/threads_post_release_ui_impr…

324856d

…ovements # Conflicts: # vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

ariskotsomitopoulos merged commit 424fb55 into develop May 17, 2022

ariskotsomitopoulos deleted the feature/aris/threads_post_release_ui_improvements branch May 17, 2022 10:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/aris/threads post release improvements #5959

Feature/aris/threads post release improvements #5959

ariskotsomitopoulos commented May 6, 2022

github-actions bot commented May 6, 2022 •

edited

Florian14 left a comment

ariskotsomitopoulos commented May 9, 2022 •

edited

Florian14 commented May 9, 2022 •

edited

ariskotsomitopoulos commented May 9, 2022 •

edited

Florian14 commented May 10, 2022 •

edited

bmarty left a comment

bmarty May 16, 2022

bmarty May 16, 2022

bmarty May 16, 2022

ariskotsomitopoulos May 16, 2022

github-actions bot commented May 17, 2022

Feature/aris/threads post release improvements #5959

Feature/aris/threads post release improvements #5959

Conversation

ariskotsomitopoulos commented May 6, 2022

github-actions bot commented May 6, 2022 • edited

Unit Test Results

Florian14 left a comment

Choose a reason for hiding this comment

ariskotsomitopoulos commented May 9, 2022 • edited

Florian14 commented May 9, 2022 • edited

ariskotsomitopoulos commented May 9, 2022 • edited

Florian14 commented May 10, 2022 • edited

bmarty left a comment

Choose a reason for hiding this comment

bmarty May 16, 2022

Choose a reason for hiding this comment

bmarty May 16, 2022

Choose a reason for hiding this comment

bmarty May 16, 2022

Choose a reason for hiding this comment

ariskotsomitopoulos May 16, 2022

Choose a reason for hiding this comment

github-actions bot commented May 17, 2022

Matrix SDK

Integration Tests Results:

github-actions bot commented May 6, 2022 •

edited

ariskotsomitopoulos commented May 9, 2022 •

edited

Florian14 commented May 9, 2022 •

edited

ariskotsomitopoulos commented May 9, 2022 •

edited

Florian14 commented May 10, 2022 •

edited