Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-40317: fixes showing new replies banner #9646

Merged

Conversation

koox00
Copy link
Contributor

@koox00 koox00 commented Jan 12, 2022

Summary

Currently we rely on the new messages line visibility on whether to show
or not the "New Replies" banner. However new messages line is not showing,
by design, on unfollowed threads. Which made the new replies banner to
show when new replies arrived for those threads.

This commit fixes the issue by guarding on the list's 'visibleStopIndex',
if visibleStopIndex is zero then the last item of the list is visible.
so we show the new replies only if the visibleStopIndex is not zero.

Ticket Link

https://mattermost.atlassian.net/browse/MM-40317

Release Note

NONE

Currently we rely on the new messages line visibility on whether to show
or not the "New Replies" banner. However new messages line is not showing,
by design, on unfollowed threads. Which made the new replies banner to
show when new replies arrived for those threads.

This commit fixes the issue by guarding on the list's 'visibleStopIndex',
if visibleStopIndex is zero then the last item of the list is visible.
so we show the new replies only if the visibleStopIndex is not zero.
@koox00 koox00 added the 2: Dev Review Requires review by a core commiter label Jan 12, 2022
@koox00 koox00 requested review from larkox, hmhealey and michelengelen and removed request for larkox January 12, 2022 18:36
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me!

Copy link
Contributor

@michelengelen michelengelen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@koox00 koox00 added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core commiter labels Jan 13, 2022
@koox00 koox00 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 13, 2022
@koox00
Copy link
Contributor Author

koox00 commented Jan 13, 2022

/e2e-test

@mattermod
Copy link
Contributor

Triggering e2e testing with options:
<nil>

@mattermod
Copy link
Contributor

@jgilliam17
Copy link
Contributor

/update-branch

@jgilliam17
Copy link
Contributor

@koox00 Thanks for running E2E already - report looks good, no PR related failures

Copy link
Contributor

@jgilliam17 jgilliam17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @koox00
Tested, looks good to merge.

  • Verified New Replies arrow appears when the new message is off the bottom of the rhs screen.

@jgilliam17 jgilliam17 removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Jan 28, 2022
@mm-cloud-bot
Copy link

Test server destroyed

@jgilliam17 jgilliam17 added the 4: Reviews Complete All reviewers have approved the pull request label Jan 28, 2022
@jgilliam17 jgilliam17 merged commit 1e63c8a into mattermost:master Jan 28, 2022
@amyblais
Copy link
Member

amyblais commented Feb 2, 2022

/cherry-pick release-6.4

@mattermod
Copy link
Contributor

Cherry pick is scheduled.

mattermost-build pushed a commit to mattermost-build/mattermost-webapp that referenced this pull request Feb 2, 2022
* MM-40317: fixes showing new replies banner

Currently we rely on the new messages line visibility on whether to show
or not the "New Replies" banner. However new messages line is not showing,
by design, on unfollowed threads. Which made the new replies banner to
show when new replies arrived for those threads.

This commit fixes the issue by guarding on the list's 'visibleStopIndex',
if visibleStopIndex is zero then the last item of the list is visible.
so we show the new replies only if the visibleStopIndex is not zero.

* Removes unneeded change

Co-authored-by: Mattermod <[email protected]>
(cherry picked from commit 1e63c8a)
@mattermod mattermod added the CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone label Feb 2, 2022
@amyblais amyblais added this to the v6.4.0 milestone Feb 2, 2022
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Feb 2, 2022
@amyblais
Copy link
Member

amyblais commented Feb 2, 2022

/cherry-pick release-6.3

@mattermod
Copy link
Contributor

Cherry pick is scheduled.

mattermost-build pushed a commit to mattermost-build/mattermost-webapp that referenced this pull request Feb 2, 2022
* MM-40317: fixes showing new replies banner

Currently we rely on the new messages line visibility on whether to show
or not the "New Replies" banner. However new messages line is not showing,
by design, on unfollowed threads. Which made the new replies banner to
show when new replies arrived for those threads.

This commit fixes the issue by guarding on the list's 'visibleStopIndex',
if visibleStopIndex is zero then the last item of the list is visible.
so we show the new replies only if the visibleStopIndex is not zero.

* Removes unneeded change

Co-authored-by: Mattermod <[email protected]>
(cherry picked from commit 1e63c8a)
mattermod pushed a commit that referenced this pull request Feb 2, 2022
mattermod pushed a commit that referenced this pull request Feb 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation release-note-none
Projects
None yet
7 participants