feat(feedback): Make "required" text for input elements configurable

[soerface]

Fix #11152

This introduces an option isRequiredLabel, and I planned to also add errorMessageText.

However, this PR is branched off from a version that is a couple days old. Right now, @c298lee is currently working on getsentry/sentry#63749, which is causing major changes and the current version on master doesn't work yet. Therefore, I didn't yet implement errorMessageText.

So consider this PR as a PoC, and either feel free to tag me when the screenshot changes are done – then I'll redo the changes based on the version that supports screenshots – or add the option on your own; however you prefer 🙂

One open question:

Until now, there was only the error message:

There was a problem submitting feedback, please wait and try again.

Now, depending on the status code, we have three error messages:

• 'Unable to send Feedback. This is because of network issues, or because you are using an ad-blocker.'
• 'Unable to send Feedback. Invalid response from server.'
• 'Unable to send Feedback'

Do you have a suggestion how we could make the message configurable, without introducing too many and redundant settings? Maybe we should go back to only one message? I'm not sure if an end-user cares about whether it is a network issue or a server error.

[billyvg]

Thanks for the PR @soerface! We could potentially reduce it to two -- we definitely want the notice about ad blockers as that is something useful to the end user. Having specific error messages helps us debug too. I think we should have 3 configurable options for the different error messages: "client error", "server error", "unknown error"

[soerface]

Thanks for your feedback @billyvg! I've brought the branch up to date with master, and added three configurable messages as requested:

• For client side: 'Unable to send Feedback. This is because of network issues, or because you are using an ad-blocker.'
• For server side: 'Unable to send Feedback. Please try again later.'
• Else: 'Unable to send Feedback.'

The current version is difficult for me to test, since the version on master still throws Error: Async loading of Feedback Modal & Screenshot integrations is not yet implemented, and since the refactoring the amount of tests in here is also reduced. The existing tests are green though.

How do you feel about the three extra parameters to sendFeedback, to be able to configure the texts for the network errors? Is this fine, or should it rather be encapsulated in some separate _options parameter or something like that?

[billyvg]

@soerface Ah right, I was not considering sendFeedback - I think we will need to leave it as-is because it will be moved into core at some point and it's also a bit awkward to treat the exception messages as UI and have it be configurable.

what do you think about removing the error text changes for now and we can get the required text merged in first as that is more straight forward?

[soerface]

@billyvg sure! I've simplified the PR, merely making (required) configurable now.

[soerface]

I've also created a PR in the documentation repository. Regarding this:

treat the exception messages as UI and have it be configurable

Should we continue a discussion about that in the issue? I do think it is valuable to have it configurable. Because, as you already mentioned:

want the notice about ad blockers as that is something useful to the end user.

Therefore, as a site-owner, I also want to make sure that this message about ad-blockers can be read by my users in their native language. But maybe we also don't need a config option for this, if the SDK / "core" can handle translating user-facing error messages.

[billyvg]

Looks like it's failing a type check:

Error: src/modal/createDialog.tsx(52,8): error TS2322: Type '{ screenshotInput: ScreenshotInput | undefined; colorScheme: "system" | "light" | "dark"; showBranding: boolean; showName: boolean; showEmail: boolean; isNameRequired: boolean; isEmailRequired: boolean; formTitle: string; cancelButtonLabel: string; submitButtonLabel: string; emailLabel: string; emailPlaceholder: string; messageLabel: string; messagePlaceholder: string; nameLabel: string; namePlaceholder: string; defaultName: any; defaultEmail: any; successMessageText: string; onFormClose: () => void; onSubmit: ({ name, email, message, attachments, source, url }: SendFeedbackParams, { includeReplay }?: SendFeedbackOptions) => Promise<TransportMakeRequestResponse>; onSubmitSuccess: (data: FeedbackFormData) => void; onSubmitError: (error: Error) => void; onFormSubmitted: () => void; open: boolean; }' is not assignable to type 'IntrinsicAttributes & Props'.
  Property 'isRequiredText' is missing in type '{ screenshotInput: ScreenshotInput | undefined; colorScheme: "system" | "light" | "dark"; showBranding: boolean; showName: boolean; showEmail: boolean; isNameRequired: boolean; isEmailRequired: boolean; formTitle: string; cancelButtonLabel: string; submitButtonLabel: string; emailLabel: string; emailPlaceholder: string; messageLabel: string; messagePlaceholder: string; nameLabel: string; namePlaceholder: string; defaultName: any; defaultEmail: any; successMessageText: string; onFormClose: () => void; onSubmit: ({ name, email, message, attachments, source, url }: SendFeedbackParams, { includeReplay }?: SendFeedbackOptions) => Promise<TransportMakeRequestResponse>; onSubmitSuccess: (data: FeedbackFormData) => void; onSubmitError: (error: Error) => void; onFormSubmitted: () => void; open: boolean; }' but required in type 'Props'.