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

MM-14196 Add virtualised list for posts #2447

Merged
merged 13 commits into from
Mar 25, 2019

Conversation

sudheerDev
Copy link
Contributor

@sudheerDev sudheerDev commented Mar 5, 2019

Summary

Known issues

  • P1: First team switch does not reliably work because of channel sync issue.
  • slow network fast posting causes scroll pops upwards
  • text box expansion should not overlap post list content
  • compact view last post jumping (due to hover menu)

QA testing cases

Performance with lots of data:

  • busy channel with many incoming posts - verify no white space when scrolling or sitting at the bottom of the channel
  • Scroll near the top of a channel with 5000+ messages and then try to switch channels. Examine perf
  • Post a message when scrolled near the top of a channel with 5000+ messages
  • Click "new messages below" indicator at the top of a channel with lots of posts.

Worst case scenarios:

  • Editing posts and adding reactions to most recent post and posts up in the channel. Verify no flickering or scroll pop
  • Channels with images, message attchements, webhook message attachments, markdown, link previews and youtube previews, long posts. Verify no pop during post, scrolling or refresh

Scrolling smoothness:

  • Scroll quickly in one direction then quickly switch scrolling direction.
  • Permalinks to posts in channels that aren't loaded yet
  • test various browsers
  • various network speeds

Misc:

  • Permalinks
  • Jump to message from search
  • channel and team switching. Compare to community

Completed:

  • Link preview and SVG can throw off scroll correction a little. That will be addressed with MM-13099 Use size_aware_image component for when loading images in posts #2334 and MM-14500 Fix for pop caused by size_aware_image #2469
  • youtube previews cause LARGE pop
  • removing posts can cause blank space at the top of the channel
  • channel header missing on channels with few posts
  • In progress: Fix issue of flicker when a a new reaction is added (Does not happen always most likely related to the way style cache is busted).
  • scroll channel down to bottom when current user make a new post
  • Permalink view should move post to middle of screen and highlight
  • Remove padding below channel header
  • logic tweak for bottom of the channel so we never see unmounted posts (now shows 10 max)
  • Fix jumping to permalink when posts are not loaded yet
  • Scroll to bottom when adding reaction or editing most recent post
  • Youtube previews causing minor incorrect scroll correction
  • Don't scroll center channel to bottom when posting in the RHS
  • Loader can get stuck at the top of the screen if we can repro
  • minor scroll bar adjustment when entering the channel on subsequent visits
  • increase number of posts to 100 (in scrolling direction)- 50 (in opposite direction)
  • increase load threshold to 1000px
  • consecutive posts are being separated in center channel
  • stop loading two pages on first load (only load 30 posts)
  • deleting system messages leaves blank space
  • fix overlapping issue when making every alternate posts in a channel that has a lot of posts loaded
  • Manual loadtest of posts to verify this issue os only related to the /test command: Fix loading indicator stuck and showing blank posts above (see image below)

Ticket Link

MM-14196

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
  • Added or updated unit tests (required for all new features)
  • Has UI changes
  • Touches critical sections of the codebase (auth, posting, etc.)

@sudheerDev sudheerDev added the Work in Progress Not yet ready for review label Mar 5, 2019
@sudheerDev sudheerDev force-pushed the scroll/MM-14196 branch 2 times, most recently from d4a690d to 39c3ad1 Compare March 5, 2019 16:11
@sudheerDev sudheerDev added the Setup Old Test Server Triggers the creation of a test server label Mar 5, 2019
@mattermost mattermost deleted a comment from mattermod Mar 5, 2019
@mattermost mattermost deleted a comment from mattermod Mar 5, 2019
@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 Mar 5, 2019
@mattermost mattermost deleted a comment from mattermod Mar 5, 2019
@mattermost mattermost deleted a comment from mattermod Mar 5, 2019
@mattermost mattermost deleted a comment from mattermod Mar 5, 2019
@sudheerDev sudheerDev added the Setup Old Test Server Triggers the creation of a test server label Mar 5, 2019
@esethna
Copy link
Contributor

esethna commented Mar 6, 2019

Thanks Sudheer, this is looking really good. Some things I noticed:

  1. White bar at the top:

image

  1. Some channels are infinitely stuck in the loading more posts state (try in "autem" and "doloremque"):
    image

Edit: actually all channels seem to be doing this at some point far enough back in the channel

  1. The above ^ makes it a bit hard to test but I feel like we are loading much too early. Notice how the scroll bar starts to shrink almost immediately as I scroll up:

scroll

As far as I know the rules we put in place last time involved loading more posts at X% from the top. If you test Slack it has a much nicer feel because they seem to only load when you are almost at the top of the loaded posts (however I would say that they are loading a bit later than I would like us to do).

  1. I did not notice any unmounted posts (white space). Is the virtualized post list active or is it just done well so I'm not noticing it?

@esethna
Copy link
Contributor

esethna commented Mar 6, 2019

Added new to do list items to PR description ^

@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 Mar 11, 2019
@sudheerDev sudheerDev force-pushed the scroll/MM-14196 branch 2 times, most recently from 702cbb3 to a123b9f Compare March 12, 2019 19:17
@sudheerDev sudheerDev added the Setup Old Test Server Triggers the creation of a test server label Mar 12, 2019
@mattermost mattermost deleted a comment from mattermod Mar 12, 2019
@mattermost mattermost deleted a comment from mattermod Mar 12, 2019
@esethna esethna 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 Mar 12, 2019
@esethna
Copy link
Contributor

esethna commented Mar 12, 2019

Updated PR description with new P1 to dos before QA testing (discussed with ST in daily standup)

components/post_view/index.js Outdated Show resolved Hide resolved
@sudheerDev
Copy link
Contributor Author

sudheerDev commented Mar 25, 2019

@esethna @DHaussermann @ogi-m
P1 (before merging to master - needs new spinmint):

  • Fix regression with jumpiness on loading new posts - Done
  • Fix permalink view + jumping to post from search - Done

P2 (after merging to master):

components/post_view/index.js Show resolved Hide resolved
components/post_view/post/post.jsx Outdated Show resolved Hide resolved
components/post_view/post_list.jsx Outdated Show resolved Hide resolved
components/post_view/post_list.jsx Outdated Show resolved Hide resolved
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.

This is great 💯

@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 Mar 25, 2019
@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 Mar 25, 2019
@sudheerDev sudheerDev merged commit ec5c279 into mattermost:master Mar 25, 2019
@sudheerDev sudheerDev deleted the scroll/MM-14196 branch March 25, 2019 18:13
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Needed Requires documentation labels Mar 25, 2019
@hanzei hanzei removed the Setup Old Test Server Triggers the creation of a test server label Mar 25, 2019
stevepartridge pushed a commit to stevepartridge/mattermost-webapp that referenced this pull request Mar 30, 2019
* MM-14196 Add virtualised list for posts

 * Use custom fork of react-window for creating virtualised lists
 * Remove all of the scroll corrections and rely on resize observer
   in virtualised list to figure out the scroll corrections
 * Use 100px as the trigger for loading more posts

* Fix channel sync issue for first team switch

* Add tests
  * Fix review comments
  * Remove global event emitter
  * remove references to postlist scroll change

* Revert css change for post_hover menu

* Remove unused import

* Add inital value for floatingTimestamp

* Update react window has to fix mattermost#2447 (comment)
@esethna esethna self-assigned this Apr 3, 2019
@esethna esethna added Docs/Not Needed Does not require documentation and removed Docs/Needed Requires documentation labels Apr 3, 2019
@esethna
Copy link
Contributor

esethna commented Apr 3, 2019

@amyblais removing the docs needed label. In my mind infinite scroll/virtualized post list is expected functionality from a users perspective, rather than something that needs to be called out in docs. Feel free to re-add if you think there's something we should cover specifically.

sudheerDev added a commit to sudheerDev/mattermost-webapp that referenced this pull request Apr 5, 2019
sudheerDev added a commit that referenced this pull request Apr 8, 2019
* Revert "MM-14810 Fix for channel header loading in center channel (#2596)"

This reverts commit 6bc18af.

* Revert "MM-14761 Loading channel with lot of unreads should show loader at the top (#2589)"

This reverts commit 3e88c52.

* Revert "MM-14810 Fix for infinite loop caused by loading channels with content less than screen height (#2572)"

This reverts commit b11b42c.

* Revert "Expose virt list scroll variables to window (#2569)"

This reverts commit 4e7715a.

* Revert "MM-14820 Fix post menu on mobile view (#2576)"

This reverts commit 8bd8dda.

* Revert "Fix postlist connector when channel is undefined (#2573)"

This reverts commit 03177f2.

* Revert "MM-14800 Fix missing messages when switching to a team not previously loaded (#2562)"

This reverts commit 3537b03.

* Revert "MM-14196 Add virtualised list for posts (#2447)"

This reverts commit ec5c279.

* Revert infinite scroll from release 5.8v (#2303)

* Revert infinite scroll changes

* Fix an error with ScrollToBottomArrows

  * Fix the error of onClick with scrollToBottomArrow prop

* Remove unused variables
@lindy65 lindy65 removed 4: Reviews Complete All reviewers have approved the pull request 3: QA Review Requires review by a QA tester labels Apr 8, 2019
@lindy65 lindy65 modified the milestones: v5.10.0, v5.12.0 Apr 13, 2019
bradjcoughlin pushed a commit to bradjcoughlin/mattermost-webapp that referenced this pull request Apr 15, 2019
* Revert "MM-14810 Fix for channel header loading in center channel (mattermost#2596)"

This reverts commit 6bc18af.

* Revert "MM-14761 Loading channel with lot of unreads should show loader at the top (mattermost#2589)"

This reverts commit 3e88c52.

* Revert "MM-14810 Fix for infinite loop caused by loading channels with content less than screen height (mattermost#2572)"

This reverts commit b11b42c.

* Revert "Expose virt list scroll variables to window (mattermost#2569)"

This reverts commit 4e7715a.

* Revert "MM-14820 Fix post menu on mobile view (mattermost#2576)"

This reverts commit 8bd8dda.

* Revert "Fix postlist connector when channel is undefined (mattermost#2573)"

This reverts commit 03177f2.

* Revert "MM-14800 Fix missing messages when switching to a team not previously loaded (mattermost#2562)"

This reverts commit 3537b03.

* Revert "MM-14196 Add virtualised list for posts (mattermost#2447)"

This reverts commit ec5c279.

* Revert infinite scroll from release 5.8v (mattermost#2303)

* Revert infinite scroll changes

* Fix an error with ScrollToBottomArrows

  * Fix the error of onClick with scrollToBottomArrow prop

* Remove unused variables
TranMacTien pushed a commit to Designveloper/mattermost-webapp that referenced this pull request Jun 13, 2019
* Revert "MM-14810 Fix for channel header loading in center channel (mattermost#2596)"

This reverts commit 6bc18af.

* Revert "MM-14761 Loading channel with lot of unreads should show loader at the top (mattermost#2589)"

This reverts commit 3e88c52.

* Revert "MM-14810 Fix for infinite loop caused by loading channels with content less than screen height (mattermost#2572)"

This reverts commit b11b42c.

* Revert "Expose virt list scroll variables to window (mattermost#2569)"

This reverts commit 4e7715a.

* Revert "MM-14820 Fix post menu on mobile view (mattermost#2576)"

This reverts commit 8bd8dda.

* Revert "Fix postlist connector when channel is undefined (mattermost#2573)"

This reverts commit 03177f2.

* Revert "MM-14800 Fix missing messages when switching to a team not previously loaded (mattermost#2562)"

This reverts commit 3537b03.

* Revert "MM-14196 Add virtualised list for posts (mattermost#2447)"

This reverts commit ec5c279.

* Revert infinite scroll from release 5.8v (mattermost#2303)

* Revert infinite scroll changes

* Fix an error with ScrollToBottomArrows

  * Fix the error of onClick with scrollToBottomArrow prop

* Remove unused variables
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants