From 26d2cb974277e1da43db0d77e9ffbdd4dc964a93 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Mon, 6 Jan 2025 18:17:03 -0800 Subject: [PATCH 01/13] Add query counts back to tabs --- .../issueViews/issueViewQueryCount.tsx | 0 .../issueList/issueViews/issueViewTab.tsx | 59 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 static/app/views/issueList/issueViews/issueViewQueryCount.tsx diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/static/app/views/issueList/issueViews/issueViewTab.tsx b/static/app/views/issueList/issueViews/issueViewTab.tsx index 9381858422e20d..4751bb5d5ca846 100644 --- a/static/app/views/issueList/issueViews/issueViewTab.tsx +++ b/static/app/views/issueList/issueViews/issueViewTab.tsx @@ -2,11 +2,17 @@ import {useContext} from 'react'; import styled from '@emotion/styled'; import {motion} from 'framer-motion'; +import Badge from 'sentry/components/badge/badge'; import {TEMPORARY_TAB_KEY} from 'sentry/components/draggableTabs/draggableTabList'; import type {MenuItemProps} from 'sentry/components/dropdownMenu'; +import QueryCount from 'sentry/components/queryCount'; import {t} from 'sentry/locale'; +import type {PageFilters} from 'sentry/types/core'; import type {InjectedRouter} from 'sentry/types/legacyReactRouter'; +import {getUtcDateString} from 'sentry/utils/dates'; import {useNavigate} from 'sentry/utils/useNavigate'; +import useOrganization from 'sentry/utils/useOrganization'; +import usePageFilters from 'sentry/utils/usePageFilters'; import EditableTabTitle from 'sentry/views/issueList/issueViews/editableTabTitle'; import {IssueViewEllipsisMenu} from 'sentry/views/issueList/issueViews/issueViewEllipsisMenu'; import { @@ -14,7 +20,9 @@ import { type IssueView, IssueViewsContext, } from 'sentry/views/issueList/issueViews/issueViews'; +import {useFetchIssueCounts} from 'sentry/views/issueList/queries/useFetchIssueCounts'; +const TAB_MAX_COUNT = 99; interface IssueViewTabProps { editingTabKey: string | null; initialTabKey: string; @@ -23,6 +31,22 @@ interface IssueViewTabProps { view: IssueView; } +const constructCountTimeFrame = ( + pageFilters: PageFilters['datetime'] +): { + end?: string; + start?: string; + statsPeriod?: string; +} => { + if (pageFilters.period) { + return {statsPeriod: pageFilters.period}; + } + return { + ...(pageFilters.start ? {start: getUtcDateString(pageFilters.start)} : {}), + ...(pageFilters.end ? {end: getUtcDateString(pageFilters.end)} : {}), + }; +}; + export function IssueViewTab({ editingTabKey, initialTabKey, @@ -31,11 +55,24 @@ export function IssueViewTab({ view, }: IssueViewTabProps) { const navigate = useNavigate(); + const organization = useOrganization(); const {cursor: _cursor, page: _page, ...queryParams} = router?.location?.query ?? {}; const {tabListState, state, dispatch} = useContext(IssueViewsContext); const {views} = state; + const pageFilters = usePageFilters(); + + // TODO(msun): Once page filters are saved to views, remember to use the view's specific + // page filters here instead of the global pageFilters, if they exists. + const {data: queryCount, isLoading: queryCountLoading} = useFetchIssueCounts({ + orgSlug: organization.slug, + query: [view.query], + project: pageFilters.selection.projects, + environment: pageFilters.selection.environments, + ...constructCountTimeFrame(pageFilters.selection.datetime), + }); + const handleDuplicateView = () => { const newViewId = generateTempViewId(); const duplicatedTab = views.find(tab => tab.key === tabListState?.selectedKey); @@ -117,6 +154,16 @@ export function IssueViewTab({ (!tabListState && view.key === initialTabKey) } /> + {!queryCountLoading && queryCount && ( + + + + )} {/* If tablistState isn't initialized, we want to load the elipsis menu for the initial tab, that way it won't load in a second later and cause the tabs to shift and animate on load. */} @@ -242,3 +289,15 @@ const TabContentWrap = styled('span')` padding: 0; gap: 6px; `; + +const QueryCountBadge = styled(Badge)` + display: flex; + height: 16px; + align-items: center; + justify-content: center; + border-radius: 10px; + background: transparent; + border: 1px solid ${p => p.theme.gray200}; + color: ${p => p.theme.gray300}; + margin-left: 0; +`; From 5c199095215f5c202aacb892381ba0dc053bd77e Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Mon, 6 Jan 2025 21:24:22 -0800 Subject: [PATCH 02/13] Add new tests, fix old ones --- .../views/issueList/issueViewsHeader.spec.tsx | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/static/app/views/issueList/issueViewsHeader.spec.tsx b/static/app/views/issueList/issueViewsHeader.spec.tsx index 78a9cc4dbfb8d5..1b35ead95934ca 100644 --- a/static/app/views/issueList/issueViewsHeader.spec.tsx +++ b/static/app/views/issueList/issueViewsHeader.spec.tsx @@ -73,6 +73,11 @@ describe('IssueViewsHeader', () => { method: 'GET', body: getRequestViews, }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + body: {}, + }); }); it('renders all tabs, selects the first one by default, and replaces the query params accordingly', async () => { @@ -120,6 +125,11 @@ describe('IssueViewsHeader', () => { }, ], }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + body: {}, + }); render(, {router: defaultRouter}); @@ -153,6 +163,11 @@ describe('IssueViewsHeader', () => { }, ], }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + body: {}, + }); render(, { router: queryOnlyRouter, @@ -277,6 +292,11 @@ describe('IssueViewsHeader', () => { }, ], }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + body: {}, + }); const defaultTabDifferentQueryRouter = RouterFixture({ location: LocationFixture({ @@ -320,6 +340,11 @@ describe('IssueViewsHeader', () => { method: 'GET', body: getRequestViews, }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + body: {}, + }); }); it('switches tabs when clicked, and updates the query params accordingly', async () => { @@ -453,6 +478,11 @@ describe('IssueViewsHeader', () => { method: 'GET', body: getRequestViews, }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + body: {}, + }); }); it('should render the correct set of actions for an unchanged tab', async () => { @@ -792,4 +822,95 @@ describe('IssueViewsHeader', () => { }); }); }); + + describe('Issue views query counts', () => { + it('should render the correct count for a single view', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/group-search-views/`, + method: 'GET', + body: [getRequestViews[0]], + }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + query: { + query: getRequestViews[0]!.query, + }, + body: { + [getRequestViews[0]!.query]: 42, + }, + }); + + render(, {router: defaultRouter}); + + expect(await screen.findByText('42')).toBeInTheDocument(); + }); + + it('should render the correct count for multiple views', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/group-search-views/`, + method: 'GET', + body: getRequestViews, + }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + body: { + [getRequestViews[0]!.query]: 42, + [getRequestViews[1]!.query]: 6, + [getRequestViews[2]!.query]: 98, + }, + }); + + render(, {router: defaultRouter}); + + expect(await screen.findByText('42')).toBeInTheDocument(); + expect(screen.getByText('6')).toBeInTheDocument(); + expect(screen.getByText('98')).toBeInTheDocument(); + }); + + it('should show a max count of 99+ if the count is greater than 99', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/group-search-views/`, + method: 'GET', + body: [getRequestViews[0]], + }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + query: { + query: getRequestViews[0]!.query, + }, + body: { + [getRequestViews[0]!.query]: 101, + }, + }); + + render(, {router: defaultRouter}); + + expect(await screen.findByText('99+')).toBeInTheDocument(); + }); + + it('should show stil show a 0 query count if the count is 0', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/group-search-views/`, + method: 'GET', + body: [getRequestViews[0]], + }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/issues-count/`, + method: 'GET', + query: { + query: getRequestViews[0]!.query, + }, + body: { + [getRequestViews[0]!.query]: 0, + }, + }); + + render(, {router: defaultRouter}); + + expect(await screen.findByText('0')).toBeInTheDocument(); + }); + }); }); From fcf22f73698acbe76f1be9d9c6d4e23af861de12 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Mon, 6 Jan 2025 22:36:19 -0800 Subject: [PATCH 03/13] fix tab line height, remove unnecessary file --- .../issueList/issueViews/editableTabTitle.tsx | 2 ++ .../issueViews/issueViewQueryCount.tsx | 0 .../issueList/issueViews/issueViewTab.tsx | 18 ++++++++++-------- 3 files changed, 12 insertions(+), 8 deletions(-) delete mode 100644 static/app/views/issueList/issueViews/issueViewQueryCount.tsx diff --git a/static/app/views/issueList/issueViews/editableTabTitle.tsx b/static/app/views/issueList/issueViews/editableTabTitle.tsx index bde64d6ef282bd..110471261c32fb 100644 --- a/static/app/views/issueList/issueViews/editableTabTitle.tsx +++ b/static/app/views/issueList/issueViews/editableTabTitle.tsx @@ -144,6 +144,7 @@ const UnselectedTabTitle = styled('div')<{isSelected: boolean}>` text-overflow: ellipsis; padding-right: 1px; cursor: pointer; + line-height: 1.45; `; const StyledGrowingInput = styled(GrowingInput)<{ @@ -160,6 +161,7 @@ const StyledGrowingInput = styled(GrowingInput)<{ text-overflow: ellipsis; cursor: text; max-width: 325px; + line-height: 1.45; &, &:focus, diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx deleted file mode 100644 index e69de29bb2d1d6..00000000000000 diff --git a/static/app/views/issueList/issueViews/issueViewTab.tsx b/static/app/views/issueList/issueViews/issueViewTab.tsx index 4751bb5d5ca846..08e7d1e9ba96c4 100644 --- a/static/app/views/issueList/issueViews/issueViewTab.tsx +++ b/static/app/views/issueList/issueViews/issueViewTab.tsx @@ -155,14 +155,16 @@ export function IssueViewTab({ } /> {!queryCountLoading && queryCount && ( - - - + + + + + )} {/* If tablistState isn't initialized, we want to load the elipsis menu for the initial tab, that way it won't load in a second later From fe8ec7e149f03b39d4043d03434156466d26849b Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Tue, 7 Jan 2025 09:55:01 -0800 Subject: [PATCH 04/13] Minor style change --- static/app/views/issueList/issueViews/issueViewTab.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/static/app/views/issueList/issueViews/issueViewTab.tsx b/static/app/views/issueList/issueViews/issueViewTab.tsx index 08e7d1e9ba96c4..9c0953835e134e 100644 --- a/static/app/views/issueList/issueViews/issueViewTab.tsx +++ b/static/app/views/issueList/issueViews/issueViewTab.tsx @@ -23,6 +23,7 @@ import { import {useFetchIssueCounts} from 'sentry/views/issueList/queries/useFetchIssueCounts'; const TAB_MAX_COUNT = 99; + interface IssueViewTabProps { editingTabKey: string | null; initialTabKey: string; @@ -64,7 +65,7 @@ export function IssueViewTab({ const pageFilters = usePageFilters(); // TODO(msun): Once page filters are saved to views, remember to use the view's specific - // page filters here instead of the global pageFilters, if they exists. + // page filters here instead of the global pageFilters, if they exist. const {data: queryCount, isLoading: queryCountLoading} = useFetchIssueCounts({ orgSlug: organization.slug, query: [view.query], From f6dc071be16d0f599f98a25a378a753fd7ed0c15 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Tue, 7 Jan 2025 13:53:02 -0800 Subject: [PATCH 05/13] Fix issue with query not updating for unsaved changes, add loading state --- .../issueViews/issueViewQueryCount.tsx | 95 +++++++++++++++++++ .../issueList/issueViews/issueViewTab.tsx | 64 +------------ .../issueList/queries/useFetchIssueCounts.tsx | 3 +- 3 files changed, 99 insertions(+), 63 deletions(-) create mode 100644 static/app/views/issueList/issueViews/issueViewQueryCount.tsx diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx new file mode 100644 index 00000000000000..2e0c8a3e382324 --- /dev/null +++ b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx @@ -0,0 +1,95 @@ +import styled from '@emotion/styled'; +import {motion} from 'framer-motion'; + +import type {PageFilters} from 'sentry/types/core'; +import {getUtcDateString} from 'sentry/utils/dates'; +import theme from 'sentry/utils/theme'; +import useOrganization from 'sentry/utils/useOrganization'; +import usePageFilters from 'sentry/utils/usePageFilters'; +import type {IssueView} from 'sentry/views/issueList/issueViews/issueViews'; +import {useFetchIssueCounts} from 'sentry/views/issueList/queries/useFetchIssueCounts'; + +const TAB_MAX_COUNT = 99; + +const constructCountTimeFrame = ( + pageFilters: PageFilters['datetime'] +): { + end?: string; + start?: string; + statsPeriod?: string; +} => { + if (pageFilters.period) { + return {statsPeriod: pageFilters.period}; + } + return { + ...(pageFilters.start ? {start: getUtcDateString(pageFilters.start)} : {}), + ...(pageFilters.end ? {end: getUtcDateString(pageFilters.end)} : {}), + }; +}; + +interface IssueViewQueryCountProps { + view: IssueView; +} + +export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { + const organization = useOrganization(); + const pageFilters = usePageFilters(); + + // TODO(msun): Once page filters are saved to views, remember to use the view's specific + // page filters here instead of the global pageFilters, if they exist. + const {data: queryCount, isFetching: queryCountFetching} = useFetchIssueCounts({ + orgSlug: organization.slug, + query: [view.unsavedChanges ? view.unsavedChanges[0] : view.query], + project: pageFilters.selection.projects, + environment: pageFilters.selection.environments, + ...constructCountTimeFrame(pageFilters.selection.datetime), + }); + + const displayedCount = + queryCount?.[view.unsavedChanges ? view.unsavedChanges[0] : view.query] ?? 0; + + return ( + + + {displayedCount > TAB_MAX_COUNT ? `${TAB_MAX_COUNT}+` : displayedCount} + + + ); +} + +const QueryCountBubble = styled(motion.span)` + line-height: 20px; + font-size: 75%; + padding: 0 5px; + min-width: 20px; + display: flex; + height: 16px; + align-items: center; + justify-content: center; + border-radius: 10px; + border: 1px solid ${p => p.theme.gray200}; + color: ${p => p.theme.gray300}; + margin-left: 0; +`; diff --git a/static/app/views/issueList/issueViews/issueViewTab.tsx b/static/app/views/issueList/issueViews/issueViewTab.tsx index 9c0953835e134e..98d64aed1ad9eb 100644 --- a/static/app/views/issueList/issueViews/issueViewTab.tsx +++ b/static/app/views/issueList/issueViews/issueViewTab.tsx @@ -2,27 +2,19 @@ import {useContext} from 'react'; import styled from '@emotion/styled'; import {motion} from 'framer-motion'; -import Badge from 'sentry/components/badge/badge'; import {TEMPORARY_TAB_KEY} from 'sentry/components/draggableTabs/draggableTabList'; import type {MenuItemProps} from 'sentry/components/dropdownMenu'; -import QueryCount from 'sentry/components/queryCount'; import {t} from 'sentry/locale'; -import type {PageFilters} from 'sentry/types/core'; import type {InjectedRouter} from 'sentry/types/legacyReactRouter'; -import {getUtcDateString} from 'sentry/utils/dates'; import {useNavigate} from 'sentry/utils/useNavigate'; -import useOrganization from 'sentry/utils/useOrganization'; -import usePageFilters from 'sentry/utils/usePageFilters'; import EditableTabTitle from 'sentry/views/issueList/issueViews/editableTabTitle'; import {IssueViewEllipsisMenu} from 'sentry/views/issueList/issueViews/issueViewEllipsisMenu'; +import {IssueViewQueryCount} from 'sentry/views/issueList/issueViews/issueViewQueryCount'; import { generateTempViewId, type IssueView, IssueViewsContext, } from 'sentry/views/issueList/issueViews/issueViews'; -import {useFetchIssueCounts} from 'sentry/views/issueList/queries/useFetchIssueCounts'; - -const TAB_MAX_COUNT = 99; interface IssueViewTabProps { editingTabKey: string | null; @@ -32,22 +24,6 @@ interface IssueViewTabProps { view: IssueView; } -const constructCountTimeFrame = ( - pageFilters: PageFilters['datetime'] -): { - end?: string; - start?: string; - statsPeriod?: string; -} => { - if (pageFilters.period) { - return {statsPeriod: pageFilters.period}; - } - return { - ...(pageFilters.start ? {start: getUtcDateString(pageFilters.start)} : {}), - ...(pageFilters.end ? {end: getUtcDateString(pageFilters.end)} : {}), - }; -}; - export function IssueViewTab({ editingTabKey, initialTabKey, @@ -56,24 +32,11 @@ export function IssueViewTab({ view, }: IssueViewTabProps) { const navigate = useNavigate(); - const organization = useOrganization(); const {cursor: _cursor, page: _page, ...queryParams} = router?.location?.query ?? {}; const {tabListState, state, dispatch} = useContext(IssueViewsContext); const {views} = state; - const pageFilters = usePageFilters(); - - // TODO(msun): Once page filters are saved to views, remember to use the view's specific - // page filters here instead of the global pageFilters, if they exist. - const {data: queryCount, isLoading: queryCountLoading} = useFetchIssueCounts({ - orgSlug: organization.slug, - query: [view.query], - project: pageFilters.selection.projects, - environment: pageFilters.selection.environments, - ...constructCountTimeFrame(pageFilters.selection.datetime), - }); - const handleDuplicateView = () => { const newViewId = generateTempViewId(); const duplicatedTab = views.find(tab => tab.key === tabListState?.selectedKey); @@ -155,18 +118,7 @@ export function IssueViewTab({ (!tabListState && view.key === initialTabKey) } /> - {!queryCountLoading && queryCount && ( - - - - - - )} + {/* If tablistState isn't initialized, we want to load the elipsis menu for the initial tab, that way it won't load in a second later and cause the tabs to shift and animate on load. */} @@ -292,15 +244,3 @@ const TabContentWrap = styled('span')` padding: 0; gap: 6px; `; - -const QueryCountBadge = styled(Badge)` - display: flex; - height: 16px; - align-items: center; - justify-content: center; - border-radius: 10px; - background: transparent; - border: 1px solid ${p => p.theme.gray200}; - color: ${p => p.theme.gray300}; - margin-left: 0; -`; diff --git a/static/app/views/issueList/queries/useFetchIssueCounts.tsx b/static/app/views/issueList/queries/useFetchIssueCounts.tsx index 386e855332a061..4a17a816f52063 100644 --- a/static/app/views/issueList/queries/useFetchIssueCounts.tsx +++ b/static/app/views/issueList/queries/useFetchIssueCounts.tsx @@ -1,5 +1,5 @@ import type {ApiQueryKey, UseApiQueryOptions} from 'sentry/utils/queryClient'; -import {useApiQuery} from 'sentry/utils/queryClient'; +import {keepPreviousData, useApiQuery} from 'sentry/utils/queryClient'; import type {QueryCount, QueryCounts} from 'sentry/views/issueList/utils'; interface FetchIssueCountsParameters { @@ -31,6 +31,7 @@ export const useFetchIssueCounts = ( ) => { return useApiQuery(makeFetchIssueCounts(params), { staleTime: 0, + placeholderData: keepPreviousData, ...options, }); }; From 5953f0c652390271b91eddbee0a695cde6c29ac1 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Tue, 7 Jan 2025 15:44:19 -0800 Subject: [PATCH 06/13] Use theme for some css --- .../app/views/issueList/issueViews/issueViewQueryCount.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx index 2e0c8a3e382324..21fa0578166bd8 100644 --- a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx +++ b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx @@ -1,6 +1,7 @@ import styled from '@emotion/styled'; import {motion} from 'framer-motion'; +import {space} from 'sentry/styles/space'; import type {PageFilters} from 'sentry/types/core'; import {getUtcDateString} from 'sentry/utils/dates'; import theme from 'sentry/utils/theme'; @@ -81,8 +82,8 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { const QueryCountBubble = styled(motion.span)` line-height: 20px; - font-size: 75%; - padding: 0 5px; + font-size: ${p => p.theme.fontSizeExtraSmall}; + padding: 0 ${space(0.5)}; min-width: 20px; display: flex; height: 16px; From c87ba48bd27b1f970167ee6dc02f16e826169f9c Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Tue, 7 Jan 2025 15:50:27 -0800 Subject: [PATCH 07/13] Make stale time 3 minutes --- static/app/views/issueList/queries/useFetchIssueCounts.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/issueList/queries/useFetchIssueCounts.tsx b/static/app/views/issueList/queries/useFetchIssueCounts.tsx index 4a17a816f52063..c79050345eae38 100644 --- a/static/app/views/issueList/queries/useFetchIssueCounts.tsx +++ b/static/app/views/issueList/queries/useFetchIssueCounts.tsx @@ -30,7 +30,7 @@ export const useFetchIssueCounts = ( options: Partial>> = {} ) => { return useApiQuery(makeFetchIssueCounts(params), { - staleTime: 0, + staleTime: 180000, // 3 minutes placeholderData: keepPreviousData, ...options, }); From 2d5726d1bbb3050eca8db11d1977ee804781e2b1 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Tue, 7 Jan 2025 16:14:49 -0800 Subject: [PATCH 08/13] Update animations to work with dark mode --- .../issueList/issueViews/issueViewQueryCount.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx index 21fa0578166bd8..6cd0069b22ee83 100644 --- a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx +++ b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx @@ -1,10 +1,10 @@ +import {useTheme} from '@emotion/react'; import styled from '@emotion/styled'; import {motion} from 'framer-motion'; import {space} from 'sentry/styles/space'; import type {PageFilters} from 'sentry/types/core'; import {getUtcDateString} from 'sentry/utils/dates'; -import theme from 'sentry/utils/theme'; import useOrganization from 'sentry/utils/useOrganization'; import usePageFilters from 'sentry/utils/usePageFilters'; import type {IssueView} from 'sentry/views/issueList/issueViews/issueViews'; @@ -35,6 +35,7 @@ interface IssueViewQueryCountProps { export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { const organization = useOrganization(); const pageFilters = usePageFilters(); + const theme = useTheme(); // TODO(msun): Once page filters are saved to views, remember to use the view's specific // page filters here instead of the global pageFilters, if they exist. @@ -54,15 +55,15 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { layout="size" animate={{ backgroundColor: queryCountFetching - ? [theme.gray100, theme.translucentSurface100, theme.gray100] - : 'transparent', + ? [theme.surface400, theme.surface100, theme.surface400] + : `#00000000`, }} transition={{ layout: { - duration: 0.1, + duration: 0.2, }, default: { - duration: 2.5, + duration: 2, repeat: queryCountFetching ? Infinity : 0, ease: 'easeInOut', }, @@ -72,7 +73,7 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { layout="position" initial={{opacity: 0}} animate={{opacity: queryCountFetching ? 0 : 1}} - transition={{duration: 0.1}} + transition={{duration: 0.15}} > {displayedCount > TAB_MAX_COUNT ? `${TAB_MAX_COUNT}+` : displayedCount} From 5a837ad0f290843cfdda595551985c7a6c255279 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Tue, 7 Jan 2025 19:40:22 -0800 Subject: [PATCH 09/13] Fix tests --- .../views/issueList/issueViewsHeader.spec.tsx | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/static/app/views/issueList/issueViewsHeader.spec.tsx b/static/app/views/issueList/issueViewsHeader.spec.tsx index 1b35ead95934ca..c1570d000172ac 100644 --- a/static/app/views/issueList/issueViewsHeader.spec.tsx +++ b/static/app/views/issueList/issueViewsHeader.spec.tsx @@ -83,10 +83,10 @@ describe('IssueViewsHeader', () => { it('renders all tabs, selects the first one by default, and replaces the query params accordingly', async () => { render(, {router: defaultRouter}); - expect(await screen.findByRole('tab', {name: 'High Priority'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Medium Priority'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Low Priority'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'High Priority'})).toHaveAttribute( + expect(await screen.findByRole('tab', {name: /High Priority/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Medium Priority/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Low Priority/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /High Priority/})).toHaveAttribute( 'aria-selected', 'true' ); @@ -133,8 +133,8 @@ describe('IssueViewsHeader', () => { render(, {router: defaultRouter}); - expect(await screen.findByRole('tab', {name: 'Prioritized'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Prioritized'})).toHaveAttribute( + expect(await screen.findByRole('tab', {name: /Prioritized/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Prioritized/})).toHaveAttribute( 'aria-selected', 'true' ); @@ -173,9 +173,9 @@ describe('IssueViewsHeader', () => { router: queryOnlyRouter, }); - expect(await screen.findByRole('tab', {name: 'Prioritized'})).toBeInTheDocument(); - expect(await screen.findByRole('tab', {name: 'Unsaved'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Unsaved'})).toHaveAttribute( + expect(await screen.findByRole('tab', {name: /Prioritized/})).toBeInTheDocument(); + expect(await screen.findByRole('tab', {name: /Unsaved/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Unsaved/})).toHaveAttribute( 'aria-selected', 'true' ); @@ -204,7 +204,7 @@ describe('IssueViewsHeader', () => { router: specificTabRouter, }); - expect(await screen.findByRole('tab', {name: 'Medium Priority'})).toHaveAttribute( + expect(await screen.findByRole('tab', {name: /Medium Priority/})).toHaveAttribute( 'aria-selected', 'true' ); @@ -225,13 +225,13 @@ describe('IssueViewsHeader', () => { router: queryOnlyRouter, }); - expect(await screen.findByRole('tab', {name: 'High Priority'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Medium Priority'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Low Priority'})).toBeInTheDocument(); + expect(await screen.findByRole('tab', {name: /High Priority/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Medium Priority/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Low Priority/})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Unsaved'})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Unsaved/})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Unsaved'})).toHaveAttribute( + expect(screen.getByRole('tab', {name: /Unsaved/})).toHaveAttribute( 'aria-selected', 'true' ); @@ -258,13 +258,13 @@ describe('IssueViewsHeader', () => { router: specificTabRouter, }); - expect(await screen.findByRole('tab', {name: 'High Priority'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Medium Priority'})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Low Priority'})).toBeInTheDocument(); + expect(await screen.findByRole('tab', {name: /High Priority/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Medium Priority/})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Low Priority/})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Unsaved'})).toBeInTheDocument(); + expect(screen.getByRole('tab', {name: /Unsaved/})).toBeInTheDocument(); - expect(screen.getByRole('tab', {name: 'Unsaved'})).toHaveAttribute( + expect(screen.getByRole('tab', {name: /Unsaved/})).toHaveAttribute( 'aria-selected', 'true' ); @@ -317,9 +317,9 @@ describe('IssueViewsHeader', () => { router: defaultTabDifferentQueryRouter, } ); - expect(await screen.findByRole('tab', {name: 'Prioritized'})).toBeInTheDocument(); + expect(await screen.findByRole('tab', {name: /Prioritized/})).toBeInTheDocument(); expect(screen.getByTestId('unsaved-changes-indicator')).toBeInTheDocument(); - expect(screen.queryByRole('tab', {name: 'Unsaved'})).not.toBeInTheDocument(); + expect(screen.queryByRole('tab', {name: /Unsaved/})).not.toBeInTheDocument(); expect(defaultTabDifferentQueryRouter.replace).toHaveBeenCalledWith( expect.objectContaining({ @@ -350,7 +350,7 @@ describe('IssueViewsHeader', () => { it('switches tabs when clicked, and updates the query params accordingly', async () => { render(, {router: defaultRouter}); - await userEvent.click(await screen.findByRole('tab', {name: 'Medium Priority'})); + await userEvent.click(await screen.findByRole('tab', {name: /Medium Priority/})); // This test inexplicably fails on the lines below. which ensure the Medium Priority tab is selected when clicked // and the High Priority tab is unselected. This behavior exists in other tests and in browser, so idk why it fails here. @@ -384,11 +384,11 @@ describe('IssueViewsHeader', () => { }); expect(await screen.findByTestId('unsaved-changes-indicator')).toBeInTheDocument(); - await userEvent.click(await screen.findByRole('tab', {name: 'Medium Priority'})); + await userEvent.click(await screen.findByRole('tab', {name: /Medium Priority/})); expect(screen.queryByTestId('unsaved-changes-indicator')).not.toBeInTheDocument(); - await userEvent.click(await screen.findByRole('tab', {name: 'High Priority'})); - expect(await screen.findByRole('tab', {name: 'High Priority'})).toHaveAttribute( + await userEvent.click(await screen.findByRole('tab', {name: /High Priority/})); + expect(await screen.findByRole('tab', {name: /High Priority/})).toHaveAttribute( 'aria-selected', 'true' ); @@ -414,7 +414,7 @@ describe('IssueViewsHeader', () => { {router: goodViewIdChangedQueryRouter} ); - expect(await screen.findByRole('tab', {name: 'Medium Priority'})).toHaveAttribute( + expect(await screen.findByRole('tab', {name: /Medium Priority/})).toHaveAttribute( 'aria-selected', 'true' ); @@ -451,7 +451,7 @@ describe('IssueViewsHeader', () => { {router: goodViewIdChangedSortRouter} ); - expect(await screen.findByRole('tab', {name: 'Medium Priority'})).toHaveAttribute( + expect(await screen.findByRole('tab', {name: /Medium Priority/})).toHaveAttribute( 'aria-selected', 'true' ); From 9e0491826f718c93a5633007d2d203fd7292b524 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Wed, 8 Jan 2025 10:47:52 -0800 Subject: [PATCH 10/13] Disable fade in animatino if query count is cached on mount --- .../issueList/issueViews/issueViewQueryCount.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx index 6cd0069b22ee83..c1a954e997df1d 100644 --- a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx +++ b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx @@ -39,7 +39,11 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { // TODO(msun): Once page filters are saved to views, remember to use the view's specific // page filters here instead of the global pageFilters, if they exist. - const {data: queryCount, isFetching: queryCountFetching} = useFetchIssueCounts({ + const { + data: queryCount, + isLoading, + isFetching, + } = useFetchIssueCounts({ orgSlug: organization.slug, query: [view.unsavedChanges ? view.unsavedChanges[0] : view.query], project: pageFilters.selection.projects, @@ -54,7 +58,7 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { {displayedCount > TAB_MAX_COUNT ? `${TAB_MAX_COUNT}+` : displayedCount} From 6f26b3303589eba27086a93cde0c13b956f67903 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Wed, 8 Jan 2025 10:53:13 -0800 Subject: [PATCH 11/13] Halt background animation once query count loads --- static/app/views/issueList/issueViews/issueViewQueryCount.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx index c1a954e997df1d..cfff2368915714 100644 --- a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx +++ b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx @@ -67,7 +67,7 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { duration: 0.2, }, default: { - duration: 2, + duration: isFetching ? 2 : 0, repeat: isFetching ? Infinity : 0, ease: 'easeInOut', }, From d17e740257ff823682fbfc1478e9e65668b9329c Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Fri, 10 Jan 2025 12:10:37 -0800 Subject: [PATCH 12/13] Preserve bubble size when making change to query --- .../issueViews/issueViewQueryCount.tsx | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx index cfff2368915714..1da4f96ebc6fac 100644 --- a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx +++ b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx @@ -1,3 +1,4 @@ +import {useEffect, useState} from 'react'; import {useTheme} from '@emotion/react'; import styled from '@emotion/styled'; import {motion} from 'framer-motion'; @@ -37,12 +38,15 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { const pageFilters = usePageFilters(); const theme = useTheme(); + const [count, setCount] = useState(0); + // TODO(msun): Once page filters are saved to views, remember to use the view's specific // page filters here instead of the global pageFilters, if they exist. const { data: queryCount, isLoading, isFetching, + isError, } = useFetchIssueCounts({ orgSlug: organization.slug, query: [view.unsavedChanges ? view.unsavedChanges[0] : view.query], @@ -51,8 +55,17 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { ...constructCountTimeFrame(pageFilters.selection.datetime), }); - const displayedCount = - queryCount?.[view.unsavedChanges ? view.unsavedChanges[0] : view.query] ?? 0; + useEffect(() => { + // Only update the count once the query has finished fetching + // This preserves the previous count while the query is fetching a new one + if (queryCount && !isFetching) { + setCount( + queryCount?.[view.unsavedChanges ? view.unsavedChanges[0] : view.query] ?? 0 + ); + } else if (isError) { + setCount(0); + } + }, [queryCount, isFetching, isError, view.query, view.unsavedChanges]); return ( - {displayedCount > TAB_MAX_COUNT ? `${TAB_MAX_COUNT}+` : displayedCount} + {count > TAB_MAX_COUNT ? `${TAB_MAX_COUNT}+` : count} ); From 8b32cdcca1fef92bb931d6c04d7cf882cb829c19 Mon Sep 17 00:00:00 2001 From: MichaelSun48 Date: Fri, 10 Jan 2025 13:01:37 -0800 Subject: [PATCH 13/13] Preserve aspect ratio of count when animation --- static/app/views/issueList/issueViews/issueViewQueryCount.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx index 1da4f96ebc6fac..3fddd47aaf4faf 100644 --- a/static/app/views/issueList/issueViews/issueViewQueryCount.tsx +++ b/static/app/views/issueList/issueViews/issueViewQueryCount.tsx @@ -89,6 +89,7 @@ export function IssueViewQueryCount({view}: IssueViewQueryCountProps) { >