Skip to content

Commit 6e11363

Browse files
committed
feat(ui): Tweak GlobalDrawer useEffect for closing
`currentDrawerConfig` undefined means the drawer should already be closed, no need to call `closeDrawer` (right?). We still want to maintain the defaut close behavior if `shouldCloseOnLocation` is not defined.
1 parent 543b503 commit 6e11363

File tree

2 files changed

+39
-37
lines changed

2 files changed

+39
-37
lines changed

static/app/components/globalDrawer/index.tsx

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ export function GlobalDrawer({children}: any) {
110110
const [currentDrawerConfig, overwriteDrawerConfig] = useState<
111111
DrawerConfig | undefined
112112
>();
113+
const currentDrawerConfigRef = useRef(currentDrawerConfig);
113114
// If no config is set, the global drawer is closed.
114115
const isDrawerOpen = !!currentDrawerConfig;
115116
const openDrawer = useCallback<DrawerContextType['openDrawer']>((renderer, options) => {
@@ -126,28 +127,27 @@ export function GlobalDrawer({children}: any) {
126127
closeDrawer();
127128
}, [currentDrawerConfig, closeDrawer]);
128129

130+
currentDrawerConfigRef.current = currentDrawerConfig;
131+
129132
// Close the drawer when the browser history changes.
130133
useLayoutEffect(
131134
() => {
132-
// Defaults to closing the drawer when the location changes
133135
if (
134-
// Drawer should be closed already if `currentDrawerConfig` is undefined
135-
currentDrawerConfig !== undefined &&
136-
(currentDrawerConfig?.options.shouldCloseOnLocationChange?.(location) ?? true)
136+
// No need to close drawer if it is not open
137+
currentDrawerConfigRef.current !== undefined &&
138+
// Otherwise, when the location changes, check callback or default to closing the drawer if it doesn't exist
139+
(currentDrawerConfigRef.current.options?.shouldCloseOnLocationChange?.(
140+
location
141+
) ??
142+
true)
137143
) {
138144
// Call `closeDrawer` without invoking `onClose` callback, since those callbacks often update the URL
139145
closeDrawer();
140146
}
141147
},
142-
// Ignoring changes to currentDrawerConfig?.options to prevent closing the drawer when it opens
143-
// eslint-disable-next-line react-hooks/exhaustive-deps
144-
[
145-
location?.pathname,
146-
location?.search,
147-
location?.hash,
148-
closeDrawer,
149-
currentDrawerConfig?.options.shouldCloseOnLocationChange,
150-
]
148+
// Ignoring changes to currentDrawerConfig and currentDrawerConfig?.options
149+
// to prevent closing the drawer when it opens.
150+
[closeDrawer, location]
151151
);
152152

153153
// Close the drawer when clicking outside the panel and options allow it.

static/app/views/releases/drawer/useReleasesDrawer.tsx

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useEffect, useRef} from 'react';
1+
import {useEffect, useMemo, useRef} from 'react';
22
import omit from 'lodash/omit';
33

44
import useDrawer from 'sentry/components/globalDrawer';
@@ -51,32 +51,34 @@ export function useReleasesDrawer() {
5151
}
5252
}, [closeDrawer, showReleasesDrawer]);
5353

54+
const openDrawerFn = useMemo(
55+
() =>
56+
function () {
57+
return <ReleasesDrawerDetails release={release} projectId={releaseProjectId} />;
58+
},
59+
[release, releaseProjectId]
60+
);
61+
62+
const openDrawerOptions = useMemo(
63+
() => ({
64+
shouldCloseOnLocationChange: () => {
65+
return false;
66+
},
67+
ariaLabel: t('Releases drawer'),
68+
transitionProps: {stiffness: 1000},
69+
onClose: () => {
70+
navigate({
71+
query: cleanLocationQuery(location.query),
72+
});
73+
},
74+
}),
75+
[location.query, navigate]
76+
);
5477
useEffect(() => {
5578
if (showReleasesDrawer === '1' && release) {
56-
openDrawer(
57-
() => <ReleasesDrawerDetails release={release} projectId={releaseProjectId} />,
58-
{
59-
shouldCloseOnLocationChange: () => {
60-
return false;
61-
},
62-
ariaLabel: t('Releases drawer'),
63-
transitionProps: {stiffness: 1000},
64-
onClose: () => {
65-
navigate({
66-
query: cleanLocationQuery(location.query),
67-
});
68-
},
69-
}
70-
);
79+
openDrawer(openDrawerFn, openDrawerOptions);
7180
}
72-
}, [
73-
location.query,
74-
navigate,
75-
openDrawer,
76-
release,
77-
releaseProjectId,
78-
showReleasesDrawer,
79-
]);
81+
}, [openDrawer, openDrawerFn, openDrawerOptions, release, showReleasesDrawer]);
8082

8183
useEffect(() => {
8284
if (rdChart) {

0 commit comments

Comments
 (0)