From b3f4abc8c1ac7d282ccc9b614d06220554891873 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Thu, 21 Apr 2022 21:50:12 -0500 Subject: [PATCH 1/9] Reimplement ScrollableList with Virtuoso --- app/soapbox/components/scrollable_list.js | 347 --------------------- app/soapbox/components/scrollable_list.tsx | 87 ++++++ 2 files changed, 87 insertions(+), 347 deletions(-) delete mode 100644 app/soapbox/components/scrollable_list.js create mode 100644 app/soapbox/components/scrollable_list.tsx diff --git a/app/soapbox/components/scrollable_list.js b/app/soapbox/components/scrollable_list.js deleted file mode 100644 index c1b8c1a52..000000000 --- a/app/soapbox/components/scrollable_list.js +++ /dev/null @@ -1,347 +0,0 @@ -import classNames from 'classnames'; -import { List as ImmutableList } from 'immutable'; -import { throttle } from 'lodash'; -import PropTypes from 'prop-types'; -import React, { PureComponent } from 'react'; -import { connect } from 'react-redux'; -import { withRouter } from 'react-router-dom'; - -import { getSettings } from 'soapbox/actions/settings'; -import PullToRefresh from 'soapbox/components/pull-to-refresh'; - -import IntersectionObserverArticleContainer from '../containers/intersection_observer_article_container'; -import IntersectionObserverWrapper from '../features/ui/util/intersection_observer_wrapper'; - -import LoadMore from './load_more'; -import MoreFollows from './more_follows'; -import { Spinner, Text } from './ui'; - -const MOUSE_IDLE_DELAY = 300; - -const mapStateToProps = state => { - const settings = getSettings(state); - - return { - autoload: settings.get('autoloadMore'), - }; -}; - -export default @connect(mapStateToProps, null, null, { forwardRef: true }) -@withRouter -class ScrollableList extends PureComponent { - - static propTypes = { - scrollKey: PropTypes.string.isRequired, - onLoadMore: PropTypes.func, - isLoading: PropTypes.bool, - showLoading: PropTypes.bool, - hasMore: PropTypes.bool, - diffCount: PropTypes.number, - prepend: PropTypes.node, - alwaysPrepend: PropTypes.bool, - emptyMessage: PropTypes.node, - children: PropTypes.node, - onScrollToTop: PropTypes.func, - onScroll: PropTypes.func, - placeholderComponent: PropTypes.object, - placeholderCount: PropTypes.number, - autoload: PropTypes.bool, - onRefresh: PropTypes.func, - className: PropTypes.string, - location: PropTypes.object, - }; - - state = { - cachedMediaWidth: 250, // Default media/card width using default theme - }; - - intersectionObserverWrapper = new IntersectionObserverWrapper(); - - mouseIdleTimer = null; - mouseMovedRecently = false; - lastScrollWasSynthetic = false; - scrollToTopOnMouseIdle = false; - - setScrollTop = newScrollTop => { - if (this.documentElement.scrollTop !== newScrollTop) { - this.lastScrollWasSynthetic = true; - this.documentElement.scrollTop = newScrollTop; - } - }; - - clearMouseIdleTimer = () => { - if (this.mouseIdleTimer === null) { - return; - } - - clearTimeout(this.mouseIdleTimer); - this.mouseIdleTimer = null; - }; - - handleMouseMove = throttle(() => { - // As long as the mouse keeps moving, clear and restart the idle timer. - this.clearMouseIdleTimer(); - this.mouseIdleTimer = setTimeout(this.handleMouseIdle, MOUSE_IDLE_DELAY); - - if (!this.mouseMovedRecently && this.documentElement.scrollTop === 0) { - // Only set if we just started moving and are scrolled to the top. - this.scrollToTopOnMouseIdle = true; - } - - // Save setting this flag for last, so we can do the comparison above. - this.mouseMovedRecently = true; - }, MOUSE_IDLE_DELAY / 2); - - handleMouseIdle = () => { - if (this.scrollToTopOnMouseIdle) { - this.setScrollTop(0); - } - - this.mouseMovedRecently = false; - this.scrollToTopOnMouseIdle = false; - } - - componentDidMount() { - this.window = window; - this.documentElement = document.scrollingElement || document.documentElement; - - this.attachScrollListener(); - this.attachIntersectionObserver(); - // Handle initial scroll position - this.handleScroll(); - } - - getScrollPosition = () => { - if (this.documentElement && (this.documentElement.scrollTop > 0 || this.mouseMovedRecently)) { - return { height: this.documentElement.scrollHeight, top: this.documentElement.scrollTop }; - } else { - return undefined; - } - } - - updateScrollBottom = (snapshot) => { - const newScrollTop = this.documentElement.scrollHeight - snapshot; - - this.setScrollTop(newScrollTop); - } - - componentDidUpdate(prevProps, prevState, snapshot) { - // Reset the scroll position when a new child comes in in order not to - // jerk the scrollbar around if you're already scrolled down the page. - if (snapshot !== null) { - this.setScrollTop(this.documentElement.scrollHeight - snapshot); - } - } - - attachScrollListener() { - this.window.addEventListener('scroll', this.handleScroll); - this.window.addEventListener('wheel', this.handleWheel); - } - - detachScrollListener() { - this.window.removeEventListener('scroll', this.handleScroll); - this.window.removeEventListener('wheel', this.handleWheel); - } - - handleScroll = throttle(() => { - const { autoload } = this.props; - - if (this.window) { - const { scrollTop, scrollHeight } = this.documentElement; - const { innerHeight } = this.window; - const offset = scrollHeight - scrollTop - innerHeight; - - if (autoload && 400 > offset && this.props.onLoadMore && this.props.hasMore && !this.props.isLoading) { - this.props.onLoadMore(); - } - - if (scrollTop < 100 && this.props.onScrollToTop) { - this.props.onScrollToTop(); - } else if (this.props.onScroll) { - this.props.onScroll(); - } - - if (!this.lastScrollWasSynthetic) { - // If the last scroll wasn't caused by setScrollTop(), assume it was - // intentional and cancel any pending scroll reset on mouse idle - this.scrollToTopOnMouseIdle = false; - } - this.lastScrollWasSynthetic = false; - } - }, 150, { - trailing: true, - }); - - handleWheel = throttle(() => { - this.scrollToTopOnMouseIdle = false; - }, 150, { - trailing: true, - }); - - getSnapshotBeforeUpdate(prevProps) { - const someItemInserted = React.Children.count(prevProps.children) > 0 && - React.Children.count(prevProps.children) < React.Children.count(this.props.children) && - this.getFirstChildKey(prevProps) !== this.getFirstChildKey(this.props); - - if (someItemInserted && (this.documentElement.scrollTop > 0 || this.mouseMovedRecently)) { - return this.documentElement.scrollHeight - this.documentElement.scrollTop; - } else { - return null; - } - } - - cacheMediaWidth = (width) => { - if (width && this.state.cachedMediaWidth !== width) { - this.setState({ cachedMediaWidth: width }); - } - } - - componentWillUnmount() { - this.clearMouseIdleTimer(); - this.detachScrollListener(); - this.detachIntersectionObserver(); - } - - attachIntersectionObserver() { - this.intersectionObserverWrapper.connect(); - } - - detachIntersectionObserver() { - this.intersectionObserverWrapper.disconnect(); - } - - getFirstChildKey(props) { - const { children } = props; - let firstChild = children; - - if (children instanceof ImmutableList) { - firstChild = children.get(0); - } else if (Array.isArray(children)) { - firstChild = children[0]; - } - - return firstChild && firstChild.key; - } - - handleLoadMore = e => { - e.preventDefault(); - this.props.onLoadMore(); - } - - getMoreFollows = () => { - const { scrollKey, isLoading, diffCount, hasMore } = this.props; - const isMoreFollows = ['followers', 'following'].some(k => k === scrollKey); - if (!(diffCount && isMoreFollows)) return null; - if (hasMore) return null; - - return ( - - ); - } - - setRef = c => { - this.node = c; - } - - renderLoading = () => { - const { className, prepend, placeholderComponent: Placeholder, placeholderCount } = this.props; - - if (Placeholder && placeholderCount > 0) { - return ( -
- {Array(placeholderCount).fill().map((_, i) => ( - - ))} -
- ); - } - - return ( -
-
- {prepend} -
- -
- -
-
- ); - } - - renderEmptyMessage = () => { - const { className, prepend, alwaysPrepend, emptyMessage } = this.props; - - return ( -
- {alwaysPrepend && prepend} - -
- {emptyMessage} -
-
- ); - } - - renderFeed = () => { - const { className, children, scrollKey, isLoading, hasMore, prepend, onLoadMore, onRefresh, placeholderComponent: Placeholder } = this.props; - const childrenCount = React.Children.count(children); - const trackScroll = true; //placeholder - const loadMore = (hasMore && onLoadMore) ? : null; - - const feed = ( -
-
- {prepend} - - {React.Children.map(children, (child, index) => ( - - {React.cloneElement(child, { - getScrollPosition: this.getScrollPosition, - updateScrollBottom: this.updateScrollBottom, - cachedMediaWidth: this.state.cachedMediaWidth, - cacheMediaWidth: this.cacheMediaWidth, - })} - - ))} - {(isLoading && Placeholder) && ( - - )} - {this.getMoreFollows()} - {loadMore} -
-
- ); - - if (onRefresh) { - return ( - - {feed} - - ); - } else { - return feed; - } - } - - render() { - const { children, showLoading, isLoading, hasMore, emptyMessage } = this.props; - const childrenCount = React.Children.count(children); - - if (showLoading) { - return this.renderLoading(); - } else if (isLoading || childrenCount > 0 || hasMore || !emptyMessage) { - return this.renderFeed(); - } else { - return this.renderEmptyMessage(); - } - } - -} diff --git a/app/soapbox/components/scrollable_list.tsx b/app/soapbox/components/scrollable_list.tsx new file mode 100644 index 000000000..e65a03bbd --- /dev/null +++ b/app/soapbox/components/scrollable_list.tsx @@ -0,0 +1,87 @@ +import React from 'react'; +import { Virtuoso } from 'react-virtuoso'; + +import PullToRefresh from 'soapbox/components/pull-to-refresh'; +// import { useSettings } from 'soapbox/hooks'; + +import { Spinner, Text } from './ui'; + +const List = React.forwardRef((props: any, ref: React.ForwardedRef) => { + const { context, ...rest } = props; + return
; +}); + +interface IScrollableList { + scrollKey?: string, + onLoadMore?: () => void, + isLoading?: boolean, + showLoading?: boolean, + hasMore?: boolean, + prepend?: React.ReactElement, + alwaysPrepend?: boolean, + emptyMessage?: React.ReactNode, + children: Iterable, + onScrollToTop?: () => void, + onScroll?: () => void, + placeholderComponent?: React.ComponentType, + placeholderCount?: number, + onRefresh?: () => Promise, + className?: string, +} + +const ScrollableList: React.FC = ({ + prepend = null, + children, + isLoading, + emptyMessage, + showLoading, + onRefresh, + onScroll, + onScrollToTop, + onLoadMore, + className, + placeholderComponent: Placeholder, + placeholderCount = 0, +}) => { + // const settings = useSettings(); + // const autoload = settings.get('autoloadMore'); + + const showPlaceholder = showLoading && Placeholder && placeholderCount > 0; + + const renderItem = (_i: number, element: JSX.Element) => { + if (showPlaceholder) { + return ; + } else { + return element; + } + }; + + return ( + + isScrolling && onScroll && onScroll()} + itemContent={renderItem} + context={{ + listClassName: className, + }} + components={{ + Header: () => prepend, + ScrollSeekPlaceholder: Placeholder as any, + EmptyPlaceholder: () => isLoading ? ( + + ) : ( + {emptyMessage} + ), + List, + }} + /> + + ); +}; + +export default ScrollableList; From 0d463bbbd18c4d04bf297bb17e412617c2e0a0a2 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 22 Apr 2022 00:49:11 -0500 Subject: [PATCH 2/9] ScrollableList: fix empty children --- app/soapbox/components/scrollable_list.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/soapbox/components/scrollable_list.tsx b/app/soapbox/components/scrollable_list.tsx index e65a03bbd..d38706437 100644 --- a/app/soapbox/components/scrollable_list.tsx +++ b/app/soapbox/components/scrollable_list.tsx @@ -61,7 +61,7 @@ const ScrollableList: React.FC = ({ isScrolling && onScroll && onScroll()} From a8c306e62bb68b259d7a43aee6fda8c2d3b69bf0 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 22 Apr 2022 10:23:53 -0500 Subject: [PATCH 3/9] ScrollableList: add placeholder footer, fix "empty" state --- app/soapbox/components/scrollable_list.tsx | 41 ++++++++++++++++++---- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/app/soapbox/components/scrollable_list.tsx b/app/soapbox/components/scrollable_list.tsx index d38706437..ce35228c8 100644 --- a/app/soapbox/components/scrollable_list.tsx +++ b/app/soapbox/components/scrollable_list.tsx @@ -6,6 +6,7 @@ import PullToRefresh from 'soapbox/components/pull-to-refresh'; import { Spinner, Text } from './ui'; +// Ensure the className winds up here const List = React.forwardRef((props: any, ref: React.ForwardedRef) => { const { context, ...rest } = props; return
; @@ -29,8 +30,10 @@ interface IScrollableList { className?: string, } +/** Legacy ScrollableList with Virtuoso for backwards-compatibility */ const ScrollableList: React.FC = ({ prepend = null, + alwaysPrepend, children, isLoading, emptyMessage, @@ -40,6 +43,7 @@ const ScrollableList: React.FC = ({ onScrollToTop, onLoadMore, className, + hasMore, placeholderComponent: Placeholder, placeholderCount = 0, }) => { @@ -47,7 +51,26 @@ const ScrollableList: React.FC = ({ // const autoload = settings.get('autoloadMore'); const showPlaceholder = showLoading && Placeholder && placeholderCount > 0; + const data = showPlaceholder ? Array(placeholderCount).fill('') : Array.from(children || []); + /* Render an empty state instead of the scrollable list */ + const renderEmpty = () => { + return ( +
+ {alwaysPrepend && prepend} + +
+ {isLoading ? ( + + ) : ( + {emptyMessage} + )} +
+
+ ); + }; + + /** Render the actual item */ const renderItem = (_i: number, element: JSX.Element) => { if (showPlaceholder) { return ; @@ -56,12 +79,22 @@ const ScrollableList: React.FC = ({ } }; + // Don't use Virtuoso's EmptyPlaceholder component so it preserves height + if (data.length === 0) { + return renderEmpty(); + } + + // Don't use Virtuoso's Footer component so it preserves spacing + if (hasMore && Placeholder) { + data.push(); + } + return ( isScrolling && onScroll && onScroll()} @@ -72,11 +105,7 @@ const ScrollableList: React.FC = ({ components={{ Header: () => prepend, ScrollSeekPlaceholder: Placeholder as any, - EmptyPlaceholder: () => isLoading ? ( - - ) : ( - {emptyMessage} - ), + EmptyPlaceholder: () => renderEmpty(), List, }} /> From ae48cb2c066ea6bad1e437898937f730cc3f2985 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 22 Apr 2022 12:24:09 -0500 Subject: [PATCH 4/9] ScrollableList: replace margins with padding --- app/soapbox/components/scrollable_list.tsx | 24 +++++++++++++++++-- app/soapbox/components/status_list.js | 8 ++++++- app/soapbox/features/admin/user_index.js | 3 ++- app/soapbox/features/blocks/index.tsx | 4 ++-- app/soapbox/features/followers/index.js | 2 +- app/soapbox/features/following/index.js | 2 +- app/soapbox/features/mutes/index.tsx | 2 +- .../ui/components/birthdays_modal.tsx | 2 +- .../ui/components/favourites_modal.js | 2 +- .../features/ui/components/mentions_modal.js | 2 +- .../ui/components/reactions_modal.tsx | 2 +- .../features/ui/components/reblogs_modal.js | 2 +- 12 files changed, 41 insertions(+), 14 deletions(-) diff --git a/app/soapbox/components/scrollable_list.tsx b/app/soapbox/components/scrollable_list.tsx index ce35228c8..e30989627 100644 --- a/app/soapbox/components/scrollable_list.tsx +++ b/app/soapbox/components/scrollable_list.tsx @@ -6,10 +6,26 @@ import PullToRefresh from 'soapbox/components/pull-to-refresh'; import { Spinner, Text } from './ui'; +type Context = { + itemClassName?: string, + listClassName?: string, +} + +type VComponent = { + context?: Context, +} + +// NOTE: It's crucial to space lists with **padding** instead of margin! +// Pass an `itemClassName` like `pb-3`, NOT a `space-y-3` className +// https://virtuoso.dev/troubleshooting#list-does-not-scroll-to-the-bottom--items-jump-around +const Item: React.FC = ({ context, ...rest }) => ( +
+); + // Ensure the className winds up here -const List = React.forwardRef((props: any, ref: React.ForwardedRef) => { +const List = React.forwardRef((props, ref) => { const { context, ...rest } = props; - return
; + return
; }); interface IScrollableList { @@ -28,6 +44,7 @@ interface IScrollableList { placeholderCount?: number, onRefresh?: () => Promise, className?: string, + itemClassName?: string, } /** Legacy ScrollableList with Virtuoso for backwards-compatibility */ @@ -43,6 +60,7 @@ const ScrollableList: React.FC = ({ onScrollToTop, onLoadMore, className, + itemClassName, hasMore, placeholderComponent: Placeholder, placeholderCount = 0, @@ -101,12 +119,14 @@ const ScrollableList: React.FC = ({ itemContent={renderItem} context={{ listClassName: className, + itemClassName, }} components={{ Header: () => prepend, ScrollSeekPlaceholder: Placeholder as any, EmptyPlaceholder: () => renderEmpty(), List, + Item, }} /> diff --git a/app/soapbox/components/status_list.js b/app/soapbox/components/status_list.js index 327820f6f..a1f35b841 100644 --- a/app/soapbox/components/status_list.js +++ b/app/soapbox/components/status_list.js @@ -1,3 +1,4 @@ +import classNames from 'classnames'; import { debounce } from 'lodash'; import PropTypes from 'prop-types'; import React from 'react'; @@ -222,7 +223,12 @@ export default class StatusList extends ImmutablePureComponent { placeholderComponent={PlaceholderStatus} placeholderCount={20} ref={this.setRef} - className={divideType === 'border' ? 'divide-y divide-solid divide-gray-200 dark:divide-gray-800' : 'sm:space-y-3 divide-y divide-solid divide-gray-200 dark:divide-gray-800 sm:divide-none'} + className={classNames('divide-y divide-solid divide-gray-200 dark:divide-slate-700', { + 'sm:divide-none': divideType !== 'border', + })} + itemClassName={classNames({ + 'sm:pb-3': divideType !== 'border', + })} {...other} > {this.renderScrollableContent()} diff --git a/app/soapbox/features/admin/user_index.js b/app/soapbox/features/admin/user_index.js index dc7691f69..eeeba734a 100644 --- a/app/soapbox/features/admin/user_index.js +++ b/app/soapbox/features/admin/user_index.js @@ -116,7 +116,8 @@ class UserIndex extends ImmutablePureComponent { showLoading={showLoading} onLoadMore={this.handleLoadMore} emptyMessage={intl.formatMessage(messages.empty)} - className='mt-4 space-y-4' + className='mt-4' + itemClassName='pb-4' > {accountIds.map(id => , diff --git a/app/soapbox/features/blocks/index.tsx b/app/soapbox/features/blocks/index.tsx index cdc3b0a98..90e41bf65 100644 --- a/app/soapbox/features/blocks/index.tsx +++ b/app/soapbox/features/blocks/index.tsx @@ -45,7 +45,7 @@ const Blocks: React.FC = () => { onLoadMore={() => handleLoadMore(dispatch)} hasMore={hasMore} emptyMessage={emptyMessage} - className='space-y-4' + itemClassName='pb-4' > {accountIds.map((id: string) => , @@ -55,4 +55,4 @@ const Blocks: React.FC = () => { ); }; -export default Blocks; \ No newline at end of file +export default Blocks; diff --git a/app/soapbox/features/followers/index.js b/app/soapbox/features/followers/index.js index aa7067b5c..1a98cb561 100644 --- a/app/soapbox/features/followers/index.js +++ b/app/soapbox/features/followers/index.js @@ -125,7 +125,7 @@ class Followers extends ImmutablePureComponent { diffCount={diffCount} onLoadMore={this.handleLoadMore} emptyMessage={} - className='space-y-4' + itemClassName='pb-4' > {accountIds.map(id => , diff --git a/app/soapbox/features/following/index.js b/app/soapbox/features/following/index.js index cfa7c2741..7ab00a7b9 100644 --- a/app/soapbox/features/following/index.js +++ b/app/soapbox/features/following/index.js @@ -125,7 +125,7 @@ class Following extends ImmutablePureComponent { diffCount={diffCount} onLoadMore={this.handleLoadMore} emptyMessage={} - className='space-y-4' + itemClassName='pb-4' > {accountIds.map(id => , diff --git a/app/soapbox/features/mutes/index.tsx b/app/soapbox/features/mutes/index.tsx index c1f306c24..06bf89e21 100644 --- a/app/soapbox/features/mutes/index.tsx +++ b/app/soapbox/features/mutes/index.tsx @@ -45,7 +45,7 @@ const Mutes: React.FC = () => { onLoadMore={() => handleLoadMore(dispatch)} hasMore={hasMore} emptyMessage={emptyMessage} - className='space-y-4' + itemClassName='pb-4' > {accountIds.map((id: string) => , diff --git a/app/soapbox/features/ui/components/birthdays_modal.tsx b/app/soapbox/features/ui/components/birthdays_modal.tsx index 8977c95ee..6c43bd7b4 100644 --- a/app/soapbox/features/ui/components/birthdays_modal.tsx +++ b/app/soapbox/features/ui/components/birthdays_modal.tsx @@ -28,7 +28,7 @@ const BirthdaysModal = ({ onClose }: IBirthdaysModal) => { {accountIds.map(id => , diff --git a/app/soapbox/features/ui/components/favourites_modal.js b/app/soapbox/features/ui/components/favourites_modal.js index 27ee0f16f..34e6ceb34 100644 --- a/app/soapbox/features/ui/components/favourites_modal.js +++ b/app/soapbox/features/ui/components/favourites_modal.js @@ -54,7 +54,7 @@ class FavouritesModal extends React.PureComponent { {accountIds.map(id => , diff --git a/app/soapbox/features/ui/components/mentions_modal.js b/app/soapbox/features/ui/components/mentions_modal.js index 6b6746eda..0f4c4626b 100644 --- a/app/soapbox/features/ui/components/mentions_modal.js +++ b/app/soapbox/features/ui/components/mentions_modal.js @@ -61,7 +61,7 @@ class MentionsModal extends React.PureComponent { body = ( {accountIds.map(id => , diff --git a/app/soapbox/features/ui/components/reactions_modal.tsx b/app/soapbox/features/ui/components/reactions_modal.tsx index 72f5eddc9..31422ab05 100644 --- a/app/soapbox/features/ui/components/reactions_modal.tsx +++ b/app/soapbox/features/ui/components/reactions_modal.tsx @@ -87,7 +87,7 @@ const ReactionsModal: React.FC = ({ onClose, statusId, ...props {accounts.map((account) => , diff --git a/app/soapbox/features/ui/components/reblogs_modal.js b/app/soapbox/features/ui/components/reblogs_modal.js index 37feaf1dc..a5945c3a1 100644 --- a/app/soapbox/features/ui/components/reblogs_modal.js +++ b/app/soapbox/features/ui/components/reblogs_modal.js @@ -72,7 +72,7 @@ class ReblogsModal extends React.PureComponent { {accountIds.map(id => , From 730556a692090dba0ac8f01841a4f92142cfb7cc Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 22 Apr 2022 12:27:37 -0500 Subject: [PATCH 5/9] ScrollableList: render a spinner if placeholder isn't provided --- app/soapbox/components/scrollable_list.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/soapbox/components/scrollable_list.tsx b/app/soapbox/components/scrollable_list.tsx index e30989627..b1626d966 100644 --- a/app/soapbox/components/scrollable_list.tsx +++ b/app/soapbox/components/scrollable_list.tsx @@ -105,6 +105,8 @@ const ScrollableList: React.FC = ({ // Don't use Virtuoso's Footer component so it preserves spacing if (hasMore && Placeholder) { data.push(); + } else if (hasMore) { + data.push(); } return ( From 083b3c22d0fb89e0f529416a554364433d832f06 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 22 Apr 2022 12:56:06 -0500 Subject: [PATCH 6/9] ScrollableList: render emptyMessage inside the PullToRefresh --- app/soapbox/components/scrollable_list.tsx | 85 +++++++++++++--------- 1 file changed, 51 insertions(+), 34 deletions(-) diff --git a/app/soapbox/components/scrollable_list.tsx b/app/soapbox/components/scrollable_list.tsx index b1626d966..3264c5369 100644 --- a/app/soapbox/components/scrollable_list.tsx +++ b/app/soapbox/components/scrollable_list.tsx @@ -68,11 +68,26 @@ const ScrollableList: React.FC = ({ // const settings = useSettings(); // const autoload = settings.get('autoloadMore'); + /** Normalized children */ + const elements = Array.from(children || []); + const showPlaceholder = showLoading && Placeholder && placeholderCount > 0; - const data = showPlaceholder ? Array(placeholderCount).fill('') : Array.from(children || []); + + // NOTE: We are doing some trickery to load a feed of placeholders + // Virtuoso's `EmptyPlaceholder` unfortunately doesn't work for our use-case + const data = showPlaceholder ? Array(placeholderCount).fill('') : elements; + const isEmpty = data.length === 0; // Yes, if it has placeholders it isn't "empty" + + // Add a placeholder at the bottom for loading + // (Don't use Virtuoso's `Footer` component because it doesn't preserve its height) + if (hasMore && Placeholder) { + data.push(); + } else if (hasMore) { + data.push(); + } /* Render an empty state instead of the scrollable list */ - const renderEmpty = () => { + const renderEmpty = (): JSX.Element => { return (
{alwaysPrepend && prepend} @@ -88,8 +103,8 @@ const ScrollableList: React.FC = ({ ); }; - /** Render the actual item */ - const renderItem = (_i: number, element: JSX.Element) => { + /** Render a single item */ + const renderItem = (_i: number, element: JSX.Element): JSX.Element => { if (showPlaceholder) { return ; } else { @@ -97,40 +112,42 @@ const ScrollableList: React.FC = ({ } }; - // Don't use Virtuoso's EmptyPlaceholder component so it preserves height - if (data.length === 0) { - return renderEmpty(); - } + /** Render the actual Virtuoso list */ + const renderFeed = (): JSX.Element => ( + isScrolling && onScroll && onScroll()} + itemContent={renderItem} + context={{ + listClassName: className, + itemClassName, + }} + components={{ + Header: () => prepend, + ScrollSeekPlaceholder: Placeholder as any, + EmptyPlaceholder: () => renderEmpty(), + List, + Item, + }} + /> + ); - // Don't use Virtuoso's Footer component so it preserves spacing - if (hasMore && Placeholder) { - data.push(); - } else if (hasMore) { - data.push(); - } + /** Conditionally render inner elements */ + const renderBody = (): JSX.Element => { + if (isEmpty) { + return renderEmpty(); + } else { + return renderFeed(); + } + }; return ( - isScrolling && onScroll && onScroll()} - itemContent={renderItem} - context={{ - listClassName: className, - itemClassName, - }} - components={{ - Header: () => prepend, - ScrollSeekPlaceholder: Placeholder as any, - EmptyPlaceholder: () => renderEmpty(), - List, - Item, - }} - /> + {renderBody()} ); }; From 006e253e23b626022ef26190e003d717d989418c Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 22 Apr 2022 13:00:12 -0500 Subject: [PATCH 7/9] ScrollableList: Don't support `autoloadMore` for now This setting allowed disabling endless scrolling, and would display a clickable "load more" button at the bottom of the feed. --- app/soapbox/components/scrollable_list.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/soapbox/components/scrollable_list.tsx b/app/soapbox/components/scrollable_list.tsx index 3264c5369..0361da981 100644 --- a/app/soapbox/components/scrollable_list.tsx +++ b/app/soapbox/components/scrollable_list.tsx @@ -2,7 +2,6 @@ import React from 'react'; import { Virtuoso } from 'react-virtuoso'; import PullToRefresh from 'soapbox/components/pull-to-refresh'; -// import { useSettings } from 'soapbox/hooks'; import { Spinner, Text } from './ui'; @@ -65,9 +64,6 @@ const ScrollableList: React.FC = ({ placeholderComponent: Placeholder, placeholderCount = 0, }) => { - // const settings = useSettings(); - // const autoload = settings.get('autoloadMore'); - /** Normalized children */ const elements = Array.from(children || []); From da17214a0b3dde0fe4530a5388717824329fd558 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 22 Apr 2022 13:05:40 -0500 Subject: [PATCH 8/9] Fix Notifications pagination --- app/soapbox/features/notifications/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/soapbox/features/notifications/index.js b/app/soapbox/features/notifications/index.js index 69c698274..4437e47f8 100644 --- a/app/soapbox/features/notifications/index.js +++ b/app/soapbox/features/notifications/index.js @@ -156,16 +156,18 @@ class Notifications extends React.PureComponent { ? () : null; + const showLoading = isLoading && !notifications || notifications.isEmpty(); + const scrollContainer = ( isScrolling && this.handleScroll()} itemContent={(_index, notification) => ( - isLoading ? ( + showLoading ? ( ) : ( Date: Fri, 22 Apr 2022 13:13:40 -0500 Subject: [PATCH 9/9] DELETE INTERSECTION OBSERVER ARTICLE --- app/soapbox/actions/height_cache.js | 17 --- .../intersection_observer_article.js | 131 ------------------ ...intersection_observer_article_container.js | 18 --- app/soapbox/features/ui/index.js | 12 +- .../features/ui/util/get_rect_from_entry.js | 21 --- .../ui/util/intersection_observer_wrapper.js | 57 -------- .../reducers/__tests__/height_cache-test.js | 14 -- app/soapbox/reducers/height_cache.js | 24 ---- app/soapbox/reducers/index.ts | 2 - 9 files changed, 1 insertion(+), 295 deletions(-) delete mode 100644 app/soapbox/actions/height_cache.js delete mode 100644 app/soapbox/components/intersection_observer_article.js delete mode 100644 app/soapbox/containers/intersection_observer_article_container.js delete mode 100644 app/soapbox/features/ui/util/get_rect_from_entry.js delete mode 100644 app/soapbox/features/ui/util/intersection_observer_wrapper.js delete mode 100644 app/soapbox/reducers/__tests__/height_cache-test.js diff --git a/app/soapbox/actions/height_cache.js b/app/soapbox/actions/height_cache.js deleted file mode 100644 index 74b8dd398..000000000 --- a/app/soapbox/actions/height_cache.js +++ /dev/null @@ -1,17 +0,0 @@ -export const HEIGHT_CACHE_SET = 'HEIGHT_CACHE_SET'; -export const HEIGHT_CACHE_CLEAR = 'HEIGHT_CACHE_CLEAR'; - -export function setHeight(key, id, height) { - return { - type: HEIGHT_CACHE_SET, - key, - id, - height, - }; -} - -export function clearHeight() { - return { - type: HEIGHT_CACHE_CLEAR, - }; -} diff --git a/app/soapbox/components/intersection_observer_article.js b/app/soapbox/components/intersection_observer_article.js deleted file mode 100644 index 96c1a5484..000000000 --- a/app/soapbox/components/intersection_observer_article.js +++ /dev/null @@ -1,131 +0,0 @@ -import { is } from 'immutable'; -import PropTypes from 'prop-types'; -import React from 'react'; - -import getRectFromEntry from '../features/ui/util/get_rect_from_entry'; -import scheduleIdleTask from '../features/ui/util/schedule_idle_task'; - -// Diff these props in the "rendered" state -const updateOnPropsForRendered = ['id', 'index', 'listLength']; -// Diff these props in the "unrendered" state -const updateOnPropsForUnrendered = ['id', 'index', 'listLength', 'cachedHeight']; - -export default class IntersectionObserverArticle extends React.Component { - - static propTypes = { - intersectionObserverWrapper: PropTypes.object.isRequired, - id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), - index: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), - listLength: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), - saveHeightKey: PropTypes.string, - cachedHeight: PropTypes.number, - onHeightChange: PropTypes.func, - children: PropTypes.node, - }; - - state = { - isHidden: false, // set to true in requestIdleCallback to trigger un-render - isIntersecting: true, - } - - shouldComponentUpdate(nextProps, nextState) { - const isUnrendered = !this.state.isIntersecting && (this.state.isHidden || this.props.cachedHeight); - const willBeUnrendered = !nextState.isIntersecting && (nextState.isHidden || nextProps.cachedHeight); - if (!!isUnrendered !== !!willBeUnrendered) { - // If we're going from rendered to unrendered (or vice versa) then update - return true; - } - // Otherwise, diff based on props - const propsToDiff = isUnrendered ? updateOnPropsForUnrendered : updateOnPropsForRendered; - return !propsToDiff.every(prop => is(nextProps[prop], this.props[prop])); - } - - componentDidMount() { - const { intersectionObserverWrapper, id } = this.props; - - intersectionObserverWrapper.observe( - id, - this.node, - this.handleIntersection, - ); - - this.componentMounted = true; - } - - componentWillUnmount() { - const { intersectionObserverWrapper, id } = this.props; - intersectionObserverWrapper.unobserve(id, this.node); - - this.componentMounted = false; - } - - handleIntersection = (entry) => { - this.entry = entry; - - scheduleIdleTask(this.calculateHeight); - this.setState(this.updateStateAfterIntersection); - } - - updateStateAfterIntersection = (prevState) => { - if (prevState.isIntersecting !== false && !this.entry.isIntersecting) { - scheduleIdleTask(this.hideIfNotIntersecting); - } - return { - isIntersecting: this.entry.isIntersecting, - isHidden: false, - }; - } - - calculateHeight = () => { - const { onHeightChange, saveHeightKey, id } = this.props; - // save the height of the fully-rendered element (this is expensive - // on Chrome, where we need to fall back to getBoundingClientRect) - this.height = getRectFromEntry(this.entry).height; - - if (onHeightChange && saveHeightKey) { - onHeightChange(saveHeightKey, id, this.height); - } - } - - hideIfNotIntersecting = () => { - if (!this.componentMounted) { - return; - } - - // When the browser gets a chance, test if we're still not intersecting, - // and if so, set our isHidden to true to trigger an unrender. The point of - // this is to save DOM nodes and avoid using up too much memory. - this.setState((prevState) => ({ isHidden: !prevState.isIntersecting })); - } - - handleRef = (node) => { - this.node = node; - } - - render() { - const { children, id, index, listLength, cachedHeight } = this.props; - const { isIntersecting, isHidden } = this.state; - - if (!isIntersecting && (isHidden || cachedHeight)) { - return ( -
- {children && React.cloneElement(children, { hidden: true })} -
- ); - } - - return ( -
- {children && React.cloneElement(children, { hidden: false })} -
- ); - } - -} diff --git a/app/soapbox/containers/intersection_observer_article_container.js b/app/soapbox/containers/intersection_observer_article_container.js deleted file mode 100644 index a112069c6..000000000 --- a/app/soapbox/containers/intersection_observer_article_container.js +++ /dev/null @@ -1,18 +0,0 @@ -import { connect } from 'react-redux'; - -import { setHeight } from '../actions/height_cache'; -import IntersectionObserverArticle from '../components/intersection_observer_article'; - -const makeMapStateToProps = (state, props) => ({ - cachedHeight: state.getIn(['height_cache', props.saveHeightKey, props.id]), -}); - -const mapDispatchToProps = (dispatch) => ({ - - onHeightChange(key, id, height) { - dispatch(setHeight(key, id, height)); - }, - -}); - -export default connect(makeMapStateToProps, mapDispatchToProps)(IntersectionObserverArticle); diff --git a/app/soapbox/features/ui/index.js b/app/soapbox/features/ui/index.js index f0761f1b1..767325638 100644 --- a/app/soapbox/features/ui/index.js +++ b/app/soapbox/features/ui/index.js @@ -35,7 +35,6 @@ import { fetchFollowRequests } from '../../actions/accounts'; import { fetchReports, fetchUsers, fetchConfig } from '../../actions/admin'; import { uploadCompose, resetCompose } from '../../actions/compose'; import { fetchFilters } from '../../actions/filters'; -import { clearHeight } from '../../actions/height_cache'; import { openModal } from '../../actions/modals'; import { expandNotifications } from '../../actions/notifications'; import { fetchScheduledStatuses } from '../../actions/scheduled_statuses'; @@ -187,7 +186,6 @@ class SwitchingColumnsArea extends React.PureComponent { static propTypes = { children: PropTypes.node, location: PropTypes.object, - onLayoutChange: PropTypes.func.isRequired, soapbox: ImmutablePropTypes.record.isRequired, features: PropTypes.object.isRequired, }; @@ -205,9 +203,6 @@ class SwitchingColumnsArea extends React.PureComponent { } handleResize = debounce(() => { - // The cached heights are no longer accurate, invalidate - this.props.onLayoutChange(); - this.setState({ mobile: isMobile(window.innerWidth) }); }, 500, { trailing: true, @@ -408,11 +403,6 @@ class UI extends React.PureComponent { mobile: isMobile(window.innerWidth), }; - handleLayoutChange = () => { - // The cached heights are no longer accurate, invalidate - this.props.dispatch(clearHeight()); - } - handleDragEnter = (e) => { e.preventDefault(); @@ -756,7 +746,7 @@ class UI extends React.PureComponent {
- + {children} diff --git a/app/soapbox/features/ui/util/get_rect_from_entry.js b/app/soapbox/features/ui/util/get_rect_from_entry.js deleted file mode 100644 index c266cd7dc..000000000 --- a/app/soapbox/features/ui/util/get_rect_from_entry.js +++ /dev/null @@ -1,21 +0,0 @@ - -// Get the bounding client rect from an IntersectionObserver entry. -// This is to work around a bug in Chrome: https://crbug.com/737228 - -let hasBoundingRectBug; - -function getRectFromEntry(entry) { - if (typeof hasBoundingRectBug !== 'boolean') { - const boundingRect = entry.target.getBoundingClientRect(); - const observerRect = entry.boundingClientRect; - hasBoundingRectBug = boundingRect.height !== observerRect.height || - boundingRect.top !== observerRect.top || - boundingRect.width !== observerRect.width || - boundingRect.bottom !== observerRect.bottom || - boundingRect.left !== observerRect.left || - boundingRect.right !== observerRect.right; - } - return hasBoundingRectBug ? entry.target.getBoundingClientRect() : entry.boundingClientRect; -} - -export default getRectFromEntry; diff --git a/app/soapbox/features/ui/util/intersection_observer_wrapper.js b/app/soapbox/features/ui/util/intersection_observer_wrapper.js deleted file mode 100644 index 3a85894a8..000000000 --- a/app/soapbox/features/ui/util/intersection_observer_wrapper.js +++ /dev/null @@ -1,57 +0,0 @@ -// Wrapper for IntersectionObserver in order to make working with it -// a bit easier. We also follow this performance advice: -// "If you need to observe multiple elements, it is both possible and -// advised to observe multiple elements using the same IntersectionObserver -// instance by calling observe() multiple times." -// https://developers.google.com/web/updates/2016/04/intersectionobserver - -class IntersectionObserverWrapper { - - callbacks = {}; - observerBacklog = []; - observer = null; - - connect(options) { - const onIntersection = (entries) => { - entries.forEach(entry => { - const id = entry.target.getAttribute('data-id'); - if (this.callbacks[id]) { - this.callbacks[id](entry); - } - }); - }; - - this.observer = new IntersectionObserver(onIntersection, options); - this.observerBacklog.forEach(([ id, node, callback ]) => { - this.observe(id, node, callback); - }); - this.observerBacklog = null; - } - - observe(id, node, callback) { - if (!this.observer) { - this.observerBacklog.push([ id, node, callback ]); - } else { - this.callbacks[id] = callback; - this.observer.observe(node); - } - } - - unobserve(id, node) { - if (this.observer) { - delete this.callbacks[id]; - this.observer.unobserve(node); - } - } - - disconnect() { - if (this.observer) { - this.callbacks = {}; - this.observer.disconnect(); - this.observer = null; - } - } - -} - -export default IntersectionObserverWrapper; diff --git a/app/soapbox/reducers/__tests__/height_cache-test.js b/app/soapbox/reducers/__tests__/height_cache-test.js deleted file mode 100644 index 4b3ed8c21..000000000 --- a/app/soapbox/reducers/__tests__/height_cache-test.js +++ /dev/null @@ -1,14 +0,0 @@ -import { Map as ImmutableMap } from 'immutable'; - -import reducer from '../height_cache'; -import { HEIGHT_CACHE_CLEAR } from '../height_cache'; - -describe('height_cache reducer', () => { - it('should return the initial state', () => { - expect(reducer(undefined, {})).toEqual(ImmutableMap()); - }); - - it('should handle HEIGHT_CACHE_CLEAR', () => { - expect(reducer(undefined, { type: HEIGHT_CACHE_CLEAR })).toEqual(ImmutableMap()); - }); -}); diff --git a/app/soapbox/reducers/height_cache.js b/app/soapbox/reducers/height_cache.js index 2664d4f82..e69de29bb 100644 --- a/app/soapbox/reducers/height_cache.js +++ b/app/soapbox/reducers/height_cache.js @@ -1,24 +0,0 @@ -import { Map as ImmutableMap } from 'immutable'; - -import { HEIGHT_CACHE_SET, HEIGHT_CACHE_CLEAR } from '../actions/height_cache'; - -const initialState = ImmutableMap(); - -const setHeight = (state, key, id, height) => { - return state.update(key, ImmutableMap(), map => map.set(id, height)); -}; - -const clearHeights = () => { - return ImmutableMap(); -}; - -export default function statuses(state = initialState, action) { - switch(action.type) { - case HEIGHT_CACHE_SET: - return setHeight(state, action.key, action.id, action.height); - case HEIGHT_CACHE_CLEAR: - return clearHeights(); - default: - return state; - } -} diff --git a/app/soapbox/reducers/index.ts b/app/soapbox/reducers/index.ts index 4e54ec19f..85116e902 100644 --- a/app/soapbox/reducers/index.ts +++ b/app/soapbox/reducers/index.ts @@ -28,7 +28,6 @@ import group_editor from './group_editor'; import group_lists from './group_lists'; import group_relationships from './group_relationships'; import groups from './groups'; -import height_cache from './height_cache'; import identity_proofs from './identity_proofs'; import instance from './instance'; import listAdder from './list_adder'; @@ -83,7 +82,6 @@ const reducers = { compose, search, notifications, - height_cache, custom_emojis, identity_proofs, lists,