From c71b90cecfdca06ccf95e65546ff81ae044001ca Mon Sep 17 00:00:00 2001 From: infinite-persistence Date: Fri, 1 Oct 2021 15:49:37 +0800 Subject: [PATCH] Fix linked-comment scrolling I think this the best solution so far, at the expense of a slight delay in scrolling if the network call stalls. - Added "fetching by ID" state so that we don't need to use the ugly N-retries method. - `scrollIntoView` doesn't work if the element is already in the viewport, and the `scrollBy` adjustment doesn't take into account the y-position restoration that we perform on certain type of pages. Use `window.scrollTo` instead and taking into account current scroll position. --- flow-typed/Comment.js | 1 + ui/component/commentsList/index.js | 2 ++ ui/component/commentsList/view.jsx | 28 ++++++++++++++-------------- ui/constants/action_types.js | 1 + ui/redux/actions/comments.js | 4 ++++ ui/redux/reducers/comments.js | 4 ++++ ui/redux/selectors/comments.js | 1 + 7 files changed, 27 insertions(+), 14 deletions(-) diff --git a/flow-typed/Comment.js b/flow-typed/Comment.js index f62993f9e..8b2ad570c 100644 --- a/flow-typed/Comment.js +++ b/flow-typed/Comment.js @@ -43,6 +43,7 @@ declare type CommentsState = { linkedCommentAncestors: { [string]: Array }, // {"linkedCommentId": ["parentId", "grandParentId", ...]} pinnedCommentsById: {}, // ClaimId -> array of pinned comment IDs isLoading: boolean, + isLoadingById: boolean, isLoadingByParentId: { [string]: boolean }, myComments: ?Set, isFetchingReacts: boolean, diff --git a/ui/component/commentsList/index.js b/ui/component/commentsList/index.js index e56b6ca6d..8d15ab707 100644 --- a/ui/component/commentsList/index.js +++ b/ui/component/commentsList/index.js @@ -9,6 +9,7 @@ import { makeSelectTopLevelCommentsForUri, makeSelectTopLevelTotalPagesForUri, selectIsFetchingComments, + selectIsFetchingCommentsById, selectIsFetchingReacts, makeSelectTotalCommentsCountForUri, selectOthersReactsById, @@ -33,6 +34,7 @@ const select = (state, props) => { claim: makeSelectClaimForUri(props.uri)(state), claimIsMine: makeSelectClaimIsMine(props.uri)(state), isFetchingComments: selectIsFetchingComments(state), + isFetchingCommentsById: selectIsFetchingCommentsById(state), isFetchingReacts: selectIsFetchingReacts(state), fetchingChannels: selectFetchingMyChannels(state), settingsByChannelId: selectSettingsByChannelId(state), diff --git a/ui/component/commentsList/view.jsx b/ui/component/commentsList/view.jsx index 8235ebd75..6a51fd42c 100644 --- a/ui/component/commentsList/view.jsx +++ b/ui/component/commentsList/view.jsx @@ -14,14 +14,12 @@ import usePersistedState from 'effects/use-persisted-state'; import { ENABLE_COMMENT_REACTIONS } from 'config'; import Empty from 'component/common/empty'; import debounce from 'util/debounce'; +import useFetched from 'effects/use-fetched'; import { useIsMobile, useIsMediumScreen } from 'effects/use-screensize'; import { getChannelIdFromClaim } from 'util/claim'; const DEBOUNCE_SCROLL_HANDLER_MS = 200; -// "3" due to 2 separate fetches needed + 1 buffer just in case. -const MAX_LINKED_COMMENT_SCROLL_ATTEMPTS = 3; - function scaleToDevicePixelRatio(value) { const devicePixelRatio = window.devicePixelRatio || 1.0; if (devicePixelRatio < 1.0) { @@ -44,6 +42,7 @@ type Props = { claimIsMine: boolean, myChannels: ?Array, isFetchingComments: boolean, + isFetchingCommentsById: boolean, isFetchingReacts: boolean, linkedCommentId?: string, totalComments: number, @@ -69,6 +68,7 @@ function CommentList(props: Props) { claimIsMine, myChannels, isFetchingComments, + isFetchingCommentsById, isFetchingReacts, linkedCommentId, totalComments, @@ -83,15 +83,15 @@ function CommentList(props: Props) { const DEFAULT_SORT = ENABLE_COMMENT_REACTIONS ? SORT_BY.POPULARITY : SORT_BY.NEWEST; const [sort, setSort] = usePersistedState('comment-sort-by', DEFAULT_SORT); const [page, setPage] = React.useState(0); - const [lcScrollAttempts, setLcScrollAttempts] = React.useState( - linkedCommentId ? 0 : MAX_LINKED_COMMENT_SCROLL_ATTEMPTS - ); const isMobile = useIsMobile(); const isMediumScreen = useIsMediumScreen(); const [expandedComments, setExpandedComments] = React.useState(!isMobile && !isMediumScreen); const totalFetchedComments = allCommentIds ? allCommentIds.length : 0; const channelId = getChannelIdFromClaim(claim); const channelSettings = channelId ? settingsByChannelId[channelId] : undefined; + const fetchedCommentsOnce = useFetched(isFetchingComments); + const fetchedReactsOnce = useFetched(isFetchingReacts); + const fetchedLinkedComment = useFetched(isFetchingCommentsById); // Display comments immediately if not fetching reactions // If not, wait to show comments until reactions are fetched @@ -203,19 +203,19 @@ function CommentList(props: Props) { // Scroll to linked-comment useEffect(() => { - if (lcScrollAttempts < MAX_LINKED_COMMENT_SCROLL_ATTEMPTS && readyToDisplayComments && !isFetchingComments) { + if (fetchedLinkedComment && fetchedCommentsOnce && fetchedReactsOnce) { const elems = document.getElementsByClassName(COMMENT_HIGHLIGHTED); if (elems.length > 0) { - setLcScrollAttempts(MAX_LINKED_COMMENT_SCROLL_ATTEMPTS); + const ROUGH_HEADER_HEIGHT = 125; // @see: --header-height const linkedComment = elems[0]; - linkedComment.scrollIntoView({ block: 'start' }); - window.scrollBy(0, -125); - } else { - setLcScrollAttempts(lcScrollAttempts + 1); + window.scrollTo({ + top: linkedComment.getBoundingClientRect().top + window.scrollY - ROUGH_HEADER_HEIGHT, + left: 0, + behavior: 'smooth', + }); } } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [readyToDisplayComments, isFetchingComments]); // We just want to respond to these, nothing else. + }, [fetchedLinkedComment, fetchedCommentsOnce, fetchedReactsOnce]); // Infinite scroll useEffect(() => { diff --git a/ui/constants/action_types.js b/ui/constants/action_types.js index 17748007e..7345e773f 100644 --- a/ui/constants/action_types.js +++ b/ui/constants/action_types.js @@ -253,6 +253,7 @@ export const COMMENT_LIST_STARTED = 'COMMENT_LIST_STARTED'; export const COMMENT_LIST_COMPLETED = 'COMMENT_LIST_COMPLETED'; export const COMMENT_LIST_FAILED = 'COMMENT_LIST_FAILED'; export const COMMENT_LIST_RESET = 'COMMENT_LIST_RESET'; +export const COMMENT_BY_ID_STARTED = 'COMMENT_BY_ID_STARTED'; export const COMMENT_BY_ID_COMPLETED = 'COMMENT_BY_ID_COMPLETED'; export const COMMENT_CREATE_STARTED = 'COMMENT_CREATE_STARTED'; export const COMMENT_CREATE_COMPLETED = 'COMMENT_CREATE_COMPLETED'; diff --git a/ui/redux/actions/comments.js b/ui/redux/actions/comments.js index b47622f38..44947d109 100644 --- a/ui/redux/actions/comments.js +++ b/ui/redux/actions/comments.js @@ -203,6 +203,10 @@ export function doCommentList( export function doCommentById(commentId: string, toastIfNotFound: boolean = true) { return (dispatch: Dispatch, getState: GetState) => { + dispatch({ + type: ACTIONS.COMMENT_BY_ID_STARTED, + }); + return Comments.comment_by_id({ comment_id: commentId, with_ancestors: true }) .then((result: CommentByIdResponse) => { const { item, items, ancestors } = result; diff --git a/ui/redux/reducers/comments.js b/ui/redux/reducers/comments.js index e967aa4ae..d4fd5b926 100644 --- a/ui/redux/reducers/comments.js +++ b/ui/redux/reducers/comments.js @@ -21,6 +21,7 @@ const defaultState: CommentsState = { superChatsByUri: {}, pinnedCommentsById: {}, // ClaimId -> array of pinned comment IDs isLoading: false, + isLoadingById: false, isLoadingByParentId: {}, isCommenting: false, myComments: undefined, @@ -335,6 +336,8 @@ export default handleActions( }; }, + [ACTIONS.COMMENT_BY_ID_STARTED]: (state) => ({ ...state, isLoadingById: true }), + [ACTIONS.COMMENT_BY_ID_COMPLETED]: (state: CommentsState, action: any) => { const { comment, ancestors } = action.data; const claimId = comment.claim_id; @@ -375,6 +378,7 @@ export default handleActions( return { ...state, + isLoadingById: false, topLevelCommentsById, topLevelTotalCommentsById, topLevelTotalPagesById, diff --git a/ui/redux/selectors/comments.js b/ui/redux/selectors/comments.js index 7094c847c..e19e236f4 100644 --- a/ui/redux/selectors/comments.js +++ b/ui/redux/selectors/comments.js @@ -9,6 +9,7 @@ const selectState = (state) => state.comments || {}; export const selectCommentsById = createSelector(selectState, (state) => state.commentById || {}); export const selectIsFetchingComments = createSelector(selectState, (state) => state.isLoading); +export const selectIsFetchingCommentsById = createSelector(selectState, (state) => state.isLoadingById); export const selectIsFetchingCommentsByParentId = createSelector(selectState, (state) => state.isLoadingByParentId); export const selectIsPostingComment = createSelector(selectState, (state) => state.isCommenting); export const selectIsFetchingReacts = createSelector(selectState, (state) => state.isFetchingReacts);