Merge pull request #9587 from getsentry/feedback-name-and-email-required

c298lee · web-flow · commit aed4bf0a06c6 · 2023-11-20T10:35:58.000-05:00
feat(feedback): Option to make name and email required
diff --git a/packages/feedback/src/widget/Dialog.ts b/packages/feedback/src/widget/Dialog.ts
@@ -45,6 +45,8 @@ export function Dialog({
   showBranding,
   showName,
   showEmail,
+  isNameRequired,
+  isEmailRequired,
   colorScheme,
   isAnonymous,
   defaultName,
@@ -102,6 +104,8 @@ export function Dialog({
     showEmail,
     showName,
     isAnonymous,
+    isEmailRequired,
+    isNameRequired,
 
     defaultName,
     defaultEmail,
diff --git a/packages/feedback/src/widget/Form.ts b/packages/feedback/src/widget/Form.ts
@@ -8,6 +8,8 @@ export interface FormComponentProps
     | 'showName'
     | 'showEmail'
     | 'isAnonymous'
+    | 'isNameRequired'
+    | 'isEmailRequired'
     | Exclude<keyof FeedbackTextConfiguration, 'buttonLabel' | 'formTitle' | 'successMessageText'>
   > {
   /**
@@ -58,6 +60,8 @@ export function Form({
   showName,
   showEmail,
   isAnonymous,
+  isNameRequired,
+  isEmailRequired,
 
   defaultName,
   defaultEmail,
@@ -113,6 +117,7 @@ export function Form({
     type: showName ? 'text' : 'hidden',
     ['aria-hidden']: showName ? 'false' : 'true',
     name: 'name',
+    required: isNameRequired,
     className: 'form__input',
     placeholder: namePlaceholder,
     value: defaultName,
@@ -123,6 +128,7 @@ export function Form({
     type: showEmail ? 'text' : 'hidden',
     ['aria-hidden']: showEmail ? 'false' : 'true',
     name: 'email',
+    required: isEmailRequired,
     className: 'form__input',
     placeholder: emailPlaceholder,
     value: defaultEmail,
@@ -168,7 +174,15 @@ export function Form({
             htmlFor: 'name',
             className: 'form__label',
           },
-          [nameLabel, nameEl],
+          [
+            createElement(
+              'span',
+              { className: 'form__label__text' },
+              nameLabel,
+              isNameRequired && createElement('span', { className: 'form__label__text--required' }, ' (required)'),
+            ),
+            nameEl,
+          ],
         ),
       !isAnonymous && !showName && nameEl,
 
@@ -180,7 +194,15 @@ export function Form({
             htmlFor: 'email',
             className: 'form__label',
           },
-          [emailLabel, emailEl],
+          [
+            createElement(
+              'span',
+              { className: 'form__label__text' },
+              emailLabel,
+              isEmailRequired && createElement('span', { className: 'form__label__text--required' }, ' (required)'),
+            ),
+            emailEl,
+          ],
         ),
       !isAnonymous && !showEmail && emailEl,
 
diff --git a/packages/feedback/src/widget/createWidget.ts b/packages/feedback/src/widget/createWidget.ts
@@ -88,9 +88,19 @@ export function createWidget({
       return;
     }
 
-    // Simple validation for now, just check for non-empty message
+    // Simple validation for now, just check for non-empty required fields
+    const emptyField = [];
+    if (options.isNameRequired && !feedback.name) {
+      emptyField.push(options.nameLabel);
+    }
+    if (options.isEmailRequired && !feedback.email) {
+      emptyField.push(options.emailLabel);
+    }
     if (!feedback.message) {
-      dialog.showError('Please enter in some feedback before submitting!');
+      emptyField.push(options.messageLabel);
+    }
+    if (emptyField.length != 0) {
+      dialog.showError(`Please enter in the following required fields: ${emptyField.join(', ')}`);
       return;
     }
 
@@ -156,9 +166,11 @@ export function createWidget({
       dialog = Dialog({
         colorScheme: options.colorScheme,
         showBranding: options.showBranding,
-        showName: options.showName,
-        showEmail: options.showEmail,
+        showName: options.showName || options.isNameRequired,
+        showEmail: options.showEmail || options.isEmailRequired,
         isAnonymous: options.isAnonymous,
+        isNameRequired: options.isNameRequired,
+        isEmailRequired: options.isEmailRequired,
         formTitle: options.formTitle,
         cancelButtonLabel: options.cancelButtonLabel,
         submitButtonLabel: options.submitButtonLabel,
diff --git a/packages/feedback/test/widget/Dialog.test.ts b/packages/feedback/test/widget/Dialog.test.ts
@@ -10,6 +10,8 @@ function renderDialog({
   showEmail = true,
   showBranding = false,
   isAnonymous = false,
+  isNameRequired = false,
+  isEmailRequired = false,
   formTitle = 'Feedback',
   defaultName = 'Foo Bar',
   defaultEmail = 'foo@example.com',
@@ -30,6 +32,8 @@ function renderDialog({
     isAnonymous,
     showName,
     showEmail,
+    isNameRequired,
+    isEmailRequired,
     showBranding,
     defaultName,
     defaultEmail,
diff --git a/packages/feedback/test/widget/Form.test.ts b/packages/feedback/test/widget/Form.test.ts
@@ -9,6 +9,8 @@ function renderForm({
   showName = true,
   showEmail = true,
   isAnonymous = false,
+  isNameRequired = false,
+  isEmailRequired = false,
   defaultName = 'Foo Bar',
   defaultEmail = 'foo@example.com',
   nameLabel = 'Name',
@@ -25,6 +27,8 @@ function renderForm({
     isAnonymous,
     showName,
     showEmail,
+    isNameRequired,
+    isEmailRequired,
     defaultName,
     defaultEmail,
     nameLabel,
@@ -80,13 +84,15 @@ describe('Form', () => {
       emailPlaceholder: 'foo@example.org!',
       messageLabel: 'Description!',
       messagePlaceholder: 'What is the issue?!',
+      isNameRequired: true,
+      isEmailRequired: true,
     });
 
     const nameLabel = formComponent.el.querySelector('label[htmlFor="name"]') as HTMLLabelElement;
     const emailLabel = formComponent.el.querySelector('label[htmlFor="email"]') as HTMLLabelElement;
     const messageLabel = formComponent.el.querySelector('label[htmlFor="message"]') as HTMLLabelElement;
-    expect(nameLabel.textContent).toBe('Name!');
-    expect(emailLabel.textContent).toBe('Email!');
+    expect(nameLabel.textContent).toBe('Name! (required)');
+    expect(emailLabel.textContent).toBe('Email! (required)');
     expect(messageLabel.textContent).toBe('Description! (required)');
 
     const nameInput = formComponent.el.querySelector('[name="name"]') as HTMLInputElement;
@@ -98,18 +104,6 @@ describe('Form', () => {
     expect(messageInput.placeholder).toBe('What is the issue?!');
   });
 
-  it('submit is enabled if message is not empty', () => {
-    const formComponent = renderForm();
-
-    const message = formComponent.el.querySelector('[name="message"]') as HTMLTextAreaElement;
-
-    message.value = 'Foo (message)';
-    message.dispatchEvent(new KeyboardEvent('keyup'));
-
-    message.value = '';
-    message.dispatchEvent(new KeyboardEvent('keyup'));
-  });
-
   it('can show error', () => {
     const formComponent = renderForm();
     const errorEl = formComponent.el.querySelector('.form__error-container') as HTMLDivElement;
@@ -148,6 +142,8 @@ describe('Form', () => {
   it('does not show name or email inputs for anonymous mode', () => {
     const onSubmit = jest.fn();
     const formComponent = renderForm({
+      isNameRequired: true,
+      isEmailRequired: true,
       isAnonymous: true,
       onSubmit,
     });
diff --git a/packages/feedback/test/widget/createWidget.test.ts b/packages/feedback/test/widget/createWidget.test.ts
@@ -171,6 +171,72 @@ describe('createWidget', () => {
     expect(shadow.querySelector('.success-message')).toBeNull();
   });
 
+  it('only submits feedback successfully when all required fields are filled', async () => {
+    const onSubmitSuccess = jest.fn(() => {});
+    const { shadow, widget } = createShadowAndWidget({
+      isNameRequired: true,
+      isEmailRequired: true,
+      onSubmitSuccess,
+    });
+
+    (sendFeedbackRequest as jest.Mock).mockImplementation(() => {
+      return true;
+    });
+    widget.actor?.el?.dispatchEvent(new Event('click'));
+
+    const nameEl = widget.dialog?.el?.querySelector('[name="name"]') as HTMLInputElement;
+    const emailEl = widget.dialog?.el?.querySelector('[name="email"]') as HTMLInputElement;
+    const messageEl = widget.dialog?.el?.querySelector('[name="message"]') as HTMLTextAreaElement;
+
+    nameEl.value = '';
+    emailEl.value = '';
+    messageEl.value = '';
+
+    widget.dialog?.el?.querySelector('form')?.dispatchEvent(new Event('submit'));
+    expect(sendFeedbackRequest).toHaveBeenCalledTimes(0);
+
+    // sendFeedbackRequest is async
+    await flushPromises();
+    expect(onSubmitSuccess).toHaveBeenCalledTimes(0);
+
+    nameEl.value = '';
+    emailEl.value = '';
+    messageEl.value = 'My feedback';
+
+    widget.dialog?.el?.querySelector('form')?.dispatchEvent(new Event('submit'));
+    expect(sendFeedbackRequest).toHaveBeenCalledTimes(0);
+
+    // sendFeedbackRequest is async
+    await flushPromises();
+    expect(onSubmitSuccess).toHaveBeenCalledTimes(0);
+
+    nameEl.value = 'Jane Doe';
+    emailEl.value = 'jane@example.com';
+    messageEl.value = 'My feedback';
+
+    widget.dialog?.el?.querySelector('form')?.dispatchEvent(new Event('submit'));
+    expect(sendFeedbackRequest).toHaveBeenCalledWith({
+      feedback: {
+        name: 'Jane Doe',
+        email: 'jane@example.com',
+        message: 'My feedback',
+        url: 'http://localhost/',
+        replay_id: undefined,
+        source: 'widget',
+      },
+    });
+
+    // sendFeedbackRequest is async
+    await flushPromises();
+    expect(onSubmitSuccess).toHaveBeenCalledTimes(1);
+
+    expect(widget.dialog).toBeUndefined();
+    expect(shadow.querySelector('.success-message')?.textContent).toBe(SUCCESS_MESSAGE_TEXT);
+
+    jest.runAllTimers();
+    expect(shadow.querySelector('.success-message')).toBeNull();
+  });
+
   it('submits feedback with error on request', async () => {
     const onSubmitError = jest.fn(() => {});
     const { shadow, widget } = createShadowAndWidget({
