Skip to content

Commit 233ccd6

Browse files
authored
fix(useFeedbackForm): Cleanup after form is closed or submitted (#74163)
Without these changes, the form after being closed still prevented the page from scrolling. I'm not sure if this is intended or is a bug with the SDK, but I was able to fix this by removing from the DOM when the form was closed or submitted.
1 parent 168eb3b commit 233ccd6

File tree

2 files changed

+25
-12
lines changed

2 files changed

+25
-12
lines changed

static/app/utils/useFeedbackForm.spec.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {GlobalFeedbackForm, useFeedbackForm} from 'sentry/utils/useFeedbackForm'
77
const mockForm = {
88
appendToDom: jest.fn(),
99
open: jest.fn(),
10+
close: jest.fn(),
1011
removeFromDom: jest.fn(),
1112
};
1213

@@ -28,6 +29,7 @@ describe('useFeedbackForm', function () {
2829
jest
2930
.spyOn(useFeedback, 'useFeedback')
3031
.mockReturnValue({feedback: mockFeedback, options: defaultOptions});
32+
jest.clearAllMocks();
3133
});
3234

3335
it('can open the form using useFeedbackForm', async function () {

static/app/utils/useFeedbackForm.tsx

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,33 +48,44 @@ function useOpenForm() {
4848
const formRef = useRef<FeedbackDialog | null>(null);
4949
const {feedback, options} = useFeedback({});
5050

51+
const cleanup = useCallback(() => {
52+
try {
53+
if (formRef.current) {
54+
formRef.current.close();
55+
formRef.current.removeFromDom();
56+
formRef.current = null;
57+
}
58+
} catch {
59+
// If the form is already removed from the DOM, then we can ignore this error
60+
}
61+
}, []);
62+
5163
const openForm = useCallback(
5264
async (optionOverrides?: UseFeedbackOptions) => {
5365
if (!feedback) {
5466
return;
5567
}
5668

5769
if (formRef.current) {
58-
formRef.current.removeFromDom();
70+
cleanup();
5971
}
6072

61-
formRef.current = await feedback.createForm({...options, ...optionOverrides});
73+
formRef.current = await feedback.createForm({
74+
...options,
75+
...optionOverrides,
76+
onFormClose: cleanup,
77+
onFormSubmitted: cleanup,
78+
});
79+
6280
formRef.current.appendToDom();
6381
formRef.current.open();
6482
},
65-
[feedback, options]
83+
[cleanup, feedback, options]
6684
);
6785

68-
// Cleanup the leftover form element when the component unmounts
6986
useEffect(() => {
70-
return () => {
71-
if (!formRef.current) {
72-
return;
73-
}
74-
75-
formRef.current.removeFromDom();
76-
};
77-
}, []);
87+
return cleanup;
88+
}, [cleanup]);
7889

7990
return feedback ? openForm : null;
8091
}

0 commit comments

Comments
 (0)