From 9285a1ccd0866a623ba74a34803e73e902e1d8f3 Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Thu, 21 Aug 2025 12:32:15 -0400 Subject: [PATCH] fix(ui): Consistently use min-width media queries Because we were using a mix of `max-width` and `min-width` media queries at the specific pixel size of the query one would apply but not the other, in some cases causing the layout to completely break. --- .../profiling/flamegraph/flamegraph.spec.tsx | 14 ----------- static/app/views/issueList/actions/index.tsx | 2 +- static/app/views/issueList/groupListBody.tsx | 6 ++--- static/app/views/nav/context.tsx | 2 +- static/app/views/nav/index.spec.tsx | 16 +++---------- .../profileSummaryPage.spec.tsx | 23 ++++++++----------- tests/js/sentry-test/utils.tsx | 16 +++++++++++++ tests/js/setup.ts | 14 +++++++++++ 8 files changed, 47 insertions(+), 46 deletions(-) diff --git a/static/app/components/profiling/flamegraph/flamegraph.spec.tsx b/static/app/components/profiling/flamegraph/flamegraph.spec.tsx index 42651a55c09784..a8c26bcc61b4d6 100644 --- a/static/app/components/profiling/flamegraph/flamegraph.spec.tsx +++ b/static/app/components/profiling/flamegraph/flamegraph.spec.tsx @@ -92,20 +92,6 @@ const flamechart = { version: '1', }; -Object.defineProperty(window, 'matchMedia', { - writable: true, - value: jest.fn().mockImplementation(query => ({ - matches: false, - media: query, - onchange: null, - addListener: jest.fn(), // Deprecated - removeListener: jest.fn(), // Deprecated - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })), -}); - describe('Flamegraph', () => { beforeEach(() => { const project = ProjectFixture({slug: 'foo-project'}); diff --git a/static/app/views/issueList/actions/index.tsx b/static/app/views/issueList/actions/index.tsx index c25a5cc35103f2..139dc77791028c 100644 --- a/static/app/views/issueList/actions/index.tsx +++ b/static/app/views/issueList/actions/index.tsx @@ -179,7 +179,7 @@ function IssueListActions({ const theme = useTheme(); const disableActions = useMedia( - `(max-width: ${isSavedSearchesOpen ? theme.breakpoints.xl : theme.breakpoints.md})` + `(width < ${isSavedSearchesOpen ? theme.breakpoints.xl : theme.breakpoints.md})` ); const numIssues = selectedIdsSet.size; diff --git a/static/app/views/issueList/groupListBody.tsx b/static/app/views/issueList/groupListBody.tsx index d052bfde2accfc..383131188f1c1f 100644 --- a/static/app/views/issueList/groupListBody.tsx +++ b/static/app/views/issueList/groupListBody.tsx @@ -100,8 +100,8 @@ function GroupList({ false ); const topIssue = groupIds[0]; - const canSelect = !useMedia( - `(max-width: ${isSavedSearchesOpen ? theme.breakpoints.xl : theme.breakpoints.md})` + const selectDisabled = useMedia( + `(width < ${isSavedSearchesOpen ? theme.breakpoints.xl : theme.breakpoints.md})` ); const columns: GroupListColumn[] = [ @@ -131,7 +131,7 @@ function GroupList({ memberList={group?.project ? memberList[group.project.slug] : undefined} displayReprocessingLayout={displayReprocessingLayout} useFilteredStats - canSelect={canSelect} + canSelect={!selectDisabled} onPriorityChange={priority => onActionTaken([id], {priority})} withColumns={columns} /> diff --git a/static/app/views/nav/context.tsx b/static/app/views/nav/context.tsx index 82a87f728deeff..024ea4d2c7f6a7 100644 --- a/static/app/views/nav/context.tsx +++ b/static/app/views/nav/context.tsx @@ -56,7 +56,7 @@ export function NavContextProvider({children}: {children: React.ReactNode}) { useState(null); const theme = useTheme(); - const isMobile = useMedia(`(max-width: ${theme.breakpoints.md})`); + const isMobile = useMedia(`(width < ${theme.breakpoints.md})`); const startInteraction = useCallback(() => { isInteractingRef.current = true; diff --git a/static/app/views/nav/index.spec.tsx b/static/app/views/nav/index.spec.tsx index 4ec18c635a81bd..036ab0330a1f23 100644 --- a/static/app/views/nav/index.spec.tsx +++ b/static/app/views/nav/index.spec.tsx @@ -10,6 +10,7 @@ import { waitFor, within, } from 'sentry-test/reactTestingLibrary'; +import {mockMatchMedia} from 'sentry-test/utils'; import ConfigStore from 'sentry/stores/configStore'; import {trackAnalytics} from 'sentry/utils/analytics'; @@ -336,23 +337,12 @@ describe('Nav', () => { }); describe('mobile navigation', () => { - const initialMatchMedia = window.matchMedia; beforeEach(() => { - // Need useMedia() to return true for isMobile query - window.matchMedia = jest.fn().mockImplementation(query => ({ - matches: true, - media: query, - onchange: null, - addListener: jest.fn(), - removeListener: jest.fn(), - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })); + mockMatchMedia(true); }); afterEach(() => { - window.matchMedia = initialMatchMedia; + jest.restoreAllMocks(); }); it('renders mobile navigation on small screen sizes', async () => { diff --git a/static/app/views/profiling/profileSummary/profileSummaryPage.spec.tsx b/static/app/views/profiling/profileSummary/profileSummaryPage.spec.tsx index b495f1fd40acb2..2f9d6faa8a036b 100644 --- a/static/app/views/profiling/profileSummary/profileSummaryPage.spec.tsx +++ b/static/app/views/profiling/profileSummary/profileSummaryPage.spec.tsx @@ -4,24 +4,11 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; import {ProjectFixture} from 'sentry-fixture/project'; import {render, screen} from 'sentry-test/reactTestingLibrary'; +import {mockMatchMedia} from 'sentry-test/utils'; import OrganizationStore from 'sentry/stores/organizationStore'; import ProfileSummaryPage from 'sentry/views/profiling/profileSummary'; -Object.defineProperty(window, 'matchMedia', { - writable: true, - value: jest.fn().mockImplementation(query => ({ - matches: false, - media: query, - onchange: null, - addListener: jest.fn(), // Deprecated - removeListener: jest.fn(), // Deprecated - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })), -}); - window.ResizeObserver = window.ResizeObserver || jest.fn().mockImplementation(() => ({ @@ -31,6 +18,14 @@ window.ResizeObserver = })); describe('ProfileSummaryPage', () => { + beforeEach(() => { + mockMatchMedia(true); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + it('renders new page', async () => { const organization = OrganizationFixture({features: []}); OrganizationStore.onUpdate(organization); diff --git a/tests/js/sentry-test/utils.tsx b/tests/js/sentry-test/utils.tsx index e11444e75708a1..3be7f206153f6b 100644 --- a/tests/js/sentry-test/utils.tsx +++ b/tests/js/sentry-test/utils.tsx @@ -51,3 +51,19 @@ export function setWindowLocation(url: string) { // global jsdom is coming from `@sentry/jest-environment` (global as any).jsdom.reconfigure({url}); } + +/** + * Mocks window.matchMedia to always return the provided `matches`. + */ +export function mockMatchMedia(matches: boolean) { + jest.spyOn(window, 'matchMedia').mockImplementation(() => ({ + matches, + media: '', + onchange: null, + addListener: jest.fn(), + removeListener: jest.fn(), + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + })); +} diff --git a/tests/js/setup.ts b/tests/js/setup.ts index cbd2504ec9c92b..0e62fe895d8b67 100644 --- a/tests/js/setup.ts +++ b/tests/js/setup.ts @@ -238,6 +238,20 @@ Object.defineProperty(window, 'getComputedStyle', { writable: true, }); +Object.defineProperty(window, 'matchMedia', { + writable: true, + value: (query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: jest.fn(), // Deprecated + removeListener: jest.fn(), // Deprecated + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + }), +}); + window.IntersectionObserver = class IntersectionObserver { root = null; rootMargin = '';