[MM-24436]- Add a threshold from bottom for new messages toast #5828

sridhar02 · 2020-07-01T06:01:51Z

Summary

Changed the buffer value to be considered from bottom for the toast message

Ticket Link

Fixes mattermost/mattermost#14904
JIRA https://mattermost.atlassian.net/browse/MM-24436

mattermod · 2020-07-01T06:01:54Z

Thanks @sridhar02 for the pull request!

Per the CONTRIBUTING.md file displayed when you created this pull request, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?

This is a standard procedure for many open source projects. Your form should be processed within 24 hours and reviewers for your pull request will be able to proceed.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

hanzei · 2020-07-01T08:05:20Z

/check-cla

sudheerDev · 2020-07-02T17:42:31Z

@sridhar02 Hey, BUFFER_TO_BE_CONSIDERED_BOTTOM is used for determining if the scroll view is at the bottom. we give a 10px buffer and still consider it as bottom.

This value is used for various states like if user is at the bottom and he receives a new message, view is auto scrolled to the bottom. Now with this change even if user is 999px he is autoscrolled to the bottom.

The way to implement is to have a look at toast_wrapper code which has a mock for archive toast which takes 1000px value into consideration.

Hope this helps. You can reach out to me incase of questions

sridhar02 · 2020-07-06T07:32:35Z

@sudheerDev can u recheck the PR, i have changed the logic to toast_wrapper component and changed test cases values related to toast_wrapper.

sudheerDev · 2020-07-07T14:03:04Z

components/toast_wrapper/toast_wrapper.jsx

@@ -84,7 +84,7 @@ class ToastWrapper extends React.PureComponent {
        }

        // show unread toast on mount when channel is not at bottom and unread count greater than 0
-        if (typeof showUnreadToast === 'undefined' && props.atBottom !== null) {
+        if (typeof showUnreadToast === 'undefined' && props.atBottom !== null && props.initScrollOffsetFromBottom > 1000) {


We want toast value to be either true or false, but for assigning the value to showUnreadToast we need to wait for the scroll position to be set i.e wait for view to scroll to position for new message etc hence we set the first value at this condition.

if (typeof showUnreadToast === 'undefined' && props.atBottom !== null) {

This is true when a component is mounted and scroll position is set for the first time because props.atBottom will be not be null anymore. But if change it to

if (typeof showUnreadToast === 'undefined' && props.atBottom !== null && props.initScrollOffsetFromBottom > 1000) {
Then it will not set the initial value of showUnreadToast anymore.

Instead this should be

if (typeof showUnreadToast === 'undefined' && props.atBottom !== null) { showUnreadToast = unreadCount > 0 && props.initScrollOffsetFromBottom > 1000; }

okay, I will change the logic.

Changes Done

Thank you, this looks good, Can you add a test case for less than 1000px?

yes I have added a new test case for less than 1000px

Willyfrog · 2020-07-20T11:37:31Z

components/toast_wrapper/toast_wrapper.jsx

@@ -85,7 +85,7 @@ class ToastWrapper extends React.PureComponent {

        // show unread toast on mount when channel is not at bottom and unread count greater than 0
        if (typeof showUnreadToast === 'undefined' && props.atBottom !== null) {
-            showUnreadToast = unreadCount > 0 && !props.atBottom;


minor nitpick, can this 1000 scroll be a named constant so it can be more easily read?

Willyfrog · 2020-07-20T11:38:12Z

minor change, not even a blocking one

esethna · 2020-07-21T20:11:14Z

@sudheerDev @sridhar02 is this ready for re-review?

sridhar02 · 2020-07-22T05:31:00Z

@esethna yes, this PR is ready for the re-review.

esethna

I'm seeing the new message line indicator appear very briefly before disappearing if the channel is within the threshold of 1000px

sridhar02 · 2020-07-27T05:52:32Z

@esethna if there is an issue with the PR, mention it, I will try to resolve it.

sudheerDev

LGTM

sudheerDev · 2020-10-14T08:18:24Z

@sridhar02 Hey Thanks for working on the feature, in future try to avoid force pushing the commits. Like in this case QA reviews are done and i see commit history changed and it would be hard to judge if you made any new changes after that

mm-cloud-bot · 2020-10-14T08:18:57Z

Test server destroyed

mm-cloud-bot · 2020-10-14T08:18:59Z

Test server destroyed

…o MM-20457 * 'master' of github.com:Tak-Iwamoto/mattermost-webapp: (87 commits) MM-T644 Integrations display on team where they were created (mattermost#6752) [MM-20478] Migrate post_header module to TypeScript (mattermost#6631) [MM-20599] Migrated select_team component to Typescript (mattermost#6574) MM-20554 Migrate 'components/delete_post_modal' module and associated tests to TypeScript (mattermost#6656) [MM-24436]- Add a threshold from bottom for new messages toast (mattermost#5828) [MM-20489] Migrate failed_post_options and its tests to typescript (mattermost#6717) [MM-28063] Cloud Telemetry - Admin Console (mattermost#6762) [MM-29559][MM-29558] Company Info Fixes (mattermost#6764) [MM-29557] [MM-29590] Update subscription when purchase modal closes (mattermost#6765) [MM-29615] Fixed subscription page so it doesn't load until subscription info is loaded (mattermost#6766) [MM-28064] Add telemetry in various places around cloud message banners (mattermost#6763) migrate changeCSS function CSS variable for mobile CSS .tutorial-steps__container selector. (mattermost#6743) [MM-27231]: cypress test for MM-T1837 (mattermost#6676) [MM-28062] Add telemetry for in-app purchase flow (mattermost#6760) MM-27454 - Contact Us and Billing Documentation Links (mattermost#6731) [MM-20514] Migrates components/password reset send link to typescript (mattermost#6584) Cloud Billing polish Soft GA (mattermost#6740) [MM-28221] Payment Info Edit/View (mattermost#6709) MM-T636 Description field for incoming and outgoing webhooks can hold 500 characters (mattermost#6682) Translations update from Weblate (mattermost#6748) ...

amyblais · 2020-10-19T20:24:48Z

/cherry-pick release-5.29

mattermod · 2020-10-19T20:24:50Z

Cherry pick is scheduled.

…rmost#5828) * Changed the buffer value to be considered from bottom for the toast message * test value changes related to isAtBottom function * Changes are set to previous values as this is not right approach * added the condition for the offset from bottom greater than 1000px * Change the test conditions according to new condition to show the toast message * logic change * eslint corrections * added a test that fails if the scroll from bottom is less than 1000px * add constant THRESHOLD_FROM_BOTTOM and replace 1000 value * Bug fix for toast flashing when you mark as unread * bug fix * tests resolved (cherry picked from commit 855633d)

#6850) * Changed the buffer value to be considered from bottom for the toast message * test value changes related to isAtBottom function * Changes are set to previous values as this is not right approach * added the condition for the offset from bottom greater than 1000px * Change the test conditions according to new condition to show the toast message * logic change * eslint corrections * added a test that fails if the scroll from bottom is less than 1000px * add constant THRESHOLD_FROM_BOTTOM and replace 1000 value * Bug fix for toast flashing when you mark as unread * bug fix * tests resolved (cherry picked from commit 855633d) Co-authored-by: Sridhar <kattasridhar02@gmail.com>

* Changed the buffer value to be considered from bottom for the toast message * test value changes related to isAtBottom function * Changes are set to previous values as this is not right approach * added the condition for the offset from bottom greater than 1000px * Change the test conditions according to new condition to show the toast message * logic change * eslint corrections * added a test that fails if the scroll from bottom is less than 1000px * add constant THRESHOLD_FROM_BOTTOM and replace 1000 value * Bug fix for toast flashing when you mark as unread * bug fix * tests resolved

#5828)" This reverts commit 855633d.

* Revert "MM-29929 Fix for unread messages toast on marking channel as unread (#7028)" This reverts commit 92f5d86. * Revert "[MM-24436]- Add a threshold from bottom for new messages toast (#5828)" This reverts commit 855633d.

* Revert "MM-29929 Fix for unread messages toast on marking channel as unread (mattermost#7028)" This reverts commit 92f5d86. * Revert "[MM-24436]- Add a threshold from bottom for new messages toast (mattermost#5828)" This reverts commit 855633d. (cherry picked from commit 3e429f7)

* Revert "MM-29929 Fix for unread messages toast on marking channel as unread (#7028)" This reverts commit 92f5d86. * Revert "[MM-24436]- Add a threshold from bottom for new messages toast (#5828)" This reverts commit 855633d. (cherry picked from commit 3e429f7) Co-authored-by: Harrison Healey <harrisonmhealey@gmail.com>

hanzei requested a review from sudheerDev July 1, 2020 08:05

hanzei added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jul 1, 2020

sridhar02 force-pushed the MM-24436 branch from 0f8faf2 to 826f18c Compare July 3, 2020 10:40

sudheerDev suggested changes Jul 7, 2020

View reviewed changes

sridhar02 force-pushed the MM-24436 branch from bfcb0cc to 156afa6 Compare July 8, 2020 02:09

sridhar02 requested a review from sudheerDev July 8, 2020 11:54

esethna added this to the v5.26 milestone Jul 9, 2020

esethna requested review from Willyfrog and jgilliam17 July 9, 2020 21:10

sudheerDev added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jul 10, 2020

sudheerDev requested a review from esethna July 10, 2020 07:26

sudheerDev added the 1: PM Review Requires review by a product manager label Jul 10, 2020

sridhar02 force-pushed the MM-24436 branch 2 times, most recently from 2b08dc8 to 72baa36 Compare July 17, 2020 04:09

amyblais removed this from the v5.26 milestone Jul 17, 2020

sridhar02 force-pushed the MM-24436 branch from 72baa36 to 8361745 Compare July 20, 2020 06:21

Willyfrog approved these changes Jul 20, 2020

View reviewed changes

sudheerDev approved these changes Jul 22, 2020

View reviewed changes

sudheerDev removed the 2: Dev Review Requires review by a core commiter label Jul 22, 2020

esethna suggested changes Jul 22, 2020

View reviewed changes

sridhar02 force-pushed the MM-24436 branch from 22d51e8 to d6e49e2 Compare July 22, 2020 15:37

esethna approved these changes Oct 13, 2020

View reviewed changes

esethna removed the 1: PM Review Requires review by a product manager label Oct 13, 2020

esethna added this to the v5.29.0 milestone Oct 13, 2020

sudheerDev approved these changes Oct 14, 2020

View reviewed changes

sudheerDev added the 4: Reviews Complete All reviewers have approved the pull request label Oct 14, 2020

sudheerDev merged commit 855633d into mattermost:master Oct 14, 2020

mattermod removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 14, 2020

amyblais added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Oct 16, 2020

mattermost-build mentioned this pull request Oct 19, 2020

Automated cherry pick of #5828 #6850

Merged

mattermod added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Oct 19, 2020

amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Oct 20, 2020

sudheerDev mentioned this pull request Nov 6, 2020

MM-29929 Fix for unread messages toast on marking channel as unread #7028

Merged

hmhealey added a commit that referenced this pull request Jan 7, 2021

Revert "[MM-24436]- Add a threshold from bottom for new messages toast (

9ecad3e

#5828)" This reverts commit 855633d.

hmhealey mentioned this pull request Jan 7, 2021

MM-31189 Revert New Messages toast PRs #7292

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-24436]- Add a threshold from bottom for new messages toast #5828

[MM-24436]- Add a threshold from bottom for new messages toast #5828

sridhar02 commented Jul 1, 2020

mattermod commented Jul 1, 2020

hanzei commented Jul 1, 2020

sudheerDev commented Jul 2, 2020

sridhar02 commented Jul 6, 2020 •

edited

Loading

sudheerDev Jul 7, 2020

sridhar02 Jul 8, 2020

sridhar02 Jul 8, 2020 •

edited

Loading

sudheerDev Jul 10, 2020

sridhar02 Jul 12, 2020

Willyfrog Jul 20, 2020

sridhar02 Jul 22, 2020

Willyfrog commented Jul 20, 2020

esethna commented Jul 21, 2020

sridhar02 commented Jul 22, 2020

esethna left a comment

sridhar02 commented Jul 27, 2020

sudheerDev left a comment

sudheerDev commented Oct 14, 2020

mm-cloud-bot commented Oct 14, 2020

mm-cloud-bot commented Oct 14, 2020

amyblais commented Oct 19, 2020

mattermod commented Oct 19, 2020

[MM-24436]- Add a threshold from bottom for new messages toast #5828

[MM-24436]- Add a threshold from bottom for new messages toast #5828

Conversation

sridhar02 commented Jul 1, 2020

Summary

Ticket Link

mattermod commented Jul 1, 2020

hanzei commented Jul 1, 2020

sudheerDev commented Jul 2, 2020

sridhar02 commented Jul 6, 2020 • edited Loading

sudheerDev Jul 7, 2020

Choose a reason for hiding this comment

sridhar02 Jul 8, 2020

Choose a reason for hiding this comment

sridhar02 Jul 8, 2020 • edited Loading

Choose a reason for hiding this comment

sudheerDev Jul 10, 2020

Choose a reason for hiding this comment

sridhar02 Jul 12, 2020

Choose a reason for hiding this comment

Willyfrog Jul 20, 2020

Choose a reason for hiding this comment

sridhar02 Jul 22, 2020

Choose a reason for hiding this comment

Willyfrog commented Jul 20, 2020

esethna commented Jul 21, 2020

sridhar02 commented Jul 22, 2020

esethna left a comment

Choose a reason for hiding this comment

sridhar02 commented Jul 27, 2020

sudheerDev left a comment

Choose a reason for hiding this comment

sudheerDev commented Oct 14, 2020

mm-cloud-bot commented Oct 14, 2020

mm-cloud-bot commented Oct 14, 2020

amyblais commented Oct 19, 2020

mattermod commented Oct 19, 2020

sridhar02 commented Jul 6, 2020 •

edited

Loading

sridhar02 Jul 8, 2020 •

edited

Loading