From ff55272d590d53771fde284d2381c5c9a8844171 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 2 Oct 2023 10:56:45 -0500 Subject: [PATCH 1/8] Timeline: put ScrollTopButton into a Portal --- src/features/ui/components/timeline.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/features/ui/components/timeline.tsx b/src/features/ui/components/timeline.tsx index a4d7d44e6..051d18122 100644 --- a/src/features/ui/components/timeline.tsx +++ b/src/features/ui/components/timeline.tsx @@ -6,6 +6,7 @@ import { defineMessages } from 'react-intl'; import { dequeueTimeline, scrollTopTimeline } from 'soapbox/actions/timelines'; import ScrollTopButton from 'soapbox/components/scroll-top-button'; import StatusList, { IStatusList } from 'soapbox/components/status-list'; +import { Portal } from 'soapbox/components/ui'; import { useAppSelector, useAppDispatch } from 'soapbox/hooks'; import { makeGetStatusIds } from 'soapbox/selectors'; @@ -51,12 +52,14 @@ const Timeline: React.FC = ({ return ( <> - + + + Date: Mon, 2 Oct 2023 11:14:36 -0500 Subject: [PATCH 2/8] Notifications: render ScrollTopButton in a Portal --- src/features/notifications/index.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/features/notifications/index.tsx b/src/features/notifications/index.tsx index 794073330..8e62ffd7f 100644 --- a/src/features/notifications/index.tsx +++ b/src/features/notifications/index.tsx @@ -14,7 +14,7 @@ import { getSettings } from 'soapbox/actions/settings'; import PullToRefresh from 'soapbox/components/pull-to-refresh'; import ScrollTopButton from 'soapbox/components/scroll-top-button'; import ScrollableList from 'soapbox/components/scrollable-list'; -import { Column } from 'soapbox/components/ui'; +import { Column, Portal } from 'soapbox/components/ui'; import PlaceholderNotification from 'soapbox/features/placeholder/components/placeholder-notification'; import { useAppDispatch, useAppSelector, useSettings } from 'soapbox/hooks'; @@ -176,11 +176,15 @@ const Notifications = () => { return ( {filterBarContainer} - + + + + + {scrollContainer} From 264082b434fe26a103609c0a22813ef82a06ab7a Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 2 Oct 2023 11:22:04 -0500 Subject: [PATCH 3/8] ScrollTopButton: a11y, to ); }; From 4e26adb789f26446ac8e25b900bd6fb5d3dcdd5e Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 2 Oct 2023 11:23:04 -0500 Subject: [PATCH 4/8] ScrollTopButton: swap out legacy Icon for UI Icon --- src/components/scroll-top-button.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/scroll-top-button.tsx b/src/components/scroll-top-button.tsx index dd42d10ff..4498e3657 100644 --- a/src/components/scroll-top-button.tsx +++ b/src/components/scroll-top-button.tsx @@ -3,8 +3,7 @@ import throttle from 'lodash/throttle'; import React, { useState, useEffect, useCallback } from 'react'; import { useIntl, MessageDescriptor } from 'react-intl'; -import Icon from 'soapbox/components/icon'; -import { Text } from 'soapbox/components/ui'; +import { Icon, Text } from 'soapbox/components/ui'; import { useSettings } from 'soapbox/hooks'; interface IScrollTopButton { @@ -87,7 +86,10 @@ const ScrollTopButton: React.FC = ({ className='flex cursor-pointer items-center space-x-1.5 whitespace-nowrap rounded-full bg-primary-600 px-4 py-2 text-white transition-transform hover:scale-105 hover:bg-primary-700 active:scale-100' onClick={handleClick} > - + {(count > 0) && ( From 2380a0b667e3f9cafd1fffef285e8b4d1488501f Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 2 Oct 2023 11:24:07 -0500 Subject: [PATCH 5/8] ScrollTopButton: render null when not visible --- src/components/scroll-top-button.tsx | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/components/scroll-top-button.tsx b/src/components/scroll-top-button.tsx index 4498e3657..0ce00ffc6 100644 --- a/src/components/scroll-top-button.tsx +++ b/src/components/scroll-top-button.tsx @@ -1,4 +1,3 @@ -import clsx from 'clsx'; import throttle from 'lodash/throttle'; import React, { useState, useEffect, useCallback } from 'react'; import { useIntl, MessageDescriptor } from 'react-intl'; @@ -35,10 +34,6 @@ const ScrollTopButton: React.FC = ({ const visible = count > 0 && scrolled; - const classes = clsx('fixed left-1/2 top-20 z-50 -translate-x-1/2', { - 'hidden': !visible, - }); - const getScrollTop = (): number => { return (document.scrollingElement || document.documentElement).scrollTop; }; @@ -80,8 +75,12 @@ const ScrollTopButton: React.FC = ({ maybeUnload(); }, [count]); + if (!visible) { + return null; + } + return ( -
+
); From 460a6e14c328becc54b49164ba934cc43be34ec3 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 2 Oct 2023 11:50:20 -0500 Subject: [PATCH 6/8] ScrollTopButton: make better use of callbacks, keep `scrolledTop` state --- src/components/scroll-top-button.tsx | 39 ++++++++++++++-------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/components/scroll-top-button.tsx b/src/components/scroll-top-button.tsx index 0ce00ffc6..4e2437b12 100644 --- a/src/components/scroll-top-button.tsx +++ b/src/components/scroll-top-button.tsx @@ -29,39 +29,40 @@ const ScrollTopButton: React.FC = ({ const intl = useIntl(); const settings = useSettings(); + // Whether we are scrolled past the `threshold`. const [scrolled, setScrolled] = useState(false); - const autoload = settings.get('autoloadTimelines') === true; + // Whether we are scrolled above the `autoloadThreshold`. + const [scrolledTop, setScrolledTop] = useState(true); + const autoload = settings.get('autoloadTimelines') === true; const visible = count > 0 && scrolled; + /** Number of pixels scrolled down from the top of the page. */ const getScrollTop = (): number => { return (document.scrollingElement || document.documentElement).scrollTop; }; - const maybeUnload = () => { - if (autoload && getScrollTop() <= autoloadThreshold) { + /** Unload feed items if scrolled to the top. */ + const maybeUnload = useCallback(() => { + if (autoload && scrolledTop) { onClick(); } - }; + }, [autoload, scrolledTop, onClick]); + /** Set state while scrolling. */ const handleScroll = useCallback(throttle(() => { - maybeUnload(); + const scrollTop = getScrollTop(); - if (getScrollTop() > threshold) { - setScrolled(true); - } else { - setScrolled(false); - } - }, 150, { trailing: true }), [autoload, threshold, autoloadThreshold, onClick]); + setScrolled(scrollTop > threshold); + setScrolledTop(scrollTop <= autoloadThreshold); - const scrollUp = () => { + }, 150, { trailing: true }), [threshold, autoloadThreshold]); + + /** Scroll to top and trigger `onClick`. */ + const handleClick: React.MouseEventHandler = useCallback(() => { window.scrollTo({ top: 0 }); - }; - - const handleClick: React.MouseEventHandler = () => { - setTimeout(scrollUp, 10); onClick(); - }; + }, [onClick]); useEffect(() => { window.addEventListener('scroll', handleScroll); @@ -69,11 +70,11 @@ const ScrollTopButton: React.FC = ({ return () => { window.removeEventListener('scroll', handleScroll); }; - }, [onClick]); + }, [handleScroll]); useEffect(() => { maybeUnload(); - }, [count]); + }, [maybeUnload]); if (!visible) { return null; From 74c231b59be1eac08f47009ef89306b0bee90c6f Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 2 Oct 2023 12:22:09 -0500 Subject: [PATCH 7/8] Stop re-rendering the ScrollTopButton so much --- src/features/notifications/index.tsx | 8 ++++---- src/features/ui/components/timeline.tsx | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/features/notifications/index.tsx b/src/features/notifications/index.tsx index 8e62ffd7f..0d9f7f608 100644 --- a/src/features/notifications/index.tsx +++ b/src/features/notifications/index.tsx @@ -104,13 +104,13 @@ const Notifications = () => { }); }; - const handleDequeueNotifications = () => { + const handleDequeueNotifications = useCallback(() => { dispatch(dequeueNotifications()); - }; + }, []); - const handleRefresh = () => { + const handleRefresh = useCallback(() => { return dispatch(expandNotifications()); - }; + }, []); useEffect(() => { handleDequeueNotifications(); diff --git a/src/features/ui/components/timeline.tsx b/src/features/ui/components/timeline.tsx index 051d18122..4db80bf90 100644 --- a/src/features/ui/components/timeline.tsx +++ b/src/features/ui/components/timeline.tsx @@ -38,9 +38,9 @@ const Timeline: React.FC = ({ const hasMore = useAppSelector(state => state.timelines.get(timelineId)?.hasMore === true); const totalQueuedItemsCount = useAppSelector(state => state.timelines.get(timelineId)?.totalQueuedItemsCount || 0); - const handleDequeueTimeline = () => { + const handleDequeueTimeline = useCallback(() => { dispatch(dequeueTimeline(timelineId, onLoadMore)); - }; + }, []); const handleScrollToTop = useCallback(debounce(() => { dispatch(scrollTopTimeline(timelineId, true)); From 7d0c699fe37622bb4ddc9e765d4987468dc7315b Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 2 Oct 2023 13:25:59 -0500 Subject: [PATCH 8/8] ScrollTopButton: delay adding the scroll listener so the feed doesn't jump when navigating back --- src/components/scroll-top-button.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/components/scroll-top-button.tsx b/src/components/scroll-top-button.tsx index 4e2437b12..84f23969e 100644 --- a/src/components/scroll-top-button.tsx +++ b/src/components/scroll-top-button.tsx @@ -32,7 +32,7 @@ const ScrollTopButton: React.FC = ({ // Whether we are scrolled past the `threshold`. const [scrolled, setScrolled] = useState(false); // Whether we are scrolled above the `autoloadThreshold`. - const [scrolledTop, setScrolledTop] = useState(true); + const [scrolledTop, setScrolledTop] = useState(false); const autoload = settings.get('autoloadTimelines') === true; const visible = count > 0 && scrolled; @@ -65,7 +65,12 @@ const ScrollTopButton: React.FC = ({ }, [onClick]); useEffect(() => { - window.addEventListener('scroll', handleScroll); + // Delay adding the scroll listener so navigating back doesn't + // unload feed items before the feed is rendered. + setTimeout(() => { + window.addEventListener('scroll', handleScroll); + handleScroll(); + }, 250); return () => { window.removeEventListener('scroll', handleScroll);