Skip to content

feat(feedback): Use only image uri in the onAddScreenshot callback #4546

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.

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

Merged
merged 20 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
38a6085
Save form state for unsubmitted data
antonis Feb 14, 2025
56a113e
Show selected screenshot
antonis Feb 14, 2025
d2dd1f3
Use image uri instead of UInt8Array in onAddScreenshot callback
antonis Feb 14, 2025
9212cb8
Merge branch 'feedback-ui' into antonis/feedback-save-state
antonis Feb 17, 2025
1efb23e
Omit isVisible from state
antonis Feb 17, 2025
59f2f57
Save/clear form state on unmount
antonis Feb 17, 2025
797611f
Pass the missing attachment parameter in the onSubmitSuccess
antonis Feb 17, 2025
dbf8c90
Merge branch 'antonis/feedback-save-state' into antonis/feedback-show…
antonis Feb 17, 2025
91117e3
Merge branch 'antonis/feedback-show-screenshot' into antonis/feedback…
antonis Feb 17, 2025
f14197d
Use only the uri parameter for the onAddScreenshot callback
antonis Feb 17, 2025
e63ee11
Merge branch 'feedback-ui' into antonis/feedback-save-state
antonis Feb 17, 2025
f220540
Merge branch 'antonis/feedback-save-state' into antonis/feedback-show…
antonis Feb 17, 2025
aae3a98
Merge branch 'antonis/feedback-show-screenshot' into antonis/feedback…
antonis Feb 17, 2025
f17106e
Use instance variable for _didSubmitForm
antonis Feb 18, 2025
f95ed97
Fixed callback function parameter name for clarity
antonis Feb 18, 2025
869f22b
Merge branch 'antonis/feedback-save-state' into antonis/feedback-show…
antonis Feb 18, 2025
6e2d6ec
Fixes lint issue
antonis Feb 18, 2025
082ef57
Merge branch 'antonis/feedback-save-state' into antonis/feedback-show…
antonis Feb 18, 2025
e5a9bcc
Merge branch 'antonis/feedback-show-screenshot' into antonis/feedback…
antonis Feb 18, 2025
6ca97ad
Merge branch 'feedback-ui' into antonis/feedback-simplify-onaddscreen…
antonis Feb 18, 2025
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
14 changes: 13 additions & 1 deletion packages/core/src/js/feedback/FeedbackForm.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,20 @@ const defaultStyles: FeedbackFormStyles = {
backgroundColor: '#eee',
padding: 15,
borderRadius: 5,
marginBottom: 20,
alignItems: 'center',
flex: 1,
},
screenshotContainer: {
flexDirection: 'row',
alignItems: 'center',
width: '100%',
marginBottom: 20,
},
screenshotThumbnail: {
width: 50,
height: 50,
borderRadius: 5,
marginRight: 10,
},
screenshotText: {
color: '#333',
Expand Down
81 changes: 64 additions & 17 deletions packages/core/src/js/feedback/FeedbackForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
...defaultConfiguration
}

private static _savedState: FeedbackFormState = {
isVisible: false,
name: '',
email: '',
description: '',
filename: undefined,
attachment: undefined,
attachmentUri: undefined,
};

public constructor(props: FeedbackFormProps) {
super(props);

Expand All @@ -45,9 +55,12 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor

this.state = {
isVisible: true,
name: currentUser.useSentryUser.name,
email: currentUser.useSentryUser.email,
description: '',
name: FeedbackForm._savedState.name || currentUser.useSentryUser.name,
email: FeedbackForm._savedState.email || currentUser.useSentryUser.email,
description: FeedbackForm._savedState.description || '',
filename: FeedbackForm._savedState.filename || undefined,
attachment: FeedbackForm._savedState.attachment || undefined,
attachmentUri: FeedbackForm._savedState.attachmentUri || undefined,
};
}

Expand Down Expand Up @@ -93,6 +106,7 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
onSubmitSuccess({ name: trimmedName, email: trimmedEmail, message: trimmedDescription, attachments: undefined });
Alert.alert(text.successMessageText);
onFormSubmitted();
this._clearFormState();
} catch (error) {
const errorString = `Feedback form submission failed: ${error}`;
onSubmitError(new Error(errorString));
Expand Down Expand Up @@ -129,7 +143,7 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
const imageUri = result.assets[0].uri;
NATIVE.getDataFromUri(imageUri).then((data) => {
if (data != null) {
this.setState({ filename, attachment: data });
this.setState({ filename, attachment: data, attachmentUri: imageUri }, this._saveFormState);
} else {
logger.error('Failed to read image data from uri:', imageUri);
}
Expand All @@ -141,12 +155,21 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
} else {
// Defaulting to the onAddScreenshot callback
const { onAddScreenshot } = { ...defaultConfiguration, ...this.props };
onAddScreenshot((filename: string, attachement: Uint8Array) => {
this.setState({ filename, attachment: attachement });
onAddScreenshot((filename: string, fileUri: string) => {
NATIVE.getDataFromUri(fileUri).then((data) => {
if (data != null) {
this.setState({ filename, attachment: data, attachmentUri: fileUri }, this._saveFormState);
} else {
logger.error('Failed to read image data from uri:', fileUri);
}
})
.catch((error) => {
logger.error('Failed to read image data from uri:', fileUri, 'error: ', error);
});
});
}
} else {
this.setState({ filename: undefined, attachment: undefined });
this.setState({ filename: undefined, attachment: undefined, attachmentUri: undefined }, this._saveFormState);
}
}

Expand Down Expand Up @@ -199,7 +222,7 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
style={styles.input}
placeholder={text.namePlaceholder}
value={name}
onChangeText={(value) => this.setState({ name: value })}
onChangeText={(value) => this.setState({ name: value }, this._saveFormState)}
/>
</>
)}
Expand All @@ -215,7 +238,7 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
placeholder={text.emailPlaceholder}
keyboardType={'email-address' as KeyboardTypeOptions}
value={email}
onChangeText={(value) => this.setState({ email: value })}
onChangeText={(value) => this.setState({ email: value }, this._saveFormState)}
/>
</>
)}
Expand All @@ -228,17 +251,25 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
style={[styles.input, styles.textArea]}
placeholder={text.messagePlaceholder}
value={description}
onChangeText={(value) => this.setState({ description: value })}
onChangeText={(value) => this.setState({ description: value }, this._saveFormState)}
multiline
/>
{(config.enableScreenshot || imagePickerConfiguration.imagePicker) && (
<TouchableOpacity style={styles.screenshotButton} onPress={this.onScreenshotButtonPress}>
<Text style={styles.screenshotText}>
{!this.state.filename && !this.state.attachment
? text.addScreenshotButtonLabel
: text.removeScreenshotButtonLabel}
</Text>
</TouchableOpacity>
<View style={styles.screenshotContainer}>
{this.state.attachmentUri && (
<Image
source={{ uri: this.state.attachmentUri }}
style={styles.screenshotThumbnail}
/>
)}
<TouchableOpacity style={styles.screenshotButton} onPress={this.onScreenshotButtonPress}>
<Text style={styles.screenshotText}>
{!this.state.filename && !this.state.attachment
? text.addScreenshotButtonLabel
: text.removeScreenshotButtonLabel}
</Text>
</TouchableOpacity>
</View>
)}
<TouchableOpacity style={styles.submitButton} onPress={this.handleFeedbackSubmit}>
<Text style={styles.submitText}>{text.submitButtonLabel}</Text>
Expand All @@ -254,4 +285,20 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
</SafeAreaView>
);
}

private _saveFormState = (): void => {
FeedbackForm._savedState = { ...this.state };
};

private _clearFormState = (): void => {
FeedbackForm._savedState = {
isVisible: false,
name: '',
email: '',
description: '',
filename: undefined,
attachment: undefined,
attachmentUri: undefined,
};
};
}
5 changes: 4 additions & 1 deletion packages/core/src/js/feedback/FeedbackForm.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export interface FeedbackCallbacks {
/**
* Callback when a screenshot is added
*/
onAddScreenshot?: (attachFile: (filename: string, data: Uint8Array) => void) => void;
onAddScreenshot?: (attachFile: (filename: string, fileUri: string) => void) => void;

/**
* Callback when feedback is successfully submitted
Expand Down Expand Up @@ -237,6 +237,8 @@ export interface FeedbackFormStyles {
cancelButton?: ViewStyle;
cancelText?: TextStyle;
screenshotButton?: ViewStyle;
screenshotContainer?: ViewStyle;
screenshotThumbnail?: ImageStyle;
screenshotText?: TextStyle;
titleContainer?: ViewStyle;
sentryLogo?: ImageStyle;
Expand All @@ -252,4 +254,5 @@ export interface FeedbackFormState {
description: string;
filename?: string;
attachment?: string | Uint8Array;
attachmentUri?: string;
}
2 changes: 1 addition & 1 deletion packages/core/src/js/feedback/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const defaultConfiguration: Partial<FeedbackFormProps> = {
);
}
},
onAddScreenshot: (_: (filename: string, data: Uint8Array) => void) => {
onAddScreenshot: (_: (filename: string, fileUri: string) => void) => {
if (__DEV__) {
Alert.alert('Development note', 'onAddScreenshot callback is not implemented.');
}
Expand Down
30 changes: 30 additions & 0 deletions packages/core/test/feedback/FeedbackForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -354,4 +354,34 @@ describe('FeedbackForm', () => {

expect(mockOnFormClose).toHaveBeenCalled();
});

it('onCancel the input is saved and restored when the form reopens', async () => {
const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);

fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), '[email protected]');
fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');

fireEvent.press(getByText(defaultProps.cancelButtonLabel));
const { queryByPlaceholderText } = render(<FeedbackForm {...defaultProps} />);

expect(queryByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe('John Doe');
expect(queryByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe('[email protected]');
expect(queryByPlaceholderText(defaultProps.messagePlaceholder).props.value).toBe('This is a feedback message.');
});

it('onSubmit the saved input is cleared and not restored when the form reopens', async () => {
const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);

fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), '[email protected]');
fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');

fireEvent.press(getByText(defaultProps.submitButtonLabel));
const { queryByPlaceholderText } = render(<FeedbackForm {...defaultProps} />);

expect(queryByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe('Test User');
expect(queryByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe('[email protected]');
expect(queryByPlaceholderText(defaultProps.messagePlaceholder).props.value).toBe('');
});
});
Loading
Loading