[MM-23761] [MM-25766] Add "More Messages" button #4416

migbot · 2020-06-11T21:14:41Z

Summary

This PR removes auto-scrolling to the new message line on channel load and adds a "More Messages" button when there are unread posts. The unread post count is calculated on channel switch from the difference channel.total_msg_count - member.msg_count and with the addition of any inserted non-message posts (ie, date line) by makePreparePostIdsForPostList. On press of the button the user is scrolled to the index of the new message line or to the index of the highestMeasuredFrameIndex if the new message line post has not yet been measured. If there are no more unread messages the button is hidden, otherwise the unread count is updated and pressing again will scroll to the updated index of the new message line or the next highestMeasuredFrameIndex.

Because we don't want a new message line to be added when a user submits their own post either in the current channel or in another channel from separate client, there was a previous condition added in makeFilterPostsAndAddSeparators to not add the new message line if the post's user_id is the current user's ID. This was problematic for this feature as the index of the final new message line would not coincide with the index of the last unread message and so in this case the "More Messages" button would never hide. To get around this I added an ownPost property to Post. When RECEIVED_POST or RECEIVED_NEW_POST actions are handled, if the post is the current user's post then the channel's lastChannelViewTime is updated to be the post's created_at value + 1, ensuring that the new message line is not added since the existing condition post.create_at > lastViewedAt will be false.

Ticket Link

https://mattermost.atlassian.net/browse/MM-23761
https://mattermost.atlassian.net/browse/MM-25766

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates

Device Information

This PR was tested on:

iOS 13 simulator and iPhone SE
Android Q emulator and Mi A3 running Android 9

mattermod · 2020-06-11T21:40:44Z

Building app in separate branch.

mattermod · 2020-06-11T21:49:24Z

Successfully building:
https://circleci.com/gh/mattermost/mattermost-mobile/6842
https://circleci.com/gh/mattermost/mattermost-mobile/6840

* Use viewAreaCoveragePercentThreshold over itemVisiblePercentThreshold * Remove currentUserId check when adding New Message line to postIds and instead update the channel last viewed time when an own post is received

esethna · 2020-06-15T15:46:05Z

Adding @matthewbirtch for UX review when this is ready for testing

matthewbirtch

Thanks for getting this in @migbot! Great work. This is going to make things so much better on the app. Below is my first review - let me know if anything is unclear.

UI tweaks

The color of the banner should be using the button background color.
The height of the banner should be 40px (looks like it's 37px)
The corner radius on the banner should be 4px (looks like it's 5px)
The icon for the arrow seems to be a bit small
The icon for the x seems to be a bit small as well
The margin to the top/left/right of the banner should be 8px (looks like it's 10px)

Here's the comparison of what's implemented vs. what was designed:

Implemented	Designed

UX issues

When you tap the banner to scroll to the new message line, can we add any easing to this scrolling animation? It would feel a lot better if we added an ease-out to this.
I tapped a banner that said '43 new messages' and it scrolled to the new message line. However it left the banner up and said '43 new messages' and didn't animate away after the scroll transition
When a banner displays and I scroll through the messages above without tapping the banner, I noticed the number changes as I scroll, which is kind of cool - nice touch! However, I scrolled to the point where the New Messages Line showed and the banner didn't animate away - it stayed on screen displaying "3 more new messages". I would have expected the count to go to zero and animate away. Also, when I tapped the banner at this point, it did nothing. Even the 'x' icon did nothing in this scenario. I should mention that this seems to be a problem either with 40+ new messages or with scrolling fast. With smaller new messages or slower scrolling, the banner seemed to behave as I would expected. See the screenshot below (notice the banner is displaying at the same time as the new messages line)

When the new messages banner is on screen and I pull up to refresh, the count seems to add 1 and then remove 1 when the load is done - it shouldn't add anything unless it finds new messages on the pull up to refresh

enahum · 2020-06-16T15:05:29Z

Thanks for the early review... this is still a WIP

josephbaylon

@migbot It's looking good from my end. Also, the raw count is now showing up on the banners.
@matthewbirtch @esethna Please update the description in this ticket to reflect the new changes agreed upon. Thanks
https://mattermost.atlassian.net/browse/MM-23761

migbot · 2020-06-27T03:25:31Z

@esethna @matthewbirtch @josephbaylon

You should see a shorter lag between count updates:
https://pr-builds.mattermost.com/mattermost-mobile/build-pr-4416-take6/Mattermost_Beta.apk
itms-services://?action=download-manifest&url=https://pr-builds.mattermost.com/mattermost-mobile/build-pr-4416-take6/Mattermost_Beta.plist

josephbaylon · 2020-06-29T14:56:02Z

@migbot LGTM. I experience smoother count updates on both iOS and Android.

amyblais · 2020-06-30T12:48:55Z

@esethna Is this something that needs to be in v1.33? We're past code complete and RC testing has started, so I'm not sure if it's too risky to add this to v1.33 in the last minute as this PR looks big.

enahum · 2020-06-30T14:05:10Z

@esethna I agree with @amyblais this should be pushed to 1.34

matthewbirtch · 2020-06-30T17:21:46Z

You should see a shorter lag between count updates:
https://pr-builds.mattermost.com/mattermost-mobile/build-pr-4416-take6/Mattermost_Beta.apk
itms-services://?action=download-manifest&url=https://pr-builds.mattermost.com/mattermost-mobile/build-pr-4416-take6/Mattermost_Beta.plist

I wasn't having a significant lag before and it seems ok now, but I'm curious if it's improved for @esethna

esethna · 2020-06-30T18:00:31Z

@migbot I think you were looking into some other issues based on the thread we had on Friday?

…dex changes

migbot · 2020-06-30T21:16:50Z

@esethna Can you try the following builds:
https://community.mattermost.com/core/pl/k8h7j7heij8ntn6cfxactyqu5c
https://community.mattermost.com/core/pl/p7b7d75wabn9fnuw6yfc6r77cr

migbot · 2020-06-30T23:39:56Z

@esethna @matthewbirtch @josephbaylon

Can you test the following (I promise this will be the last one 🤞 !):
https://community.mattermost.com/core/pl/cpqozarhoifjx8iq6u8d56k8wc
https://community.mattermost.com/core/pl/c5oqjra36ibc8qtz1w4pk4oorc

matthewbirtch

@migbot, it's still feeling a bit buggy/unresponsive to me:

There is still a bit of a delay to update the count when I tap on a banner that has a count larger than 60 new messages
I got a banner with "105 new messages" that when I tapped, it scrolled up, but did not update the count when it stopped (see screen recording)

Link to screen recording

enahum · 2020-07-02T13:58:55Z

@matthewbirtch I want to understand this better

There is still a bit of a delay to update the count when I tap on a banner that has a count larger than 60 new messages

Cause the count has to wait until we fetch more posts from the server to get the new value, isn’t that expected? I struggle to see a wat around that as we can’t know the amount until the posts are actually fetched... or is it that I’m not understanding the issue that you are seeing?

migbot · 2020-07-02T17:27:23Z

I got a banner with "105 new messages" that when I tapped, it scrolled up, but did not update the count when it stopped (see screen recording)

Link to screen recording

@matthewbirtch Do you see this consistently? And does this happen when you mark a message as unread in the channel or without having marked a message as unread?

matthewbirtch · 2020-07-02T17:43:08Z

@mig

@matthewbirtch Do you see this consistently? And does this happen when you mark a message as unread in the channel or without having marked a message as unread?

@migbot it doesn't appear to be happening consistently. I just tried to reproduce on many different channels by marking messages as unread, but can't seem to reproduce the case I got on video

matthewbirtch · 2020-07-02T17:45:49Z

Cause the count has to wait until we fetch more posts from the server to get the new value, isn’t that expected? I struggle to see a wat around that as we can’t know the amount until the posts are actually fetched... or is it that I’m not understanding the issue that you are seeing?

@enahum We know the total number at the beginning when the banner shows. Then when we scroll, we should know the number of posts we just scrolled past. If that's true, can we not subtract that number from the initial total to get the right number immediately without any delay? Or am I not understanding that correctly

migbot · 2020-07-02T17:58:55Z

Cause the count has to wait until we fetch more posts from the server to get the new value, isn’t that expected? I struggle to see a wat around that as we can’t know the amount until the posts are actually fetched... or is it that I’m not understanding the issue that you are seeing?

@enahum We know the total number at the beginning when the banner shows. Then when we scroll, we should know the number of posts we just scrolled past. If that's true, can we not subtract that number from the initial total to get the right number immediately without any delay? Or am I not understanding that correctly

The initial unread count is not a true total unread count because it only accounts for message posts. In the list we insert non-message posts, like date lines and combined user activity, and these need to be added to the unread count as they load in order to keep the unread count and the new line message index synced. So if we were to subtract from the unread count when scroll ends you would see the unread count change once for the subtraction and then again as the non-message posts in the next set are loaded and the unread count is increased.

migbot · 2020-07-03T05:42:01Z

Closing in favor of #4416.

Add more messages button

081454e

migbot added Work In Progress Not yet ready for review Build Apps for PR Build the mobile app for iOS and Android to test labels Jun 11, 2020

migbot added this to the v1.33 milestone Jun 11, 2020

Update existing tests

5edc234

migbot added Build Apps for PR Build the mobile app for iOS and Android to test and removed Build Apps for PR Build the mobile app for iOS and Android to test labels Jun 11, 2020

mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 11, 2020

metanerd added the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 12, 2020

mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 12, 2020

migbot added 2 commits June 12, 2020 15:17

Various fixes:

b649146

* Use viewAreaCoveragePercentThreshold over itemVisiblePercentThreshold * Remove currentUserId check when adding New Message line to postIds and instead update the channel last viewed time when an own post is received

Update snapshots

a7071fc

migbot added the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 12, 2020

mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 12, 2020

esethna requested a review from matthewbirtch June 15, 2020 15:45

migbot added 2 commits June 15, 2020 11:08

Add showMoreMessagesButton prop and default to false

2526884

Android fixes

845472e

matthewbirtch requested changes Jun 16, 2020

View reviewed changes

Add tests

5bd224e

migbot added the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 16, 2020

mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 16, 2020

Localize more messages text

0a40228

mattermost deleted a comment from mattermod Jun 16, 2020

mattermost deleted a comment from metanerd Jun 16, 2020

josephbaylon reviewed Jun 26, 2020

View reviewed changes

Use default windowSize and lower POST_CHUNK_SIZE and delays

af3869e

migbot added the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 27, 2020

mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 27, 2020

mattermost deleted a comment from mattermod Jun 27, 2020

enahum modified the milestones: v1.33, v1.34 Jun 30, 2020

enahum removed the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Jun 30, 2020

migbot added 2 commits June 30, 2020 13:00

Queue a cancel timer that gets cleared only when the newMessageLineIn…

8156c00

…dex changes

Define uncancel func

b00878e

Increase cancelTimer delay

98038ea

matthewbirtch requested changes Jul 2, 2020

View reviewed changes

migbot closed this Jul 3, 2020

migbot deleted the MM-23761/more-messages-banner branch July 3, 2020 05:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-23761] [MM-25766] Add "More Messages" button #4416

[MM-23761] [MM-25766] Add "More Messages" button #4416

migbot commented Jun 11, 2020 •

edited

mattermod commented Jun 11, 2020

mattermod commented Jun 11, 2020

esethna commented Jun 15, 2020

matthewbirtch left a comment

enahum commented Jun 16, 2020

josephbaylon left a comment

migbot commented Jun 27, 2020

josephbaylon commented Jun 29, 2020

amyblais commented Jun 30, 2020

enahum commented Jun 30, 2020

matthewbirtch commented Jun 30, 2020

esethna commented Jun 30, 2020

migbot commented Jun 30, 2020

migbot commented Jun 30, 2020

matthewbirtch left a comment •

edited

enahum commented Jul 2, 2020

migbot commented Jul 2, 2020

matthewbirtch commented Jul 2, 2020

matthewbirtch commented Jul 2, 2020 •

edited

migbot commented Jul 2, 2020

migbot commented Jul 3, 2020

[MM-23761] [MM-25766] Add "More Messages" button #4416

[MM-23761] [MM-25766] Add "More Messages" button #4416

Conversation

migbot commented Jun 11, 2020 • edited

Summary

Ticket Link

Checklist

Device Information

mattermod commented Jun 11, 2020

mattermod commented Jun 11, 2020

esethna commented Jun 15, 2020

matthewbirtch left a comment

Choose a reason for hiding this comment

UI tweaks

UX issues

enahum commented Jun 16, 2020

josephbaylon left a comment

Choose a reason for hiding this comment

migbot commented Jun 27, 2020

josephbaylon commented Jun 29, 2020

amyblais commented Jun 30, 2020

enahum commented Jun 30, 2020

matthewbirtch commented Jun 30, 2020

esethna commented Jun 30, 2020

migbot commented Jun 30, 2020

migbot commented Jun 30, 2020

matthewbirtch left a comment • edited

Choose a reason for hiding this comment

enahum commented Jul 2, 2020

migbot commented Jul 2, 2020

matthewbirtch commented Jul 2, 2020

matthewbirtch commented Jul 2, 2020 • edited

migbot commented Jul 2, 2020

migbot commented Jul 3, 2020

migbot commented Jun 11, 2020 •

edited

matthewbirtch left a comment •

edited

matthewbirtch commented Jul 2, 2020 •

edited