Add warning on empty quiz response #1931

alexsanford · 2017-10-05T17:48:41Z

Adds a new option to quizzes. When this option is enabled, users taking the quiz will see a warning if they have left any responses blank. They may still opt to submit the form with the blank responses.

Testing

Create a lesson with a quiz.
Check the box "Warn user on empty response".
Take the quiz. Leave some questions blank and ensure that the alert appears.
Ensure that "Ok" and "Cancel" in the alert behave as expected.
Test with all different question types (multi-line and file upload were particularly tricky, please test those on various browsers).
Turn the option off. Ensure that you can submit empty responses without seeing the alert.

Notes

This iteration will only warn the user. There is no option to actually prevent the user from submitting if there are empty responses.
We're just using a JS dialog here. I'm not sure that's what we want from a design perspective. Are there any modal dialogs that we use elsewhere on the frontend in Sensei? Alternatively I could just display a message right in the html. But I think the alert works for now.
I had to do a couple of hacky things in JS (no surprise there...) but I tried to make the code as clean as possible. Please feel free to call me out on anything nasty, and I can clean it up.
There are no tests for the JS functionality. @pgk I couldn't find any obvious existing JS tests. I'm assuming we don't have anything set up yet?
I set up gulp to do more JS minification, and updated the minified files across the board
The function sensei_quiz_action_on_empty_response is set up so that we could add other actions later (e.g. error)

…esponse

ice9js

I gave your changes a first look and it's looking good. I left some syntax related suggestions as well as one question for now.

ice9js · 2017-10-06T07:02:57Z

includes/template-functions.php

+
+	$warn = get_post_meta( $sensei_question_loop[ 'quiz_id' ], '_warn_on_empty_response', true );
+
+	if ( ! empty( $warn ) )


I think it's not necessarily a good practice to skip the curly braces - even for such short expressions.
You can also always use the shorthand notation which would be even better in this case:

return empty( $warn ) ? 'none' : 'warn';

I think it's not necessarily a good practice to skip the curly braces - even for such short expressions.

Good point! Especially when using goto statements 😄

Changed to the ternary statement.

ice9js · 2017-10-06T07:04:20Z

gulpfile.js

+		.pipe( rename({ extname: '.min.js' }) )
+		.pipe( chmod( 644 ) )
+		.pipe( gulp.dest( 'assets/js/frontend' ) );
+});


Another potentially missing space between } and ).

ice9js · 2017-10-06T07:05:58Z

gulpfile.js

+	return gulp.src( paths.frontendScripts )
+		// This will minify and rename to *.min.js
+		.pipe( uglify() )
+		.pipe( rename({ extname: '.min.js' }) )


I think there should be spaces between all the braces here according to our JS style guidelines. Not sure if that applies here as well though ?

Yeah good call. Most of this was copy+pasted, so I'll fix the spacing.

ice9js · 2017-10-06T08:44:49Z

includes/class-sensei-frontend.php

+				wp_register_script( 'sensei-quiz-js', esc_url( Sensei()->plugin_url . 'assets/js/frontend/quiz' . $suffix . '.js' ), array( 'jquery' ), '1', true );
+				wp_enqueue_script( 'sensei-quiz-js' );
+				wp_localize_script( 'sensei-quiz-js', 'quizL10n', array(
+					'empty_response_warn' => __( 'Some responses are empty! Are you sure you want to submit?' )


Missing a trailing comma.

ice9js · 2017-10-06T09:00:55Z

assets/js/frontend/quiz.js

+		button.removeAttr( 'clicked' );
+
+		// Only continue for the "Complete Quiz" button
+		if ( button.attr( 'name' ) != 'quiz_complete' ) {


So if I understand this correctly - this should not show the modal when pressing the Complete Quiz button ?
I'm asking as that still shows the modal for me and I'm actually wondering if we'd want such behaviour at all instead of always showing it when there are some empty fields ?

Actually, this should only show the modal when the user clicks the Complete Quiz button. If the button is not Complete Quiz, then we return here.

This is because the warning only makes sense (in my opinion) when the user is submitting the quiz to be graded. If they are clicking on Reset Quiz or Save Quiz, for example, then I don't think we need to warn them of empty responses.

Does that make sense?

donnapep · 2017-10-06T12:13:22Z

Check the box "Warn user on empty response".

I'm not seeing this box. Can you share a screenshot please? Thx!

alexsanford · 2017-10-06T12:54:31Z

@donnapep http://cld.wthms.co/7zZVde

This avoids having an "if" statement without braces, which can cause potential issues down the road. The ternary statement in this case is more succinct, and safer.

donnapep · 2017-10-06T13:18:16Z

@donnapep http://cld.wthms.co/7zZVde

Thanks! Was looking for it at the question level.

We're just using a JS dialog here. I'm not sure that's what we want from a design perspective. Are there any modal dialogs that we use elsewhere on the frontend in Sensei? Alternatively I could just display a message right in the html. But I think the alert works for now.

I agree. This is not very good UX. 😄 Ideally we would use a modal here. There are actually a fair number of places in Sensei that would benefit from having one. Given that would be a fairly large endeavour though, for the purposes of this change, showing an inline message sounds like an acceptable alternative. You could model it after this type of message that Sensei already has:

donnapep · 2017-10-06T13:29:30Z

Or maybe it would be better yellow since it requires user action. So something like this, but minus the icon and plus 2 buttons:

Changed to a warning on the page, styled along with the rest of the site, instead of a JS alert box.

alexsanford · 2017-10-06T19:43:10Z

Pushed the new warning dialog for empty responses:

I think it looks ok. Maybe not fantastic. But it's pretty consistent with the rest of the UI though. Might want to tweak the colorscheme a bit. I'm not sure about the yellow and blue? Would love to get some thoughts here.

@donnapep @ice9js I added some complexity in order to get this to work (if ( confirm( ... ) ) is much simpler!) so I would appreciate some eyes on my latest commit, and a re-test if you have some time 😄

donnapep · 2017-10-16T14:04:00Z

@alexsanford @pgk In going through existing Sensei issues, there are a couple of related issues that could also affect or be affected by this change, specifically #1509 and #1488.

I wonder if perhaps we should slow down a bit, and think about the overall quiz-taking flow at a high-level. Maybe in the end we'd put this change in after all, but at least we'd have done a thorough review first and determined what the optimal quiz workflow should be before pushing changes that later we realize we don't want or need.

Thoughts?

pgk · 2017-10-18T11:23:10Z

Adding this to future milestone along with the pr

AiramMontessori · 2018-03-19T14:54:19Z

Hi, is there any chance to see this feature sooner or later? In our process, it is extremely penalizing that our users can submit quiz by leaving some replies empty or by not replying to questions. It is even more problematic because there's no history of submitted quizzes. Meaning, if a user forgot one "answer" to a quiz, we do not validate the lesson, we ask them to then reset their quiz and we lose all the initial replies he already submitted. Of course in this case we request them to only reply to the "forgotten" question.
The consequence is that when we do not approve a quiz, we then have to copy-paste all the answers of a user in a Word document ... not very efficient.

There could be an optional feature at least in the back-end that, when enabled, automatically repopulates the fields of the quiz with all previous given answers upon "reset the quiz" button hit. It will save our users and ourselves tons of time and lots of hassles.

Thanks for the great work guys anyway!

donnapep · 2018-03-20T10:38:34Z

@AiramMontessori There are a number of issues that have been reported with quizzes recently. We've realized that this is one area of Sensei that would benefit greatly from a full review of current functionality. Until that is done, I'm wary of introducing small changes such as this one, which we could easily realize later wasn't properly thought through and perhaps even makes it more difficult to make other improvements.

For that reason, this change won't be going into Sensei in its current form. But rest assured that we will be taking a good hard look at the existing quiz functionality to see what improvements can be made there.

I've copied your comment over to #1718 as it's related to the discussion on that issue.

Thx.

alexsanford added 5 commits October 5, 2017 12:35

Add Gulp task to build frontend JS

4a040d3

Rebuild JS

9f30c94

Add option to warn on empty response

68954fd

Add data attr to html indicating that the quiz should warn on empty r…

4df19c5

…esponse

Add JS warning when some form fields are empty

76b985b

alexsanford added [Status] Needs Design Review [Type] Enhancement labels Oct 5, 2017

alexsanford self-assigned this Oct 5, 2017

alexsanford requested review from dbtlr, pgk, donnapep and ice9js October 5, 2017 17:48

ice9js mentioned this pull request Oct 6, 2017

Prevent 'Enter' from submitting a quiz #1924

Closed

ice9js reviewed Oct 6, 2017

View reviewed changes

alexsanford added 2 commits October 6, 2017 10:09

Change an "if" statement to ternary

4a1460a

This avoids having an "if" statement without braces, which can cause potential issues down the road. The ternary statement in this case is more succinct, and safer.

Fix style issues

cc19fee

Change the UI for the empty response warning

7fcf3cc

Changed to a warning on the page, styled along with the rest of the site, instead of a JS alert box.

donnapep mentioned this pull request Oct 6, 2017

Quiz - Pressing the enter key completes the quiz #1654

Closed

pgk added this to the Future Release milestone Oct 18, 2017

pgk mentioned this pull request Oct 18, 2017

Quiz can be completed without uploading a file #1718

Open

alexsanford removed their assignment Oct 19, 2017

donnapep mentioned this pull request Oct 25, 2017

Quiz Frontend Improvements #1940

Closed

donnapep removed this from the Future Release milestone Oct 31, 2017

donnapep removed the [Status] Needs Design Review label Feb 3, 2018

donnapep removed the [Status] Needs Dev Review label Mar 12, 2018

donnapep closed this Mar 20, 2018

donnapep deleted the add/1718-warning-on-empty-quiz-response branch March 20, 2018 10:38

dericleeyy mentioned this pull request Oct 15, 2024

No warning is shown when submitting empty quiz responses #7689

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add warning on empty quiz response #1931

Add warning on empty quiz response #1931

alexsanford commented Oct 5, 2017

ice9js left a comment

ice9js Oct 6, 2017

alexsanford Oct 6, 2017

ice9js Oct 6, 2017

alexsanford Oct 6, 2017

ice9js Oct 6, 2017

alexsanford Oct 6, 2017

ice9js Oct 6, 2017

alexsanford Oct 6, 2017

ice9js Oct 6, 2017

alexsanford Oct 6, 2017

donnapep commented Oct 6, 2017

alexsanford commented Oct 6, 2017

donnapep commented Oct 6, 2017

donnapep commented Oct 6, 2017

alexsanford commented Oct 6, 2017

donnapep commented Oct 16, 2017 •

edited

Loading

pgk commented Oct 18, 2017

AiramMontessori commented Mar 19, 2018

donnapep commented Mar 20, 2018


		$warn = get_post_meta( $sensei_question_loop[ 'quiz_id' ], '_warn_on_empty_response', true );

		if ( ! empty( $warn ) )

Add warning on empty quiz response #1931

Add warning on empty quiz response #1931

Conversation

alexsanford commented Oct 5, 2017

Testing

Notes

ice9js left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donnapep commented Oct 6, 2017

alexsanford commented Oct 6, 2017

donnapep commented Oct 6, 2017

donnapep commented Oct 6, 2017

alexsanford commented Oct 6, 2017

donnapep commented Oct 16, 2017 • edited Loading

pgk commented Oct 18, 2017

AiramMontessori commented Mar 19, 2018

donnapep commented Mar 20, 2018

donnapep commented Oct 16, 2017 •

edited

Loading