From e35a0942d78aae3dbd4be2f6497caa48447aa60f Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Fri, 11 Jul 2025 18:19:30 -0700 Subject: [PATCH 1/5] ref(feedback): Replace `react-virtualized` with `@tanstack/react-virtual` in Feedback --- .../components/feedback/list/feedbackList.tsx | 173 +++++------------- .../feedback/list/feedbackListItem.tsx | 9 +- .../infiniteList/infiniteListItems.tsx | 152 +++++++++++++++ .../infiniteList/infiniteListState.tsx | 59 ++++++ .../utils/api/useFetchInfiniteListData.tsx | 86 --------- static/app/utils/array/uniqBy.ts | 13 ++ 6 files changed, 276 insertions(+), 216 deletions(-) create mode 100644 static/app/components/infiniteList/infiniteListItems.tsx create mode 100644 static/app/components/infiniteList/infiniteListState.tsx delete mode 100644 static/app/utils/api/useFetchInfiniteListData.tsx create mode 100644 static/app/utils/array/uniqBy.ts diff --git a/static/app/components/feedback/list/feedbackList.tsx b/static/app/components/feedback/list/feedbackList.tsx index 035f355d0b9643..9529b6129c7560 100644 --- a/static/app/components/feedback/list/feedbackList.tsx +++ b/static/app/components/feedback/list/feedbackList.tsx @@ -1,11 +1,4 @@ -import {Fragment, useMemo, useRef} from 'react'; -import type {ListRowProps} from 'react-virtualized'; -import { - AutoSizer, - CellMeasurer, - InfiniteLoader, - List as ReactVirtualizedList, -} from 'react-virtualized'; +import {Fragment, useMemo} from 'react'; import styled from '@emotion/styled'; import waitingForEventImg from 'sentry-images/spot/waiting-for-event.svg'; @@ -15,157 +8,94 @@ import ErrorBoundary from 'sentry/components/errorBoundary'; import FeedbackListHeader from 'sentry/components/feedback/list/feedbackListHeader'; import FeedbackListItem from 'sentry/components/feedback/list/feedbackListItem'; import useFeedbackQueryKeys from 'sentry/components/feedback/useFeedbackQueryKeys'; +import InfiniteListItems from 'sentry/components/infiniteList/infiniteListItems'; +import InfiniteListState from 'sentry/components/infiniteList/infiniteListState'; import LoadingIndicator from 'sentry/components/loadingIndicator'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; -import useFetchInfiniteListData from 'sentry/utils/api/useFetchInfiniteListData'; +import uniqueBy from 'sentry/utils/array/uniqBy'; import type {FeedbackIssueListItem} from 'sentry/utils/feedback/types'; import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState'; -import useVirtualizedList from 'sentry/views/replays/detail/useVirtualizedList'; +import {useInfiniteApiQuery} from 'sentry/utils/queryClient'; -// Ensure this object is created once as it is an input to -// `useVirtualizedList`'s memoization -const cellMeasurer = { - fixedWidth: true, - minHeight: 24, -}; - -function NoFeedback({title, subtitle}: {subtitle: string; title: string}) { +function NoFeedback() { return ( - - No feedback found spot illustration - {title} -

{subtitle}

- + + {t('A + {t('Inbox Zero')} +

{t('You have two options: take a nap or be productive.')}

+ ); } export default function FeedbackList() { const {listQueryKey} = useFeedbackQueryKeys(); - const { - isFetchingNextPage, - isFetchingPreviousPage, - isLoading, // If anything is loaded yet - getRow, - isRowLoaded, - issues, - loadMoreRows, - hits, - } = useFetchInfiniteListData({ + const queryResult = useInfiniteApiQuery({ queryKey: listQueryKey ?? ['infinite', ''], - uniqueField: 'id', enabled: Boolean(listQueryKey), }); + const issues = useMemo( + () => uniqueBy(queryResult.data?.pages.flatMap(([pageData]) => pageData) ?? [], 'id'), + [queryResult.data?.pages] + ); const checkboxState = useListItemCheckboxContext({ - hits, + hits: issues.length, knownIds: issues.map(issue => issue.id), queryKey: listQueryKey, }); - const listRef = useRef(null); - - const deps = useMemo(() => [isLoading, issues.length], [isLoading, issues.length]); - const {cache, updateList} = useVirtualizedList({ - cellMeasurer, - ref: listRef, - deps, - }); - - const renderRow = ({index, key, style, parent}: ListRowProps) => { - const item = getRow({index}); - if (!item) { - return null; - } - - return ( - - - { - checkboxState.toggleSelected(item.id); - }} - style={style} - /> - - - ); - }; - return ( - null} + loadingMessage={() => } > - {({onRowsRendered, registerChild}) => ( - - {({width, height}) => ( - - isLoading ? ( - - ) : ( - - ) - } - onRowsRendered={onRowsRendered} - overscanRowCount={5} - ref={e => { - listRef.current = e; - registerChild(e); + + estimateSize={() => 24} + queryResult={queryResult} + itemRenderer={({item}) => ( + + { + checkboxState.toggleSelected(item.id); }} - rowCount={issues.length} - rowHeight={cache.rowHeight} - rowRenderer={renderRow} - width={width} /> - )} - - )} - - - {isFetchingPreviousPage ? ( - - - - ) : null} - - - {isFetchingNextPage ? ( - - - - ) : null} - + + )} + emptyMessage={() => } + loadingMoreMessage={() => ( + + + + + + )} + loadingCompleteMessage={() => null} + /> + ); } const FeedbackListItems = styled('div')` - display: grid; + display: flex; + flex-direction: column; flex-grow: 1; - min-height: 300px; + padding-bottom: ${space(0.5)}; `; -const FloatingContainer = styled('div')` - position: absolute; +const Centered = styled('div')` justify-self: center; `; -const Wrapper = styled('div')` - display: flex; +const NoFeedbackWrapper = styled('div')` padding: ${space(4)} ${space(4)}; flex-direction: column; align-items: center; @@ -175,12 +105,9 @@ const Wrapper = styled('div')` @media (max-width: ${p => p.theme.breakpoints.sm}) { font-size: ${p => p.theme.fontSize.md}; } - position: relative; - top: 50%; - transform: translateY(-50%); `; -const EmptyMessage = styled('div')` +const NoFeedbackMessage = styled('div')` font-weight: ${p => p.theme.fontWeight.bold}; color: ${p => p.theme.gray400}; diff --git a/static/app/components/feedback/list/feedbackListItem.tsx b/static/app/components/feedback/list/feedbackListItem.tsx index 7256e2c9c3cf9a..efcc2216d52e87 100644 --- a/static/app/components/feedback/list/feedbackListItem.tsx +++ b/static/app/components/feedback/list/feedbackListItem.tsx @@ -1,4 +1,3 @@ -import type {CSSProperties} from 'react'; import styled from '@emotion/styled'; import {ActorAvatar} from 'sentry/components/core/avatar/actorAvatar'; @@ -29,8 +28,6 @@ interface Props { feedbackItem: FeedbackIssueListItem; isSelected: 'all-selected' | boolean; onSelect: (isSelected: boolean) => void; - ref?: React.Ref; - style?: CSSProperties; } function useIsSelectedFeedback({feedbackItem}: {feedbackItem: FeedbackIssueListItem}) { @@ -41,7 +38,7 @@ function useIsSelectedFeedback({feedbackItem}: {feedbackItem: FeedbackIssueListI return feedbackId === feedbackItem.id; } -function FeedbackListItem({feedbackItem, isSelected, onSelect, style, ref}: Props) { +export default function FeedbackListItem({feedbackItem, isSelected, onSelect}: Props) { const organization = useOrganization(); const isOpen = useIsSelectedFeedback({feedbackItem}); const {feedbackHasReplay} = useReplayCountForFeedbacks(); @@ -53,7 +50,7 @@ function FeedbackListItem({feedbackItem, isSelected, onSelect, style, ref}: Prop const hasComments = feedbackItem.numComments > 0; return ( - + = Pick> & U; + +interface Props { + itemRenderer: ({ + item, + virtualItem, + }: { + item: Data; + virtualItem: VirtualItem; + }) => React.ReactNode; + queryResult: Overwrite< + Pick< + UseInfiniteQueryResult>, Error>, + 'data' | 'hasNextPage' | 'isFetchingNextPage' | 'fetchNextPage' + >, + {fetchNextPage: () => Promise} + >; + emptyMessage?: () => React.ReactNode; + estimateSize?: () => number; + loadingCompleteMessage?: () => React.ReactNode; + loadingMoreMessage?: () => React.ReactNode; + overscan?: number; +} + +export default function InfiniteListItems({ + estimateSize, + itemRenderer, + emptyMessage = EmptyMessage, + loadingCompleteMessage = LoadingCompleteMessage, + loadingMoreMessage = LoadingMoreMessage, + overscan, + queryResult, +}: Props) { + const {data, hasNextPage, isFetchingNextPage, fetchNextPage} = queryResult; + const loadedRows = data ? data.pages.flatMap(d => d[0]) : []; + const parentRef = useRef(null); + + const rowVirtualizer = useVirtualizer({ + count: hasNextPage ? loadedRows.length + 1 : loadedRows.length, + getScrollElement: () => parentRef.current, + estimateSize: estimateSize ?? (() => 100), + overscan: overscan ?? 5, + }); + const items = rowVirtualizer.getVirtualItems(); + + useEffect(() => { + const lastItem = items.at(-1); + if (!lastItem) { + return; + } + + if (lastItem.index >= loadedRows.length - 1 && hasNextPage && !isFetchingNextPage) { + fetchNextPage(); + } + }, [hasNextPage, fetchNextPage, loadedRows.length, isFetchingNextPage, items]); + + return ( + + + + {items.length ? null : emptyMessage()} + {items.map(virtualItem => { + const isLoaderRow = virtualItem.index > loadedRows.length - 1; + const item = loadedRows.at(virtualItem.index); + + return ( +
  • + {isLoaderRow + ? hasNextPage + ? loadingMoreMessage() + : loadingCompleteMessage() + : item && itemRenderer({virtualItem, item})} +
    • + ); + })} +     +     +     + ); +} + +function EmptyMessage() { + return

    {t('No items to show')}

    ; +} + +function LoadingMoreMessage() { + return ( +
    + +
    + ); +} + +function LoadingCompleteMessage() { + return

    {t('Nothing more to load')}

    ; +} + +const FlexOverscroll = styled('div')` + display: flex; + width: 100%; + height: 100%; + flex-direction: column; + overflow: auto; + overscroll-behavior: contain; + contain: strict; +`; + +const FlexListContainer = styled('div')` + position: absolute; + display: flex; + width: 100%; + flex-direction: column; +`; + +const PositionedList = styled('ul')` + position: absolute; + left: 0; + top: 0; + width: 100%; + + margin: 0; + padding: 0; + list-style: none; + & > li { + margin: 0; + padding: 0; + } +`; + +const Footer = styled('footer')` + position: absolute; + bottom: 0; + z-index: ${p => p.theme.zIndex.initial}; + display: flex; + width: 100%; + flex-grow: 1; + align-items: center; + justify-content: center; +`; diff --git a/static/app/components/infiniteList/infiniteListState.tsx b/static/app/components/infiniteList/infiniteListState.tsx new file mode 100644 index 00000000000000..852e1d64d8277b --- /dev/null +++ b/static/app/components/infiniteList/infiniteListState.tsx @@ -0,0 +1,59 @@ +import {Fragment} from 'react'; +import type {UseInfiniteQueryResult, UseQueryResult} from '@tanstack/react-query'; + +import type {ApiResult} from 'sentry/api'; + +interface Props { + children: React.ReactNode; + queryResult: + | Pick, Error>, 'status' | 'error' | 'isFetching'> + | Pick< + UseInfiniteQueryResult, + 'status' | 'error' | 'isFetching' | 'isFetchingNextPage' + >; + backgroundUpdatingMessage?: () => React.ReactNode; + errorMessage?: (props: {error: Error}) => React.ReactNode; + loadingMessage?: () => React.ReactNode; +} + +export default function InfiniteListState({ + backgroundUpdatingMessage = BackgroundUpdatingMessage, + children, + errorMessage = ErrorMessage, + loadingMessage = LoadingMessage, + queryResult, +}: Props) { + const {status, error, isFetching} = queryResult; + if (status === 'pending') { + return loadingMessage(); + } + if (status === 'error') { + return errorMessage({error: error!}); + } + + // It's fetching in the background if: + // - it's a regular QueryResult, and isFetching is true + // - it's an InfiniteQueryResult, and itFetching is true, but we're not fetching the next page + const isBackgroundUpdating = + isFetching && + ('isFetchingNextPage' in queryResult ? !queryResult.isFetchingNextPage : true); + + return ( + + {children} + {isBackgroundUpdating ? backgroundUpdatingMessage() : null} + + ); +} + +function LoadingMessage() { + return

    Loading...

    ; +} + +function ErrorMessage({error}: {error: Error}) { + return

    Error: {error.message}

    ; +} + +function BackgroundUpdatingMessage() { + return
    Background Updating...
    ; +} diff --git a/static/app/utils/api/useFetchInfiniteListData.tsx b/static/app/utils/api/useFetchInfiniteListData.tsx deleted file mode 100644 index 5ef4b323376f2f..00000000000000 --- a/static/app/utils/api/useFetchInfiniteListData.tsx +++ /dev/null @@ -1,86 +0,0 @@ -import {useCallback, useMemo} from 'react'; -import type {Index, IndexRange} from 'react-virtualized'; - -import type {InfiniteApiQueryKey} from 'sentry/utils/queryClient'; -import {useInfiniteApiQuery} from 'sentry/utils/queryClient'; - -function uniqueItems>( - items: Data[], - uniqueField: string -) { - const uniqueIds = new Set(items.map(item => item[uniqueField])); - return items.filter(item => { - if (uniqueIds.has(item[uniqueField])) { - uniqueIds.delete(item[uniqueField]); - return true; - } - return false; - }); -} - -interface Props { - queryKey: NonNullable; - uniqueField: string; - enabled?: boolean; -} - -export default function useFetchInfiniteListData>({ - queryKey, - uniqueField, - enabled, -}: Props) { - const { - data, - error, - fetchNextPage, - hasNextPage, - isError, - isFetching, // If the network is active - isFetchingNextPage, - isFetchingPreviousPage, - isPending, // If anything is loaded yet - } = useInfiniteApiQuery({ - queryKey, - enabled, - }); - - const issues = useMemo( - () => uniqueItems(data?.pages.flatMap(([pageData]) => pageData) ?? [], uniqueField), - [data, uniqueField] - ); - - const getRow = useCallback( - ({index}: Index): Data | undefined => issues?.[index], - [issues] - ); - - const isRowLoaded = useCallback(({index}: Index) => Boolean(issues[index]), [issues]); - - const loadMoreRows = useCallback( - ({startIndex: _1, stopIndex: _2}: IndexRange): Promise => - hasNextPage ? fetchNextPage() : Promise.resolve(), - [hasNextPage, fetchNextPage] - ); - - const hits = useMemo( - () => - data?.pages.map(([, , resp]) => Number(resp?.getResponseHeader('X-Hits'))) ?? [], - [data] - ); - - return { - error, - hasNextPage, - isError, - isFetching, // If the network is active - isFetchingNextPage, - isFetchingPreviousPage, - isLoading: isPending, // If anything is loaded yet - // Below are fields that are shims for react-virtualized - getRow, - isRowLoaded, - issues, - loadMoreRows, - hits: Math.max(...hits), - }; -} diff --git a/static/app/utils/array/uniqBy.ts b/static/app/utils/array/uniqBy.ts new file mode 100644 index 00000000000000..498fc3f5a6e72c --- /dev/null +++ b/static/app/utils/array/uniqBy.ts @@ -0,0 +1,13 @@ +export default function uniqueBy>( + items: Data[], + uniqueField: keyof Data +) { + const uniqueIds = new Set(items.map(item => item[uniqueField])); + return items.filter(item => { + if (uniqueIds.has(item[uniqueField])) { + uniqueIds.delete(item[uniqueField]); + return true; + } + return false; + }); +} From b9492a9fc2901c7bb4c9f1d358c60952eae1f3e0 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Mon, 14 Jul 2025 11:53:54 -0700 Subject: [PATCH 2/5] Try to select all feedbacks by reading `X-Hits` --- static/app/components/feedback/list/feedbackList.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/static/app/components/feedback/list/feedbackList.tsx b/static/app/components/feedback/list/feedbackList.tsx index 9529b6129c7560..f4c4442b87d634 100644 --- a/static/app/components/feedback/list/feedbackList.tsx +++ b/static/app/components/feedback/list/feedbackList.tsx @@ -40,7 +40,9 @@ export default function FeedbackList() { [queryResult.data?.pages] ); const checkboxState = useListItemCheckboxContext({ - hits: issues.length, + hits: Number( + queryResult.data?.pages[0]?.[2]?.getResponseHeader('X-Hits') ?? issues.length + ), knownIds: issues.map(issue => issue.id), queryKey: listQueryKey, }); From 2c9d00594c1ede4cac222331f51bcee9ba351fe6 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Mon, 14 Jul 2025 16:47:39 -0700 Subject: [PATCH 3/5] remove unneeded css props --- static/app/components/feedback/list/feedbackList.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/static/app/components/feedback/list/feedbackList.tsx b/static/app/components/feedback/list/feedbackList.tsx index f4c4442b87d634..d574f96592184f 100644 --- a/static/app/components/feedback/list/feedbackList.tsx +++ b/static/app/components/feedback/list/feedbackList.tsx @@ -99,8 +99,6 @@ const Centered = styled('div')` const NoFeedbackWrapper = styled('div')` padding: ${space(4)} ${space(4)}; - flex-direction: column; - align-items: center; text-align: center; color: ${p => p.theme.subText}; From 635eb8169b87f771dec3ac2c2cf7430806d0eb11 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Tue, 15 Jul 2025 10:18:56 -0700 Subject: [PATCH 4/5] translate default inifiniteState messages --- static/app/components/infiniteList/infiniteListState.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/static/app/components/infiniteList/infiniteListState.tsx b/static/app/components/infiniteList/infiniteListState.tsx index 852e1d64d8277b..aa76a7cbfe30c3 100644 --- a/static/app/components/infiniteList/infiniteListState.tsx +++ b/static/app/components/infiniteList/infiniteListState.tsx @@ -2,6 +2,7 @@ import {Fragment} from 'react'; import type {UseInfiniteQueryResult, UseQueryResult} from '@tanstack/react-query'; import type {ApiResult} from 'sentry/api'; +import {t} from 'sentry/locale'; interface Props { children: React.ReactNode; @@ -47,13 +48,13 @@ export default function InfiniteListState({ } function LoadingMessage() { - return

    Loading...

    ; + return

    {t('Loading...')}

    ; } function ErrorMessage({error}: {error: Error}) { - return

    Error: {error.message}

    ; + return

    {t('Error: %s', error.message)}

    ; } function BackgroundUpdatingMessage() { - return
    Background Updating...
    ; + return
    {t('Background Updating...')}
    ; } From 87bfa4704cee600b3bf1408d4372e91c81e0da70 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Tue, 15 Jul 2025 10:21:50 -0700 Subject: [PATCH 5/5] use lodash for uniqBy --- .../app/components/feedback/list/feedbackList.tsx | 4 ++-- static/app/utils/array/uniqBy.ts | 13 ------------- 2 files changed, 2 insertions(+), 15 deletions(-) delete mode 100644 static/app/utils/array/uniqBy.ts diff --git a/static/app/components/feedback/list/feedbackList.tsx b/static/app/components/feedback/list/feedbackList.tsx index d574f96592184f..3515ffb726f16f 100644 --- a/static/app/components/feedback/list/feedbackList.tsx +++ b/static/app/components/feedback/list/feedbackList.tsx @@ -1,5 +1,6 @@ import {Fragment, useMemo} from 'react'; import styled from '@emotion/styled'; +import uniqBy from 'lodash/uniqBy'; import waitingForEventImg from 'sentry-images/spot/waiting-for-event.svg'; @@ -13,7 +14,6 @@ import InfiniteListState from 'sentry/components/infiniteList/infiniteListState' import LoadingIndicator from 'sentry/components/loadingIndicator'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; -import uniqueBy from 'sentry/utils/array/uniqBy'; import type {FeedbackIssueListItem} from 'sentry/utils/feedback/types'; import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState'; import {useInfiniteApiQuery} from 'sentry/utils/queryClient'; @@ -36,7 +36,7 @@ export default function FeedbackList() { }); const issues = useMemo( - () => uniqueBy(queryResult.data?.pages.flatMap(([pageData]) => pageData) ?? [], 'id'), + () => uniqBy(queryResult.data?.pages.flatMap(([pageData]) => pageData) ?? [], 'id'), [queryResult.data?.pages] ); const checkboxState = useListItemCheckboxContext({ diff --git a/static/app/utils/array/uniqBy.ts b/static/app/utils/array/uniqBy.ts deleted file mode 100644 index 498fc3f5a6e72c..00000000000000 --- a/static/app/utils/array/uniqBy.ts +++ /dev/null @@ -1,13 +0,0 @@ -export default function uniqueBy>( - items: Data[], - uniqueField: keyof Data -) { - const uniqueIds = new Set(items.map(item => item[uniqueField])); - return items.filter(item => { - if (uniqueIds.has(item[uniqueField])) { - uniqueIds.delete(item[uniqueField]); - return true; - } - return false; - }); -}