Skip to content

Commit bd717ac

Browse files
committed
refactor: switch to existing buttonEnabled helper
1 parent c8e47f1 commit bd717ac

File tree

6 files changed

+54
-72
lines changed

6 files changed

+54
-72
lines changed

openassessment/xblock/static/dist/manifest.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
"openassessment-editor-textarea.js.map": "/openassessment-editor-textarea.cbb31e4372be87d437fb.js.map",
55
"openassessment-editor-tinymce.js": "/openassessment-editor-tinymce.2a1e66e98a2a1132f633.js",
66
"openassessment-editor-tinymce.js.map": "/openassessment-editor-tinymce.2a1e66e98a2a1132f633.js.map",
7-
"openassessment-lms.css": "/openassessment-lms.3b32189187e8e2d3e0c3.css",
8-
"openassessment-lms.js": "/openassessment-lms.3b32189187e8e2d3e0c3.js",
9-
"openassessment-lms.css.map": "/openassessment-lms.3b32189187e8e2d3e0c3.css.map",
10-
"openassessment-lms.js.map": "/openassessment-lms.3b32189187e8e2d3e0c3.js.map",
7+
"openassessment-lms.css": "/openassessment-lms.1d1152611b06737ce755.css",
8+
"openassessment-lms.js": "/openassessment-lms.1d1152611b06737ce755.js",
9+
"openassessment-lms.css.map": "/openassessment-lms.1d1152611b06737ce755.css.map",
10+
"openassessment-lms.js.map": "/openassessment-lms.1d1152611b06737ce755.js.map",
1111
"openassessment-ltr.css": "/openassessment-ltr.97990de9668aaa4cd1d5.css",
1212
"openassessment-ltr.js": "/openassessment-ltr.97990de9668aaa4cd1d5.js",
1313
"openassessment-ltr.css.map": "/openassessment-ltr.97990de9668aaa4cd1d5.css.map",

openassessment/xblock/static/dist/openassessment-lms.1d1152611b06737ce755.css

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openassessment/xblock/static/dist/openassessment-lms.3b32189187e8e2d3e0c3.js renamed to openassessment/xblock/static/dist/openassessment-lms.1d1152611b06737ce755.js

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openassessment/xblock/static/dist/openassessment-lms.3b32189187e8e2d3e0c3.css

Lines changed: 0 additions & 3 deletions
This file was deleted.

openassessment/xblock/static/js/spec/lms/oa_response.js

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,17 @@ describe("OpenAssessment.ResponseView", function() {
152152
stubConfirm = didConfirm;
153153
};
154154

155+
/**
156+
Helper function to assert response buttons are all consistent and in an expected state
157+
(disabled or enabled).
158+
**/
159+
var expectResponseFieldsEnabled = function(view, expectedEnabled) {
160+
expect(view.baseView.buttonEnabled('.step--response__submit')).toBe(expectedEnabled);
161+
expect(view.baseView.buttonEnabled('.step--response .file__upload')).toBe(expectedEnabled);
162+
expect(view.baseView.buttonEnabled('.step--response .delete__uploaded__file')).toBe(expectedEnabled);
163+
expect(view.baseView.buttonEnabled('.step--response .submission__answer__upload')).toBe(expectedEnabled);
164+
};
165+
155166
// Store window URL object for replacing after tests;
156167
const windowURL = window.URL;
157168
const mockURL = {
@@ -239,21 +250,21 @@ describe("OpenAssessment.ResponseView", function() {
239250
// response is blank
240251
view.response(['', '']);
241252
view.handleResponseChanged();
242-
expect(view.submitEnabled()).toBe(true);
253+
expectResponseFieldsEnabled(view, true);
243254
expect(view.isValidForSubmit()).toBe(false);
244255
expect(view.saveStatus()).toContain('This response has not been saved');
245256

246257
// response is whitespace
247258
view.response([' \n \n ', ' ']);
248259
view.handleResponseChanged();
249-
expect(view.submitEnabled()).toBe(true);
260+
expectResponseFieldsEnabled(view, true);
250261
expect(view.isValidForSubmit()).toBe(false);
251262
expect(view.saveStatus()).toContain('This response has not been saved');
252263

253264
// response is not blank
254265
view.response(['Test response 1', ' ']);
255266
view.handleResponseChanged();
256-
expect(view.submitEnabled()).toBe(true);
267+
expectResponseFieldsEnabled(view, true);
257268
expect(view.isValidForSubmit()).toBe(true);
258269
expect(view.saveStatus()).toContain('Saving draft');
259270
});
@@ -262,14 +273,14 @@ describe("OpenAssessment.ResponseView", function() {
262273
view.textResponse = 'optional';
263274
view.fileUploadResponse = 'required';
264275

265-
expect(view.submitEnabled()).toBe(true);
276+
expectResponseFieldsEnabled(view, true);
266277
expect(view.isValidForSubmit()).toBe(false);
267278

268279
var files = [{type: 'application/pdf', size: 1024, name: 'application.pdf', data: ''}];
269280
view.prepareUpload(files, 'pdf-and-image', ['test description']);
270281
view.uploadFiles();
271282

272-
expect(view.submitEnabled()).toBe(true);
283+
expectResponseFieldsEnabled(view, true);
273284
expect(view.isValidForSubmit()).toBe(true);
274285
});
275286

@@ -336,7 +347,7 @@ describe("OpenAssessment.ResponseView", function() {
336347
expect(server.submit).not.toHaveBeenCalled();
337348
});
338349

339-
it("disables the submit button on submission", function() {
350+
it("disables the response buttons on submission", function() {
340351
// Prevent the server's response from resolving,
341352
// so we can see what happens before view gets re-rendered.
342353
spyOn(server, 'submit').and.callFake(function() {
@@ -345,10 +356,10 @@ describe("OpenAssessment.ResponseView", function() {
345356

346357
view.response(['Test response 1', 'Test response 2']);
347358
view.handleSubmitClicked();
348-
expect(view.submitEnabled()).toBe(false);
359+
expectResponseFieldsEnabled(view, false);
349360
});
350361

351-
it("re-enables the submit button on submission error", function() {
362+
it("re-enables the response buttons on submission error", function() {
352363
// Simulate a server error
353364
spyOn(server, 'submit').and.callFake(function() {
354365
return $.Deferred(function(defer) {
@@ -359,11 +370,11 @@ describe("OpenAssessment.ResponseView", function() {
359370
view.response(['Test response 1', 'Test response 2']);
360371
view.handleSubmitClicked();
361372

362-
// Expect the submit button to have been re-enabled
363-
expect(view.submitEnabled()).toBe(true);
373+
// Expect the response buttons to have been re-enabled
374+
expectResponseFieldsEnabled(view, true);
364375
});
365376

366-
it("re-enables the submit button after cancelling", function() {
377+
it("re-enables the response buttons after cancelling", function() {
367378
// Simulate the user cancelling the submission
368379
setStubConfirm(false);
369380
spyOn(server, 'submit').and.callThrough();
@@ -372,8 +383,8 @@ describe("OpenAssessment.ResponseView", function() {
372383
view.response(['Test response 1', 'Test response 2']);
373384
view.handleSubmitClicked();
374385

375-
// Expect the submit button to be re-enabled
376-
expect(view.submitEnabled()).toBe(true);
386+
// Expect the response buttons to be re-enabled
387+
expectResponseFieldsEnabled(view, true);
377388
});
378389

379390
it("moves to the next step on duplicate submission error", function() {
@@ -620,7 +631,7 @@ describe("OpenAssessment.ResponseView", function() {
620631
view.prepareUpload(files, 'image', ['i1', 'i2']);
621632
view.uploadFiles()
622633
expect(view.isValidForSubmit()).toBe(true);
623-
expect(view.submitEnabled()).toBe(true);
634+
expectResponseFieldsEnabled(view, true);
624635
expect(view.files.length).toEqual(2);
625636
view.prepareUpload(files, 'image', ['i1', 'i2']);
626637
view.uploadFiles()
@@ -641,7 +652,7 @@ describe("OpenAssessment.ResponseView", function() {
641652
view.prepareUpload(files, 'image', ['i1', 'i2','i1', 'i2','i1', 'i2','i1', 'i2','i1', 'i2','i1', 'i2',
642653
'i1', 'i2','i1', 'i2','i1', 'i2','i1', 'i2',]);
643654
expect(view.isValidForSubmit()).toBe(false);
644-
expect(view.submitEnabled()).toBe(true);
655+
expectResponseFieldsEnabled(view, true);
645656
expect(view.getSavedFileCount()).toEqual(20);
646657
var files2 = [];
647658
for(i=0; i<2;i++) {

openassessment/xblock/static/js/src/lms/oa_response.js

Lines changed: 19 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -166,19 +166,19 @@ export class ResponseView {
166166
// Override default form submission
167167
eventObject.preventDefault();
168168
$('.submission__answer__display__file', view.element).removeClass('is--hidden');
169-
view.disableFields();
169+
view.enableFields(false);
170170
spinner.removeClass('is--hidden');
171171
if (view.hasAllUploadFiles()) {
172172
const promise = view.uploadFiles();
173173
promise.then(() => {
174-
view.enableFields();
174+
view.enableFields(true);
175175
spinner.addClass('is--hidden');
176176
// Reset the internal files state once upload is complete,
177177
// to avoid the files being processed again if the user clicks upload again.
178178
this.files = null;
179179
});
180180
} else {
181-
view.enableFields();
181+
view.enableFields(true);
182182
spinner.addClass('is--hidden');
183183
}
184184
},
@@ -293,48 +293,19 @@ export class ResponseView {
293293
}
294294

295295
/**
296-
Enable/disable the submit button.
297-
Check that whether the submit button is enabled.
296+
Enable or disable the response fields.
297+
Disable the fields to avoid any changes while an action is processing.
298+
For example, use this when beginning a file upload, or submitting the response.
299+
Then enable them again when the action is complete.
298300
299301
Args:
300-
enabled (bool): If specified, set the state of the button.
301-
302-
Returns:
303-
bool: Whether the button is enabled.
304-
305-
Examples:
306-
>> view.submitEnabled(true); // enable the button
307-
>> view.submitEnabled(); // check whether the button is enabled
308-
>> true
309-
* */
310-
submitEnabled(enabled) {
311-
return this.baseView.buttonEnabled('.step--response__submit', enabled);
312-
}
313-
314-
/**
315-
* Disable the fields to avoid any changes while an action is processing.
316-
* For example, use this when beginning a file upload, or submitting the response.
317-
*/
318-
disableFields() {
319-
this.submitEnabled(false);
320-
321-
const responseEl = $('.step--response', this.element);
322-
responseEl.find('.file__upload').prop('disabled', true);
323-
responseEl.find('.delete__uploaded__file').prop('disabled', true);
324-
responseEl.find('.submission__answer__upload').prop('disabled', true);
325-
}
326-
327-
/**
328-
* Enable the fields to allow changes again.
329-
* Use this to revert the changes made by `disableFields()`.
302+
enabled (bool): set the state of the fields.
330303
*/
331-
enableFields() {
332-
this.submitEnabled(true);
333-
334-
const responseEl = $('.step--response', this.element);
335-
responseEl.find('.file__upload').prop('disabled', false);
336-
responseEl.find('.delete__uploaded__file').prop('disabled', false);
337-
responseEl.find('.submission__answer__upload').prop('disabled', false);
304+
enableFields(enabled) {
305+
this.baseView.buttonEnabled('.step--response__submit', enabled);
306+
this.baseView.buttonEnabled('.step--response .file__upload', enabled);
307+
this.baseView.buttonEnabled('.step--response .delete__uploaded__file', enabled);
308+
this.baseView.buttonEnabled('.step--response .submission__answer__upload', enabled);
338309
}
339310

340311
/**
@@ -555,7 +526,7 @@ export class ResponseView {
555526

556527
// Immediately disable the submit button to prevent multiple submission,
557528
// and other form fields to avoid race conditions if anything changes.
558-
this.disableFields();
529+
this.enableFields(false);
559530

560531
const view = this;
561532
const title = gettext('Confirm Submit Response');
@@ -578,7 +549,7 @@ export class ResponseView {
578549
title,
579550
msg,
580551
() => view.submit(),
581-
() => view.enableFields(),
552+
() => view.enableFields(true),
582553
);
583554
}
584555

@@ -600,7 +571,7 @@ export class ResponseView {
600571
if (errMsg) { this.baseView.toggleActionError('submit', errMsg); }
601572

602573
// Re-enable the submit button and the rest of the form so the user can retry
603-
this.enableFields();
574+
this.enableFields(true);
604575
}
605576
});
606577
}
@@ -871,13 +842,13 @@ export class ResponseView {
871842
*/
872843
handleDeleteFileClick(target) {
873844
const view = this;
874-
this.disableFields();
845+
this.enableFields(false);
875846
const filenum = $(target).attr('filenum');
876847
this.confirmationDialog.confirm(
877848
gettext('Confirm Delete Uploaded File'),
878849
this.getConfirmRemoveUploadedFileMessage(filenum),
879-
() => view.removeUploadedFile(filenum).always(() => view.enableFields()),
880-
() => view.enableFields(),
850+
() => view.removeUploadedFile(filenum).always(() => view.enableFields(true)),
851+
() => view.enableFields(true),
881852
);
882853
}
883854

0 commit comments

Comments
 (0)