Skip to content

Commit f185df6

Browse files
c298leec298lee
committed
comments
1 parent 9086e25 commit f185df6

File tree

4 files changed

+106
-68
lines changed

4 files changed

+106
-68
lines changed

‎packages/feedback/src/widget/Form.ts‎

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ export function Form({
114114

115115
const nameEl = createElement('input', {
116116
id: 'name',
117-
type: showName || isNameRequired ? 'text' : 'hidden',
118-
['aria-hidden']: showName || isNameRequired ? 'false' : 'true',
117+
type: showName ? 'text' : 'hidden',
118+
['aria-hidden']: showName ? 'false' : 'true',
119119
name: 'name',
120120
required: isNameRequired,
121121
className: 'form__input',
@@ -125,8 +125,8 @@ export function Form({
125125

126126
const emailEl = createElement('input', {
127127
id: 'email',
128-
type: showEmail || isEmailRequired ? 'text' : 'hidden',
129-
['aria-hidden']: showEmail || isEmailRequired ? 'false' : 'true',
128+
type: showEmail ? 'text' : 'hidden',
129+
['aria-hidden']: showEmail ? 'false' : 'true',
130130
name: 'email',
131131
required: isEmailRequired,
132132
className: 'form__input',
@@ -167,48 +167,44 @@ export function Form({
167167
errorEl,
168168

169169
!isAnonymous &&
170-
(showName || isNameRequired) &&
170+
showName &&
171171
createElement(
172172
'label',
173173
{
174174
htmlFor: 'name',
175175
className: 'form__label',
176176
},
177-
isNameRequired
178-
? [
179-
createElement(
180-
'span',
181-
{ className: 'form__label__text' },
182-
nameLabel,
183-
createElement('span', { className: 'form__label__text--required' }, ' (required)'),
184-
),
185-
nameEl,
186-
]
187-
: [nameLabel, nameEl],
177+
[
178+
createElement(
179+
'span',
180+
{ className: 'form__label__text' },
181+
nameLabel,
182+
isNameRequired && createElement('span', { className: 'form__label__text--required' }, ' (required)'),
183+
),
184+
nameEl,
185+
],
188186
),
189-
!isAnonymous && !(showName || isNameRequired) && nameEl,
187+
!isAnonymous && !showName && nameEl,
190188

191189
!isAnonymous &&
192-
(showEmail || isEmailRequired) &&
190+
showEmail &&
193191
createElement(
194192
'label',
195193
{
196194
htmlFor: 'email',
197195
className: 'form__label',
198196
},
199-
isEmailRequired
200-
? [
201-
createElement(
202-
'span',
203-
{ className: 'form__label__text' },
204-
emailLabel,
205-
createElement('span', { className: 'form__label__text--required' }, ' (required)'),
206-
),
207-
emailEl,
208-
]
209-
: [emailLabel, emailEl],
197+
[
198+
createElement(
199+
'span',
200+
{ className: 'form__label__text' },
201+
emailLabel,
202+
isEmailRequired && createElement('span', { className: 'form__label__text--required' }, ' (required)'),
203+
),
204+
emailEl,
205+
],
210206
),
211-
!isAnonymous && !(showEmail || isEmailRequired) && emailEl,
207+
!isAnonymous && !showEmail && emailEl,
212208

213209
createElement(
214210
'label',

‎packages/feedback/src/widget/createWidget.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,18 @@ export function createWidget({
9494
return;
9595
}
9696

97+
// Simple validation for now, just check for non-empty message if name required
98+
if (options.isNameRequired && !feedback.name) {
99+
dialog.showError('Please enter in a name before submitting!');
100+
return;
101+
}
102+
103+
// Simple validation for now, just check for non-empty message if name required
104+
if (options.isEmailRequired && !feedback.email) {
105+
dialog.showError('Please enter in an email before submitting!');
106+
return;
107+
}
108+
97109
const result = await handleFeedbackSubmit(dialog, feedback);
98110

99111
// Error submitting feedback
@@ -156,8 +168,8 @@ export function createWidget({
156168
dialog = Dialog({
157169
colorScheme: options.colorScheme,
158170
showBranding: options.showBranding,
159-
showName: options.showName,
160-
showEmail: options.showEmail,
171+
showName: options.showName || options.isNameRequired,
172+
showEmail: options.showEmail || options.isEmailRequired,
161173
isAnonymous: options.isAnonymous,
162174
isNameRequired: options.isNameRequired,
163175
isEmailRequired: options.isEmailRequired,

‎packages/feedback/test/widget/Form.test.ts‎

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -104,42 +104,6 @@ describe('Form', () => {
104104
expect(messageInput.placeholder).toBe('What is the issue?!');
105105
});
106106

107-
it('submit is enabled if message is not empty', () => {
108-
const formComponent = renderForm();
109-
110-
const message = formComponent.el.querySelector('[name="message"]') as HTMLTextAreaElement;
111-
112-
message.value = 'Foo (message)';
113-
message.dispatchEvent(new KeyboardEvent('keyup'));
114-
115-
message.value = '';
116-
message.dispatchEvent(new KeyboardEvent('keyup'));
117-
});
118-
119-
it('submit is enabled if name is required and is not empty', () => {
120-
const formComponent = renderForm({ isNameRequired: true });
121-
122-
const name = formComponent.el.querySelector('[name="name"]') as HTMLTextAreaElement;
123-
124-
name.value = 'Foo Bar';
125-
name.dispatchEvent(new KeyboardEvent('keyup'));
126-
127-
name.value = '';
128-
name.dispatchEvent(new KeyboardEvent('keyup'));
129-
});
130-
131-
it('submit is enabled if email is required and is not empty', () => {
132-
const formComponent = renderForm({ isEmailRequired: true });
133-
134-
const email = formComponent.el.querySelector('[name="email"]') as HTMLTextAreaElement;
135-
136-
email.value = 'foo@example.org';
137-
email.dispatchEvent(new KeyboardEvent('keyup'));
138-
139-
email.value = '';
140-
email.dispatchEvent(new KeyboardEvent('keyup'));
141-
});
142-
143107
it('can show error', () => {
144108
const formComponent = renderForm();
145109
const errorEl = formComponent.el.querySelector('.form__error-container') as HTMLDivElement;

‎packages/feedback/test/widget/createWidget.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,72 @@ describe('createWidget', () => {
171171
expect(shadow.querySelector('.success-message')).toBeNull();
172172
});
173173

174+
it('only submits feedback successfully when all required fields are filled', async () => {
175+
const onSubmitSuccess = jest.fn(() => {});
176+
const { shadow, widget } = createShadowAndWidget({
177+
isNameRequired: true,
178+
isEmailRequired: true,
179+
onSubmitSuccess,
180+
});
181+
182+
(sendFeedbackRequest as jest.Mock).mockImplementation(() => {
183+
return true;
184+
});
185+
widget.actor?.el?.dispatchEvent(new Event('click'));
186+
187+
const nameEl = widget.dialog?.el?.querySelector('[name="name"]') as HTMLInputElement;
188+
const emailEl = widget.dialog?.el?.querySelector('[name="email"]') as HTMLInputElement;
189+
const messageEl = widget.dialog?.el?.querySelector('[name="message"]') as HTMLTextAreaElement;
190+
191+
nameEl.value = '';
192+
emailEl.value = '';
193+
messageEl.value = '';
194+
195+
widget.dialog?.el?.querySelector('form')?.dispatchEvent(new Event('submit'));
196+
expect(sendFeedbackRequest).toHaveBeenCalledTimes(0);
197+
198+
// sendFeedbackRequest is async
199+
await flushPromises();
200+
expect(onSubmitSuccess).toHaveBeenCalledTimes(0);
201+
202+
nameEl.value = '';
203+
emailEl.value = '';
204+
messageEl.value = 'My feedback';
205+
206+
widget.dialog?.el?.querySelector('form')?.dispatchEvent(new Event('submit'));
207+
expect(sendFeedbackRequest).toHaveBeenCalledTimes(0);
208+
209+
// sendFeedbackRequest is async
210+
await flushPromises();
211+
expect(onSubmitSuccess).toHaveBeenCalledTimes(0);
212+
213+
nameEl.value = 'Jane Doe';
214+
emailEl.value = 'jane@example.com';
215+
messageEl.value = 'My feedback';
216+
217+
widget.dialog?.el?.querySelector('form')?.dispatchEvent(new Event('submit'));
218+
expect(sendFeedbackRequest).toHaveBeenCalledWith({
219+
feedback: {
220+
name: 'Jane Doe',
221+
email: 'jane@example.com',
222+
message: 'My feedback',
223+
url: 'http://localhost/',
224+
replay_id: undefined,
225+
source: 'widget',
226+
},
227+
});
228+
229+
// sendFeedbackRequest is async
230+
await flushPromises();
231+
expect(onSubmitSuccess).toHaveBeenCalledTimes(1);
232+
233+
expect(widget.dialog).toBeUndefined();
234+
expect(shadow.querySelector('.success-message')?.textContent).toBe(SUCCESS_MESSAGE_TEXT);
235+
236+
jest.runAllTimers();
237+
expect(shadow.querySelector('.success-message')).toBeNull();
238+
});
239+
174240
it('submits feedback with error on request', async () => {
175241
const onSubmitError = jest.fn(() => {});
176242
const { shadow, widget } = createShadowAndWidget({

0 commit comments

Comments
 (0)