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

MM-13180 Call load posts earlier for infinite scroll #2079

Merged
merged 4 commits into from
Dec 24, 2018

Conversation

sudheerDev
Copy link
Contributor

Summary

Loads posts earlier than existing trigger.

Existing trigger is at 40% of clientHeight(visible scroll area) so typically about 0.4 * 800(clientHeight) i.e 200px from the top of the postList. If there are 4000px of posts in total at present we trigger to load posts when user reaches to 3800px

With this PR we change this to load when scroll position is at 30% from the top. i.e if 4000px we trigger to load posts at 3040px. i.e by 0.3 * (4000 - 800) i.e 960px

Ticket Link

MM-13180

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed
  • Has UI changes

@sudheerDev sudheerDev added the 2: Dev Review Requires review by a core commiter label Nov 26, 2018
@sudheerDev sudheerDev added this to the v5.6.0 milestone Nov 26, 2018
@sudheerDev
Copy link
Contributor Author

@esethna would you like to test on a spin mint?

@sudheerDev sudheerDev assigned sudheerDev and unassigned sudheerDev Nov 26, 2018
@sudheerDev sudheerDev added the Setup Old Test Server Triggers the creation of a test server label Nov 26, 2018
Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

LGTM, just have a non-blocking request

components/post_view/post_list.jsx Outdated Show resolved Hide resolved
@saturninoabril saturninoabril added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Nov 27, 2018
@esethna esethna removed the Setup Old Test Server Triggers the creation of a test server label Nov 27, 2018
@mattermost mattermost deleted a comment from mattermod Nov 27, 2018
@mattermost mattermost deleted a comment from mattermod Nov 27, 2018
@mattermost mattermost deleted a comment from mattermod Nov 27, 2018
@esethna esethna added the Setup Old Test Server Triggers the creation of a test server label Nov 27, 2018
@sudheerDev sudheerDev added Awaiting Submitter Action Blocked on the author and removed 4: Reviews Complete All reviewers have approved the pull request Setup Old Test Server Triggers the creation of a test server labels Nov 29, 2018
@sudheerDev sudheerDev removed this from the v5.6.0 milestone Nov 29, 2018
@sudheerDev sudheerDev added the Setup Old Test Server Triggers the creation of a test server label Dec 20, 2018
@mattermost mattermost deleted a comment from mattermod Dec 20, 2018
@mattermost mattermost deleted a comment from mattermod Dec 20, 2018
@mattermost mattermost deleted a comment from mattermod Dec 20, 2018
@sudheerDev sudheerDev added Setup Old Test Server Triggers the creation of a test server and removed Setup Old Test Server Triggers the creation of a test server labels Dec 20, 2018
@sudheerDev sudheerDev added Setup Old Test Server Triggers the creation of a test server and removed Setup Old Test Server Triggers the creation of a test server labels Dec 20, 2018
@mattermost mattermost deleted a comment from mattermod Dec 20, 2018
@mattermost mattermost deleted a comment from mattermod Dec 20, 2018
@mattermost mattermost deleted a comment from mattermod Dec 20, 2018
@sudheerDev sudheerDev added the Setup Old Test Server Triggers the creation of a test server label Dec 20, 2018
@mattermod
Copy link
Contributor

Setup Test Server label detected. Spinmint test server created if build succeeds (checks pass and no conflicts with base branch).

@mattermod
Copy link
Contributor

Spinmint test server created at: https://i-07d1888c53b6bedeb.test.spinmint.com

Test Admin Account: Username: sysadmin | Password: sysadmin

Test User Account: Username: user-1 | Password: user-1

Instance ID: i-07d1888c53b6bedeb

Copy link
Contributor

@esethna esethna left a comment

Choose a reason for hiding this comment

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

Awesome work, looks great

@esethna esethna removed Awaiting Submitter Action Blocked on the author Setup Old Test Server Triggers the creation of a test server labels Dec 20, 2018
@mattermod
Copy link
Contributor

Spinmint test server destroyed

@esethna esethna added the 4: Reviews Complete All reviewers have approved the pull request label Dec 20, 2018
@esethna
Copy link
Contributor

esethna commented Dec 20, 2018

Can this be merged now or do we need to wait for the perf improvements to go in forst?

@esethna esethna added the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Dec 20, 2018
@sudheerDev
Copy link
Contributor Author

@esethna This is not blocked IMO and better we have this in master for comparing perf benefits from the other PR's

@sudheerDev sudheerDev added this to the v5.8.0 milestone Dec 21, 2018
@sudheerDev sudheerDev removed the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Dec 24, 2018
@sudheerDev sudheerDev merged commit 9723e31 into mattermost:master Dec 24, 2018
@sudheerDev sudheerDev deleted the MM-13180 branch December 24, 2018 07:32
@lindy65 lindy65 removed the 4: Reviews Complete All reviewers have approved the pull request label Jan 3, 2019
@JtheBAB JtheBAB mentioned this pull request Jan 15, 2019
9 tasks
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 23, 2019
@lindalumitchell lindalumitchell added the Tests/Not Needed Does not require new release tests label Feb 13, 2019
tuannguyen041094 pushed a commit to Designveloper/mattermost-webapp that referenced this pull request Apr 24, 2019
* MM-13180 Call load posts earlier for infinite scroll

* Add check for max height for loading posts

* Fix issue of loading posts not getting triggered at right position
when lot of posts are loaded

  * scrollHeightAoveFoldForLoad can be really high when lot of posts are loaded
    comaparision with max value should be with scrollTop value

* Change the max-value of sctollTop to 2500 for triggering loadPosts
TranMacTien pushed a commit to Designveloper/mattermost-webapp that referenced this pull request Jun 13, 2019
* MM-13180 Call load posts earlier for infinite scroll

* Add check for max height for loading posts

* Fix issue of loading posts not getting triggered at right position
when lot of posts are loaded

  * scrollHeightAoveFoldForLoad can be really high when lot of posts are loaded
    comaparision with max value should be with scrollTop value

* Change the max-value of sctollTop to 2500 for triggering loadPosts
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed Does not require new release tests
Projects
None yet
8 participants