Skip to content

feat(feedback): Option to make name and email required #9587

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 7 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions packages/feedback/src/widget/Dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export function Dialog({
showBranding,
showName,
showEmail,
isNameRequired,
isEmailRequired,
colorScheme,
isAnonymous,
defaultName,
Expand Down Expand Up @@ -102,6 +104,8 @@ export function Dialog({
showEmail,
showName,
isAnonymous,
isEmailRequired,
isNameRequired,

defaultName,
defaultEmail,
Expand Down
26 changes: 24 additions & 2 deletions packages/feedback/src/widget/Form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ export interface FormComponentProps
| 'showName'
| 'showEmail'
| 'isAnonymous'
| 'isNameRequired'
| 'isEmailRequired'
| Exclude<keyof FeedbackTextConfiguration, 'buttonLabel' | 'formTitle' | 'successMessageText'>
> {
/**
Expand Down Expand Up @@ -58,6 +60,8 @@ export function Form({
showName,
showEmail,
isAnonymous,
isNameRequired,
isEmailRequired,

defaultName,
defaultEmail,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,

Expand All @@ -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,

Expand Down
20 changes: 16 additions & 4 deletions packages/feedback/src/widget/createWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

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

Ah good catch, I think maybe > 0 would read slightly more straightforward

dialog.showError(`Please enter in the following required fields: ${emptyField.join(', ')}`);
return;
}

Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions packages/feedback/test/widget/Dialog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ function renderDialog({
showEmail = true,
showBranding = false,
isAnonymous = false,
isNameRequired = false,
isEmailRequired = false,
formTitle = 'Feedback',
defaultName = 'Foo Bar',
defaultEmail = '[email protected]',
Expand All @@ -30,6 +32,8 @@ function renderDialog({
isAnonymous,
showName,
showEmail,
isNameRequired,
isEmailRequired,
showBranding,
defaultName,
defaultEmail,
Expand Down
24 changes: 10 additions & 14 deletions packages/feedback/test/widget/Form.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ function renderForm({
showName = true,
showEmail = true,
isAnonymous = false,
isNameRequired = false,
isEmailRequired = false,
defaultName = 'Foo Bar',
defaultEmail = '[email protected]',
nameLabel = 'Name',
Expand All @@ -25,6 +27,8 @@ function renderForm({
isAnonymous,
showName,
showEmail,
isNameRequired,
isEmailRequired,
defaultName,
defaultEmail,
nameLabel,
Expand Down Expand Up @@ -80,13 +84,15 @@ describe('Form', () => {
emailPlaceholder: '[email protected]!',
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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
});
Expand Down
66 changes: 66 additions & 0 deletions packages/feedback/test/widget/createWidget.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '[email protected]';
messageEl.value = 'My feedback';

widget.dialog?.el?.querySelector('form')?.dispatchEvent(new Event('submit'));
expect(sendFeedbackRequest).toHaveBeenCalledWith({
feedback: {
name: 'Jane Doe',
email: '[email protected]',
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();
});
Comment on lines +174 to +238
Copy link
Member

Choose a reason for hiding this comment

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

Nice 🎉


it('submits feedback with error on request', async () => {
const onSubmitError = jest.fn(() => {});
const { shadow, widget } = createShadowAndWidget({
Expand Down