Skip to content

ref(feedbackModal): Move inner components to module scope for static definitions #120042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sentry wants to merge 2 commits into
base: master
Choose a base branch
from
+164 −92
Open

ref(feedbackModal): Move inner components to module scope for static definitions #120042

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db3e91c
ref(feedbackModal): Move inner components to module scope for static …
ryan953 Jul 19, 2026
46100bd
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 164 additions & 92 deletions static/app/components/featureFeedback/feedbackModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Fragment, useCallback, useMemo, useState} from 'react';
import {Fragment, useMemo, useState} from 'react';
import {css, useTheme} from '@emotion/react';
import type {Event} from '@sentry/core';
import {
Comment on lines +1 to 4

@sentry sentry Bot Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The useCallback hook is used but not imported from 'react', which will cause a ReferenceError when the component renders.
Severity: CRITICAL

Suggested Fix

Add useCallback to the import statement from 'react' at the top of the file. The import should look like: import {Fragment, useMemo, useState, useCallback} from 'react';.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/components/featureFeedback/feedbackModal.tsx#L1-L4

Potential issue: The `useCallback` hook is used in the `FeedbackModal` component to
define the `handleSubmit` function. However, `useCallback` is not imported from the
'react' library in the import statement at the top of the file. This will cause a
`ReferenceError: useCallback is not defined` at runtime whenever the `FeedbackModal`
component is mounted, which will break the feedback functionality.

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down Expand Up @@ -83,6 +83,113 @@
useNewUserFeedback?: boolean;
};

type FeedbackModalHeaderProps = {
children: React.ReactNode;
header: ModalRenderProps['Header'];
};

function FeedbackModalHeader({children, header: Header}: FeedbackModalHeaderProps) {
return (
<Header closeButton>
<h3>{children}</h3>
</Header>
);
}

type FeedbackModalBodyProps = {
body: ModalRenderProps['Body'];
children: React.ReactNode;
isSelfHosted: boolean;
showSelfHostedMessage?: boolean;
};

function FeedbackModalBody({
children,
body: Body,
isSelfHosted,
showSelfHostedMessage = true,
}: FeedbackModalBodyProps) {
return (
<Body>
{children}
{isSelfHosted && showSelfHostedMessage && (
<Alert.Container>
<Alert variant="info" showIcon={false}>
{tct(
"You agree that any feedback you submit is subject to Sentry's [privacyPolicy:Privacy Policy] and Sentry may use such feedback without restriction or obligation.",
{
privacyPolicy: <ExternalLink href="https://sentry.io/privacy/" />,
}
)}
</Alert>
</Alert.Container>
)}
</Body>
);
}

type FeedbackModalFooterProps = {
closeModal: () => void;
footer: ModalRenderProps['Footer'];
handleSubmit: (submitEventData?: Event) => void;
hasCustomChildren: boolean;
isScreenSmall: boolean;
state: {subject?: any; [key: string]: any};
onBack?: () => void;
onNext?: () => void;
primaryDisabledReason?: string;
secondaryAction?: React.ReactNode;
submitEventData?: Event;
};

function FeedbackModalFooter({
footer: Footer,
closeModal,
handleSubmit,
isScreenSmall,
state,
hasCustomChildren,
onBack,
onNext,
submitEventData,
primaryDisabledReason,
secondaryAction,
}: FeedbackModalFooterProps) {
return (
<Footer>
{secondaryAction && (
<Container flex="1" alignSelf="center">
{secondaryAction}
</Container>
)}
{onBack && (
<Container marginRight="md" width="100%">
<Button onClick={onBack}>{t('Back')}</Button>
</Container>
)}
<Grid flow="column" align="center" gap="md">
<Button onClick={closeModal}>{t('Cancel')}</Button>
<Button
variant="primary"
tooltipProps={{
title: !hasCustomChildren
? defined(state.subject)
? undefined
: t('Required fields must be filled out')
: primaryDisabledReason,
}}
onClick={onNext ?? (() => handleSubmit(submitEventData))}
disabled={
!hasCustomChildren ? !defined(state.subject) : defined(primaryDisabledReason)
}
>
{onNext ? t('Next') : isScreenSmall ? t('Submit') : t('Submit Feedback')}
</Button>
</Grid>
</Footer>
);
}

/**
* A modal that allows users to submit feedback to Sentry (feedbacks project).
*
Expand Down Expand Up @@ -116,7 +223,7 @@
return;
}, [projectsLoaded, projects, location.query.project]);

const handleSubmit = useCallback(

Check failure on line 226 in static/app/components/featureFeedback/feedbackModal.tsx

View workflow job for this annotation

GitHub Actions / typescript 

Cannot find name 'useCallback'.
(submitEventData?: Event) => {
const message = `${props.featureName} feedback by ${user.email}`;

Expand Down Expand Up @@ -194,90 +301,7 @@
]
);

const ModalHeader = useCallback(
({children: headerChildren}: {children: React.ReactNode}) => {
return (
<Header closeButton>
<h3>{headerChildren}</h3>
</Header>
);
},
[Header]
);

const ModalFooter = useCallback(
({
onBack,
onNext,
submitEventData,
primaryDisabledReason,
secondaryAction,
}: Parameters<ChildrenProps<T>['Footer']>[0]) => {
return (
<Footer>
{secondaryAction && (
<Container flex="1" alignSelf="center">
{secondaryAction}
</Container>
)}
{onBack && (
<Container marginRight="md" width="100%">
<Button onClick={onBack}>{t('Back')}</Button>
</Container>
)}
<Grid flow="column" align="center" gap="md">
<Button onClick={closeModal}>{t('Cancel')}</Button>
<Button
variant="primary"
tooltipProps={{
title:
props.children === undefined
? defined(state.subject)
? undefined
: t('Required fields must be filled out')
: primaryDisabledReason,
}}
onClick={onNext ?? (() => handleSubmit(submitEventData))}
disabled={
props.children === undefined
? !defined(state.subject)
: defined(primaryDisabledReason)
}
>
{onNext ? t('Next') : isScreenSmall ? t('Submit') : t('Submit Feedback')}
</Button>
</Grid>
</Footer>
);
},
[Footer, isScreenSmall, closeModal, handleSubmit, state, props.children]
);

const ModalBody = useCallback(
({
children: bodyChildren,
showSelfHostedMessage = true,
}: Parameters<ChildrenProps<T>['Body']>[0]) => {
return (
<Body>
{bodyChildren}
{isSelfHosted && showSelfHostedMessage && (
<Alert.Container>
<Alert variant="info" showIcon={false}>
{tct(
"You agree that any feedback you submit is subject to Sentry's [privacyPolicy:Privacy Policy] and Sentry may use such feedback without restriction or obligation.",
{
privacyPolicy: <ExternalLink href="https://sentry.io/privacy/" />,
}
)}
</Alert>
</Alert.Container>
)}
</Body>
);
},
[Body, isSelfHosted]
);
const hasCustomChildren = props.children !== undefined;

function handleFieldChange<Field extends keyof T>(field: Field, value: T[Field]) {
const newState = cloneDeep(state);
Expand All @@ -290,8 +314,8 @@

return (
<Fragment>
<ModalHeader>{t('Submit Feedback')}</ModalHeader>
<ModalBody>
<FeedbackModalHeader header={Header}>{t('Submit Feedback')}</FeedbackModalHeader>
<FeedbackModalBody body={Body} isSelfHosted={isSelfHosted}>
<SelectField
label={t('Type of feedback')}
name="subject"
Expand Down Expand Up @@ -328,18 +352,66 @@
}
/>
</FieldGroup>
</ModalBody>
<ModalFooter secondaryAction={props?.secondaryAction} />
</FeedbackModalBody>
<FeedbackModalFooter
footer={Footer}
closeModal={closeModal}
handleSubmit={handleSubmit}
isScreenSmall={isScreenSmall}
state={state}
hasCustomChildren={hasCustomChildren}
secondaryAction={props?.secondaryAction}
/>
</Fragment>
);
}

// For custom children, create bound wrappers that match the ChildrenProps API.
// These are passed as function values (not used as JSX element types here),
// so they do not trigger the static-component-definitions convention.
const boundHeader: ChildrenProps<T>['Header'] = ({children: headerChildren}) => (
<FeedbackModalHeader header={Header}>{headerChildren}</FeedbackModalHeader>
);
const boundBody: ChildrenProps<T>['Body'] = ({
children: bodyChildren,
showSelfHostedMessage,
}) => (
<FeedbackModalBody
body={Body}
isSelfHosted={isSelfHosted}
showSelfHostedMessage={showSelfHostedMessage}
>
{bodyChildren}
</FeedbackModalBody>
);

@cursor cursor Bot Jul 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom modal body remounts

Medium Severity

For custom children, boundHeader and boundBody are new function references on every FeedbackModal render. Consumers render them as JSX components (for example Body), so React treats the type as changed whenever modal state updates and remounts those subtrees. That regresses the prior useCallback behavior where header and body stayed stable across state changes, and can drop focus while typing in nested fields.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit db3e91c. Configure here.

const boundFooter: ChildrenProps<T>['Footer'] = ({
onBack,
onNext,
submitEventData,
primaryDisabledReason,
secondaryAction,
}) => (
<FeedbackModalFooter
footer={Footer}
closeModal={closeModal}
handleSubmit={handleSubmit}
isScreenSmall={isScreenSmall}
state={state}
hasCustomChildren={hasCustomChildren}
onBack={onBack}
onNext={onNext}
submitEventData={submitEventData}
primaryDisabledReason={primaryDisabledReason}
secondaryAction={secondaryAction}
/>
);

return (
<Fragment>
{props.children({
Header: ModalHeader,
Body: ModalBody,
Footer: ModalFooter,
Header: boundHeader,
Body: boundBody,
Footer: boundFooter,
onFieldChange: handleFieldChange,
state,
})}
Expand Down
Loading