From 0a34bcfcba395a41c500543e19a8843fdb49a7a7 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 3 Jun 2022 12:24:55 -0500 Subject: [PATCH 1/7] StatusList: fix infinite scrolling (apparently I don't understand how useCallback works) --- app/soapbox/components/status_list.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/soapbox/components/status_list.tsx b/app/soapbox/components/status_list.tsx index a421f62a3..edfcd505a 100644 --- a/app/soapbox/components/status_list.tsx +++ b/app/soapbox/components/status_list.tsx @@ -68,11 +68,11 @@ const StatusList: React.FC = ({ }; const handleLoadOlder = useCallback(debounce(() => { - const loadMoreID = lastStatusId || statusIds.last(); - if (onLoadMore && loadMoreID) { - onLoadMore(loadMoreID); + const maxId = lastStatusId || statusIds.last(); + if (onLoadMore && maxId) { + onLoadMore(maxId); } - }, 300, { leading: true }), []); + }, 300, { leading: true }), [onLoadMore, lastStatusId, statusIds.last()]); const selectChild = (index: number) => { node.current?.scrollIntoView({ From edf366858a55165411ebe50e50b37cea9afcc297 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 3 Jun 2022 12:25:59 -0500 Subject: [PATCH 2/7] StatusListContainer: bust useCallback if timelineId changes --- app/soapbox/features/ui/containers/status_list_container.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/soapbox/features/ui/containers/status_list_container.tsx b/app/soapbox/features/ui/containers/status_list_container.tsx index 4dafbbf6d..196d58fc8 100644 --- a/app/soapbox/features/ui/containers/status_list_container.tsx +++ b/app/soapbox/features/ui/containers/status_list_container.tsx @@ -38,11 +38,11 @@ const StatusListContainer: React.FC = ({ const handleScrollToTop = useCallback(debounce(() => { dispatch(scrollTopTimeline(timelineId, true)); - }, 100), []); + }, 100), [timelineId]); const handleScroll = useCallback(debounce(() => { dispatch(scrollTopTimeline(timelineId, false)); - }, 100), []); + }, 100), [timelineId]); return ( <> From 2aa4b41528e0b3fda3382876dc838efb94f3fdae Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 3 Jun 2022 12:31:23 -0500 Subject: [PATCH 3/7] StatusListContainer --> Timeline --- app/soapbox/features/community_timeline/index.js | 4 ++-- app/soapbox/features/direct_timeline/index.tsx | 4 ++-- app/soapbox/features/groups/timeline/index.js | 4 ++-- app/soapbox/features/hashtag_timeline/index.js | 4 ++-- app/soapbox/features/home_timeline/index.js | 4 ++-- app/soapbox/features/list_timeline/index.tsx | 4 ++-- app/soapbox/features/public_timeline/index.js | 4 ++-- app/soapbox/features/remote_timeline/index.tsx | 4 ++-- app/soapbox/features/test_timeline/index.tsx | 4 ++-- .../status_list_container.tsx => components/timeline.tsx} | 6 +++--- 10 files changed, 21 insertions(+), 21 deletions(-) rename app/soapbox/features/ui/{containers/status_list_container.tsx => components/timeline.tsx} (92%) diff --git a/app/soapbox/features/community_timeline/index.js b/app/soapbox/features/community_timeline/index.js index 1ccc6076f..cb64b49b3 100644 --- a/app/soapbox/features/community_timeline/index.js +++ b/app/soapbox/features/community_timeline/index.js @@ -9,7 +9,7 @@ import { expandCommunityTimeline } from 'soapbox/actions/timelines'; import SubNavigation from 'soapbox/components/sub_navigation'; import { Column } from 'soapbox/components/ui'; -import StatusListContainer from '../ui/containers/status_list_container'; +import Timeline from '../ui/components/timeline'; import ColumnSettings from './containers/column_settings_container'; @@ -81,7 +81,7 @@ class CommunityTimeline extends React.PureComponent { return ( - { onSelected={handleSuggestion} /> - { const me = state.get('me'); @@ -90,7 +90,7 @@ class GroupTimeline extends React.PureComponent { )}
- ({ hasUnread: state.getIn(['timelines', `hashtag:${props.params.id}`, 'unread']) > 0, @@ -114,7 +114,7 @@ class HashtagTimeline extends React.PureComponent { return ( - } ) : ( - {
*/} - } - = ({ params }) => { /> } - { return ( - } diff --git a/app/soapbox/features/ui/containers/status_list_container.tsx b/app/soapbox/features/ui/components/timeline.tsx similarity index 92% rename from app/soapbox/features/ui/containers/status_list_container.tsx rename to app/soapbox/features/ui/components/timeline.tsx index 196d58fc8..3265509f5 100644 --- a/app/soapbox/features/ui/containers/status_list_container.tsx +++ b/app/soapbox/features/ui/components/timeline.tsx @@ -14,11 +14,11 @@ const messages = defineMessages({ queue: { id: 'status_list.queue_label', defaultMessage: 'Click to see {count} new {count, plural, one {post} other {posts}}' }, }); -interface IStatusListContainer extends Omit { +interface ITimeline extends Omit { timelineId: string, } -const StatusListContainer: React.FC = ({ +const Timeline: React.FC = ({ timelineId, onLoadMore, ...rest @@ -68,4 +68,4 @@ const StatusListContainer: React.FC = ({ ); }; -export default StatusListContainer; +export default Timeline; From 6da72d874e5267ae532ef75f2d7060b94adda1cd Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 3 Jun 2022 12:34:30 -0500 Subject: [PATCH 4/7] TimelineQueueButtonHeader --> ScrollTopButton --- ..._header.test.js => scroll-top-button.test.js} | 16 ++++++++-------- ...e_button_header.tsx => scroll-top-button.tsx} | 6 +++--- app/soapbox/features/notifications/index.js | 4 ++-- app/soapbox/features/ui/components/timeline.tsx | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) rename app/soapbox/components/__tests__/{timeline_queue_button_header.test.js => scroll-top-button.test.js} (79%) rename app/soapbox/components/{timeline_queue_button_header.tsx => scroll-top-button.tsx} (93%) diff --git a/app/soapbox/components/__tests__/timeline_queue_button_header.test.js b/app/soapbox/components/__tests__/scroll-top-button.test.js similarity index 79% rename from app/soapbox/components/__tests__/timeline_queue_button_header.test.js rename to app/soapbox/components/__tests__/scroll-top-button.test.js index bc011d309..c23a733ef 100644 --- a/app/soapbox/components/__tests__/timeline_queue_button_header.test.js +++ b/app/soapbox/components/__tests__/scroll-top-button.test.js @@ -3,17 +3,17 @@ import React from 'react'; import { defineMessages } from 'react-intl'; import { render, screen } from '../../jest/test-helpers'; -import TimelineQueueButtonHeader from '../timeline_queue_button_header'; +import ScrollTopButton from '../scroll-top-button'; const messages = defineMessages({ queue: { id: 'status_list.queue_label', defaultMessage: 'Click to see {count} new {count, plural, one {post} other {posts}}' }, }); -describe('', () => { +describe('', () => { it('renders correctly', async() => { render( - {}} // eslint-disable-line react/jsx-no-bind timelineId='home' message={messages.queue} @@ -24,8 +24,8 @@ describe('', () => { expect(screen.queryAllByRole('link')).toHaveLength(0); render( - {}} // eslint-disable-line react/jsx-no-bind timelineId='home' message={messages.queue} @@ -36,8 +36,8 @@ describe('', () => { expect(screen.getByText(/Click to see\s+1\s+new post/, { hidden: true })).toBeInTheDocument(); render( - {}} // eslint-disable-line react/jsx-no-bind timelineId='home' message={messages.queue} diff --git a/app/soapbox/components/timeline_queue_button_header.tsx b/app/soapbox/components/scroll-top-button.tsx similarity index 93% rename from app/soapbox/components/timeline_queue_button_header.tsx rename to app/soapbox/components/scroll-top-button.tsx index 292368571..11ceb9e40 100644 --- a/app/soapbox/components/timeline_queue_button_header.tsx +++ b/app/soapbox/components/scroll-top-button.tsx @@ -8,7 +8,7 @@ import { Text } from 'soapbox/components/ui'; import { useAppSelector, useSettings } from 'soapbox/hooks'; import { shortNumberFormat } from 'soapbox/utils/numbers'; -interface ITimelineQueueButtonHeader { +interface IScrollTopButton { onClick: () => void, timelineId: string, message: MessageDescriptor, @@ -16,7 +16,7 @@ interface ITimelineQueueButtonHeader { autoloadThreshold?: number, } -const TimelineQueueButtonHeader: React.FC = ({ +const ScrollTopButton: React.FC = ({ onClick, timelineId, message, @@ -82,4 +82,4 @@ const TimelineQueueButtonHeader: React.FC = ({ ); }; -export default TimelineQueueButtonHeader; +export default ScrollTopButton; diff --git a/app/soapbox/features/notifications/index.js b/app/soapbox/features/notifications/index.js index 8ef05fb73..d3b5cb318 100644 --- a/app/soapbox/features/notifications/index.js +++ b/app/soapbox/features/notifications/index.js @@ -14,8 +14,8 @@ import { dequeueNotifications, } from 'soapbox/actions/notifications'; import { getSettings } from 'soapbox/actions/settings'; +import ScrollTopButton from 'soapbox/components/scroll-top-button'; import ScrollableList from 'soapbox/components/scrollable_list'; -import TimelineQueueButtonHeader from 'soapbox/components/timeline_queue_button_header'; import { Column } from 'soapbox/components/ui'; import PlaceholderNotification from 'soapbox/features/placeholder/components/placeholder_notification'; @@ -195,7 +195,7 @@ class Notifications extends React.PureComponent { return ( {filterBarContainer} - = ({ return ( <> - Date: Fri, 3 Jun 2022 12:38:54 -0500 Subject: [PATCH 5/7] ScrollTopButton: restore `count` prop so it works in Notifications again --- app/soapbox/components/scroll-top-button.tsx | 7 +++---- app/soapbox/features/ui/components/timeline.tsx | 3 ++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/soapbox/components/scroll-top-button.tsx b/app/soapbox/components/scroll-top-button.tsx index 11ceb9e40..8fed6cc78 100644 --- a/app/soapbox/components/scroll-top-button.tsx +++ b/app/soapbox/components/scroll-top-button.tsx @@ -5,12 +5,12 @@ import { useIntl, MessageDescriptor } from 'react-intl'; import Icon from 'soapbox/components/icon'; import { Text } from 'soapbox/components/ui'; -import { useAppSelector, useSettings } from 'soapbox/hooks'; +import { useSettings } from 'soapbox/hooks'; import { shortNumberFormat } from 'soapbox/utils/numbers'; interface IScrollTopButton { onClick: () => void, - timelineId: string, + count: number, message: MessageDescriptor, threshold?: number, autoloadThreshold?: number, @@ -18,14 +18,13 @@ interface IScrollTopButton { const ScrollTopButton: React.FC = ({ onClick, - timelineId, + count, message, threshold = 400, autoloadThreshold = 50, }) => { const intl = useIntl(); const settings = useSettings(); - const count = useAppSelector(state => state.timelines.getIn([timelineId, 'totalQueuedItemsCount'])); const [scrolled, setScrolled] = useState(false); const autoload = settings.get('autoloadTimelines') === true; diff --git a/app/soapbox/features/ui/components/timeline.tsx b/app/soapbox/features/ui/components/timeline.tsx index 2c143d8fd..74efaac2e 100644 --- a/app/soapbox/features/ui/components/timeline.tsx +++ b/app/soapbox/features/ui/components/timeline.tsx @@ -31,6 +31,7 @@ const Timeline: React.FC = ({ const isLoading = useAppSelector(state => state.timelines.getIn([timelineId, 'isLoading'], true) === true); const isPartial = useAppSelector(state => state.timelines.getIn([timelineId, 'isPartial'], false) === true); const hasMore = useAppSelector(state => state.timelines.getIn([timelineId, 'hasMore']) === true); + const totalQueuedItemsCount = useAppSelector(state => state.timelines.getIn([timelineId, 'totalQueuedItemsCount'])); const handleDequeueTimeline = () => { dispatch(dequeueTimeline(timelineId, onLoadMore)); @@ -49,7 +50,7 @@ const Timeline: React.FC = ({ From fbb460817f97e7fbf073598236eb5f1ae83d82db Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Fri, 3 Jun 2022 13:09:46 -0500 Subject: [PATCH 6/7] Add JSDoc comments to timeline components --- app/soapbox/components/scroll-top-button.tsx | 8 ++++- app/soapbox/components/scrollable_list.tsx | 34 +++++++++++++++---- app/soapbox/components/status_list.tsx | 17 +++++++--- .../features/ui/components/timeline.tsx | 3 ++ 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/app/soapbox/components/scroll-top-button.tsx b/app/soapbox/components/scroll-top-button.tsx index 8fed6cc78..698c92779 100644 --- a/app/soapbox/components/scroll-top-button.tsx +++ b/app/soapbox/components/scroll-top-button.tsx @@ -9,13 +9,19 @@ import { useSettings } from 'soapbox/hooks'; import { shortNumberFormat } from 'soapbox/utils/numbers'; interface IScrollTopButton { + /** Callback when clicked, and also when scrolled to the top. */ onClick: () => void, + /** Number of unread items. */ count: number, + /** Message to display in the button (should contain a `{count}` value). */ message: MessageDescriptor, + /** Distance from the top of the screen (scrolling down) before the button appears. */ threshold?: number, + /** Distance from the top of the screen (scrolling up) before the action is triggered. */ autoloadThreshold?: number, } +/** Floating new post counter above timelines, clicked to scroll to top. */ const ScrollTopButton: React.FC = ({ onClick, count, @@ -41,7 +47,7 @@ const ScrollTopButton: React.FC = ({ } else { setScrolled(false); } - }, 150, { trailing: true }), []); + }, 150, { trailing: true }), [autoload, threshold, autoloadThreshold]); const scrollUp = () => { window.scrollTo({ top: 0, behavior: 'smooth' }); diff --git a/app/soapbox/components/scrollable_list.tsx b/app/soapbox/components/scrollable_list.tsx index e546f2a45..8b803be66 100644 --- a/app/soapbox/components/scrollable_list.tsx +++ b/app/soapbox/components/scrollable_list.tsx @@ -9,6 +9,7 @@ import { useSettings } from 'soapbox/hooks'; import LoadMore from './load_more'; import { Spinner, Text } from './ui'; +/** Custom Viruoso component context. */ type Context = { itemClassName?: string, listClassName?: string, @@ -20,6 +21,7 @@ type SavedScrollPosition = { offset: number, } +/** Custom Virtuoso Item component representing a single scrollable item. */ // 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 @@ -27,6 +29,7 @@ const Item: Components['Item'] = ({ context, ...rest }) => (
); +/** Custom Virtuoso List component for the outer container. */ // Ensure the className winds up here const List: Components['List'] = React.forwardRef((props, ref) => { const { context, ...rest } = props; @@ -34,28 +37,47 @@ const List: Components['List'] = React.forwardRef((props, ref) => { }); interface IScrollableList extends VirtuosoProps { + /** Unique key to preserve the scroll position when navigating back. */ scrollKey?: string, + /** Pagination callback when the end of the list is reached. */ onLoadMore?: () => void, + /** Whether the data is currently being fetched. */ isLoading?: boolean, + /** Whether to actually display the loading state. */ showLoading?: boolean, + /** Whether we expect an additional page of data. */ hasMore?: boolean, + /** Additional element to display at the top of the list. */ prepend?: React.ReactNode, + /** Whether to display the prepended element. */ alwaysPrepend?: boolean, + /** Message to display when the list is loaded but empty. */ emptyMessage?: React.ReactNode, + /** Scrollable content. */ children: Iterable, + /** Callback when the list is scrolled to the top. */ onScrollToTop?: () => void, + /** Callback when the list is scrolled. */ onScroll?: () => void, + /** Placeholder component to render while loading. */ placeholderComponent?: React.ComponentType | React.NamedExoticComponent, + /** Number of placeholders to render while loading. */ placeholderCount?: number, + /** Pull to refresh callback. */ onRefresh?: () => Promise, + /** Extra class names on the Virtuoso element. */ className?: string, + /** Class names on each item container. */ itemClassName?: string, + /** `id` attribute on the Virtuoso element. */ id?: string, + /** CSS styles on the Virtuoso element. */ style?: React.CSSProperties, + /** Whether to use the window to scroll the content instead of Virtuoso's container. */ useWindowScroll?: boolean } -/** Legacy ScrollableList with Virtuoso for backwards-compatibility */ +/** Legacy ScrollableList with Virtuoso for backwards-compatibility. */ const ScrollableList = React.forwardRef(({ scrollKey, prepend = null, @@ -88,7 +110,7 @@ const ScrollableList = React.forwardRef(({ const topIndex = useRef(scrollData ? scrollData.index : 0); const topOffset = useRef(scrollData ? scrollData.offset : 0); - /** Normalized children */ + /** Normalized children. */ const elements = Array.from(children || []); const showPlaceholder = showLoading && Placeholder && placeholderCount > 0; @@ -129,7 +151,7 @@ const ScrollableList = React.forwardRef(({ }; }, []); - /* Render an empty state instead of the scrollable list */ + /* Render an empty state instead of the scrollable list. */ const renderEmpty = (): JSX.Element => { return (
@@ -146,7 +168,7 @@ const ScrollableList = React.forwardRef(({ ); }; - /** Render a single item */ + /** Render a single item. */ const renderItem = (_i: number, element: JSX.Element): JSX.Element => { if (showPlaceholder) { return ; @@ -192,7 +214,7 @@ const ScrollableList = React.forwardRef(({ return 0; }, [showLoading, initialTopMostItemIndex]); - /** Render the actual Virtuoso list */ + /** Render the actual Virtuoso list. */ const renderFeed = (): JSX.Element => ( (({ /> ); - /** Conditionally render inner elements */ + /** Conditionally render inner elements. */ const renderBody = (): JSX.Element => { if (isEmpty) { return renderEmpty(); diff --git a/app/soapbox/components/status_list.tsx b/app/soapbox/components/status_list.tsx index edfcd505a..37e37f59d 100644 --- a/app/soapbox/components/status_list.tsx +++ b/app/soapbox/components/status_list.tsx @@ -14,24 +14,31 @@ import type { VirtuosoHandle } from 'react-virtuoso'; import type { IScrollableList } from 'soapbox/components/scrollable_list'; interface IStatusList extends Omit { + /** Unique key to preserve the scroll position when navigating back. */ scrollKey: string, + /** List of status IDs to display. */ statusIds: ImmutableOrderedSet, + /** Last _unfiltered_ status ID (maxId) for pagination. */ lastStatusId?: string, + /** Pinned statuses to show at the top of the feed. */ featuredStatusIds?: ImmutableOrderedSet, + /** Pagination callback when the end of the list is reached. */ onLoadMore?: (lastStatusId: string) => void, + /** Whether the data is currently being fetched. */ isLoading: boolean, + /** Whether the server did not return a complete page. */ isPartial?: boolean, + /** Whether we expect an additional page of data. */ hasMore: boolean, - prepend?: React.ReactNode, + /** Message to display when the list is loaded but empty. */ emptyMessage: React.ReactNode, - alwaysPrepend?: boolean, + /** ID of the timeline in Redux. */ timelineId?: string, - queuedItemSize?: number, - onScrollToTop?: () => void, - onScroll?: () => void, + /** Whether to display a gap or border between statuses in the list. */ divideType: 'space' | 'border', } +/** Feed of statuses, built atop ScrollableList. */ const StatusList: React.FC = ({ statusIds, lastStatusId, diff --git a/app/soapbox/features/ui/components/timeline.tsx b/app/soapbox/features/ui/components/timeline.tsx index 74efaac2e..af3ff8e8c 100644 --- a/app/soapbox/features/ui/components/timeline.tsx +++ b/app/soapbox/features/ui/components/timeline.tsx @@ -15,9 +15,11 @@ const messages = defineMessages({ }); interface ITimeline extends Omit { + /** ID of the timeline in Redux. */ timelineId: string, } +/** Scrollable list of statuses from a timeline in the Redux store. */ const Timeline: React.FC = ({ timelineId, onLoadMore, @@ -55,6 +57,7 @@ const Timeline: React.FC = ({ /> Date: Fri, 3 Jun 2022 13:16:22 -0500 Subject: [PATCH 7/7] ScrollTopButton: fix tests, actually don't use shortNumberFormat for now (it messes up pluralization) --- .../__tests__/scroll-top-button.test.js | 23 +++++++------------ app/soapbox/components/scroll-top-button.tsx | 3 +-- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/app/soapbox/components/__tests__/scroll-top-button.test.js b/app/soapbox/components/__tests__/scroll-top-button.test.js index c23a733ef..89518b977 100644 --- a/app/soapbox/components/__tests__/scroll-top-button.test.js +++ b/app/soapbox/components/__tests__/scroll-top-button.test.js @@ -1,4 +1,3 @@ -import { fromJS } from 'immutable'; import React from 'react'; import { defineMessages } from 'react-intl'; @@ -14,37 +13,31 @@ describe('', () => { render( {}} // eslint-disable-line react/jsx-no-bind - timelineId='home' + onClick={() => {}} + count={0} message={messages.queue} />, - undefined, - { timelines: fromJS({ home: { totalQueuedItemsCount: 0 } }) }, ); expect(screen.queryAllByRole('link')).toHaveLength(0); render( {}} // eslint-disable-line react/jsx-no-bind - timelineId='home' + onClick={() => {}} + count={1} message={messages.queue} />, - undefined, - { timelines: fromJS({ home: { totalQueuedItemsCount: 1 } }) }, ); - expect(screen.getByText(/Click to see\s+1\s+new post/, { hidden: true })).toBeInTheDocument(); + expect(screen.getByText('Click to see 1 new post', { hidden: true })).toBeInTheDocument(); render( {}} // eslint-disable-line react/jsx-no-bind - timelineId='home' + onClick={() => {}} + count={9999999} message={messages.queue} />, - undefined, - { timelines: fromJS({ home: { totalQueuedItemsCount: 9999999 } }) }, ); - expect(screen.getByText(/10.*M/, { hidden: true })).toBeInTheDocument(); + expect(screen.getByText('Click to see 9999999 new posts', { hidden: true })).toBeInTheDocument(); }); }); diff --git a/app/soapbox/components/scroll-top-button.tsx b/app/soapbox/components/scroll-top-button.tsx index 698c92779..eb5d00593 100644 --- a/app/soapbox/components/scroll-top-button.tsx +++ b/app/soapbox/components/scroll-top-button.tsx @@ -6,7 +6,6 @@ import { useIntl, MessageDescriptor } from 'react-intl'; import Icon from 'soapbox/components/icon'; import { Text } from 'soapbox/components/ui'; import { useSettings } from 'soapbox/hooks'; -import { shortNumberFormat } from 'soapbox/utils/numbers'; interface IScrollTopButton { /** Callback when clicked, and also when scrolled to the top. */ @@ -79,7 +78,7 @@ const ScrollTopButton: React.FC = ({ {(count > 0) && ( - {intl.formatMessage(message, { count: shortNumberFormat(count) })} + {intl.formatMessage(message, { count })} )}