[SOL-1944] Submit Answer UI #86

e-kolpakov · 2016-07-27T14:20:51Z

Description: This PR introduces concept of attempts to DnDv2 and adds UI elements to submit an answer and display number of attempts.

JIRA: Part of SOL-1944

Discussions: Discovery document for DnDv2 assessment mode (relevant section: 2.2.3) - covers only UI part of it, click handlers and backend processing will be implemented as a separate PR.

Sandbox URLs:

Currently pinned to 5754ac6

Screenshots:

Student view - can't submit as no items are placed

Student view - can submit (+ hover over reset):

Note "Reset problem" and "Keyboard help" are reworked to look more like the upcoming CAPA Problem redesign and edx-pattern-library in general.

Testing instructions:

Create/edit DnDv2; put it into assessment mode
Set "Max attempts" to zero.
Publish, open in LMS.
Note that "Submit" button is displayed. Make sure problem is in its initial state (reset if needed). Submit button should be disabled. Attempts info should not be displayed.
Put any item to correct zone. "Submit" button should become enabled.
Return to Studio, set "Max attempts" to any positive integer. Publish.
Return to LMS. Note attempts info is displayed. Should read "You have used 0 of <number_from_step_6> attempts."

Not implemented as part of this PR - expected behavior:

"Submit" button do nothing.
Used attempts are always zero.

Author concerns:

This PR copies some classes from edx-pattern-library into DnDv2 own css. Other options considered seems like a greater evil:

edx-pattern-library is only available in lms-main-v2.css in edX-platform, and that file is not added to courseware page
Including edx-pattern-library into DnDv2 is feasible, but problematic: needs converting css into sass/scss, installing edx-pattern-library as node module, build system or script and finally committing both source sass/scss and generated files. All in all - too much trouble (and definitely too much for this "feature" PR)

On the other hand, classes copied are more or less colocated, so when edx-pattern-library classes become available on courseware pages it should be sufficient to just drop their copied counterparts from DnDv2 css.

Reviewers

@mtyaka
TNL: @cahrens and/or @staubina
a11y: @cptvitamin
Product: @sstack22

openedx-webhooks · 2016-07-27T14:20:57Z

Thanks for the pull request, @e-kolpakov! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: https://openedx-webhooks.herokuapp.com/github/process_pr?number=86&repo=edx-solutions%2Fxblock-drag-and-drop-v2

e-kolpakov · 2016-07-28T14:22:42Z

@mtyaka for some reason tests are extremely flaky - at least one parameterized_final_feedback_and_reset or zone align test fail, but not a single one failed two times in a row + everything passes locally. I don't expect to change anything, so please ignore the failing CI build. Meanwhile, I'll try to make it pass.

UPD: and just after I have said that, CI build finishes successfully. :)

mtyaka · 2016-07-28T15:36:36Z

drag_and_drop_v2/drag_and_drop_v2.py

+        display_name=_("Maximum number of attempts"),
+        help=_(
+            "Limits the number of attempts learner can use to complete the problem. "
+            "For unlimited attempts, set to zero. This setting have no effect in standard mode."


typo: "have no effect" -> "has no effect"

Actually, this field and related studio edit functionality have already been implemented in #81, you should probably remove this code from this PR.

mtyaka · 2016-07-31T12:19:29Z

@e-kolpakov Thanks for the updates! FYI both #81 and #82 have been merged into master, so if you rebase you can remove the extra commits from this PR.

I have been testing this locally. The code looks good and it's working correctly. I only have a few suggestions regarding the styles of the buttons. (I am no UI expert, but these suggestions should make the styles closer to the CAPA styles from https://github.com/edx/edx-platform/pull/13052) :

In the Studio, the buttons and the "attempts" text look really tiny (see screenshot).
I think it would look better if the margin/padding between the submit button and the "You have used ..." text would be larger, closer to the screenshots from the CAPA PR. Same thing for "Keyboard Help"/"Reset" buttons - I would increase the padding between button text and the vertical line that divides the buttons.
The vertical line between "Keyboard Help" and "Reset" could be a bit brighter. It looks brighter in CAPA PR. Also, if you look closely, the top and bottom of the line are not straight (they are affected by buttons round borders).
I think it would work better if disabled "Reset" button didn't have a grey background (the text should be greyed out, but the background should remain white). I believe that's how it works in the linked CAPA PR. The grey background draws too much attention to the disabled "Reset" button.

Tiny buttons in the Studio:

Buttons in the LMS:

The sizes are good, but notice the issues outlined above:

the paddings could be increased
the "Reset" button should not have a grey background
the line between the right floated buttons is too dark and has rounded endings

e-kolpakov · 2016-08-01T10:04:00Z

Rebased from a909bd8

e-kolpakov · 2016-08-01T14:02:55Z

@mtyaka I've addressed the issues you have outlined - please review one more time.

mtyaka · 2016-08-01T14:26:32Z

Thanks @e-kolpakov, it looks very nice!

👍

I tested to make sure the buttons work correctly and look pretty
I read through the code
~~Includes documentation~~ (We will update the README (and our demo course) once assessment mode functionality is complete.)

e-kolpakov · 2016-08-01T16:51:40Z

@cahrens, @staubina, @cptvitamin, @sstack22 this PR is ready for your review.

cahrens · 2016-08-01T17:20:05Z

@alisan @chris-mike, look, they've copied our new button bar design! 💯

cahrens · 2016-08-01T18:11:04Z

The focus should go someplace sensible after "Reset" is pressed. For capa problems, focus will go back to the top of the problem (the "meta" section after the problem name). See https://openedx.atlassian.net/browse/TNL-4882

And I know Submit isn't working yet, but for that story you should think about where keyboard focus should go.

cahrens · 2016-08-01T18:14:39Z

I believe the number of attempts remaining should be linked to the Submit button through aria-describedby. @cptvitamin can confirm.

cahrens · 2016-08-01T18:57:37Z

In Studio the icons are italicized. You should be using spans for icons, not i tags. Also make sure to mark the icons as aria-hidden.

For examples, see http://ux.edx.org/design_elements/icons/.

cahrens · 2016-08-01T18:59:12Z

drag_and_drop_v2/public/js/drag_and_drop.js

-                            h('button.modal-dismiss-button', gettext("OK"))
+            h('div.keyboard-help-dialog', [
+                h('div.modal-window-overlay'),
+                h('div.modal-window', {attributes: {role: 'dialog', 'aria-labelledby': 'modal-window-title'}}, [


Incorrect aria-labelledby. Has to be an ID, not a class.

@cahrens technically I've just moved this code around (so this is an issue with previous versions of the code), but I've fixed it anyway.

e-kolpakov · 2016-08-02T14:01:51Z

@cahrens no, I was trying to explain that I was not able to do that :)

e-kolpakov · 2016-08-02T14:34:12Z

@cahrens I've just updated the sandbox

cahrens · 2016-08-03T01:42:38Z

For focus after pressing Reset, it might be better to put the focus up on the problem description (so it is read again by the screen reader). @cptvitamin can advise on this detail in his review.

Also, as I mentioned above the number of attempts remaining should be linked to the Submit button through aria-describedby. Again, @cptvitamin can confirm.

cahrens · 2016-08-03T01:45:36Z

I still have a concern about the button bar on small devices-- when it wraps, the help and reset buttons should be left-aligned above Submit (to match the new capa behavior).

If you have trouble achieving this, check in with @alisan617.

cahrens · 2016-08-03T01:58:40Z

drag_and_drop_v2/public/js/drag_and_drop.js

@@ -202,7 +202,7 @@ function DragAndDropTemplates(configuration) {
                h('div.modal-window-overlay'),
                h('div.modal-window', {attributes: {role: 'dialog', 'aria-labelledby': 'modal-window-title'}}, [
                    h('div.modal-header', [
-                        h('h2.modal-window-title', gettext('Keyboard Help'))
+                        h('h2.modal-window-title#modal-window-title', gettext('Keyboard Help'))


This caused a non-unique ID when 2 problems are on the same page. It is very easy to look for these sort of issues using Chrome and the aXe developer tool plugin.

e-kolpakov · 2016-08-03T10:53:04Z

@cahrens

Focus on description after reset - I've tried to do that, but for some reason neither Chrome nor Firefox set focus on description section, and AFAIK screen reader does not read it. Maybe I'm doing something wrong (I haven't spent more than 15 minutes trying to achieve this), so I would wait for @cptvitamin comments on this.
Buttons bar on small screens - implemented (looked up in https://github.com/edx/edx-platform/pull/13052)
Other review notes - addressed

staubina · 2016-08-03T12:54:51Z

My issue was corrected in another PR
👍

cahrens · 2016-08-03T13:43:35Z

@e-kolpakov Can you update the sandbox? I'm still seeing the help and reset buttons be right-aligned on wrap.

e-kolpakov · 2016-08-03T13:47:16Z

@cahrens sure - give me 15 minutes.

e-kolpakov · 2016-08-03T13:55:37Z

@cahrens updated.

cahrens · 2016-08-03T17:17:37Z

I'm still seeing issues with wrapping.

e-kolpakov · 2016-08-04T09:39:59Z

@cahrens that's because I'm just copying WIP styles from CAPA problems redesign: https://github.com/edx/edx-platform/pull/13052#issuecomment-237292836 :)

I've fixed the problem, but there it is not ideal still - for some screen sizes and "Submit" button length, sidebar buttons will still be shown on the right:

I would suggest keeping this "good enough" solution (I believe it is good enough, but correct me if I'm wrong) and make it better when/if CAPA redesign yields a better approach.

I'm updating sandbox right now, so hopefully by the time you see this message it will be ready.

cahrens · 2016-08-04T13:43:08Z

Nit: the Help/Reset buttons stay left-aligned for a time during the "wrapping" phase. See image below. Perhaps @alisan617 will come up with a better approach in the capa work, and then you guys can copy it then.

Update: Oh, and I just saw your comment about this! :)

cahrens · 2016-08-04T13:43:53Z

👍

e-kolpakov · 2016-08-04T13:53:55Z

@cahrens Thank you!

sstack22 · 2016-08-10T15:29:44Z

Sorry for not commenting sooner - 👍

bradenmacdonald · 2016-08-10T17:15:44Z

Merging now. Thanks all!

Due to scheduling requirements, full a11y review of this work is being tracked separately as SOL-1978

Edit: Actually, will squash first.

[#SOL-1944] Review notes: responsiveness, ARIA attributes, focus after reset

e-kolpakov changed the title ~~Submit Answer UI~~ [SOL-1944] Submit Answer UI Jul 27, 2016

e-kolpakov force-pushed the ekolpakov/assessment-submit-ux branch 2 times, most recently from 0d660d4 to 0450f4c Compare July 28, 2016 12:41

mtyaka reviewed Jul 28, 2016
View reviewed changes

bradenmacdonald mentioned this pull request Jul 28, 2016

Problem (capa) feedback rework openedx/edx-platform#13052

Merged

13 tasks

e-kolpakov force-pushed the ekolpakov/assessment-submit-ux branch 2 times, most recently from 2af17ff to a909bd8 Compare July 29, 2016 10:19

e-kolpakov force-pushed the ekolpakov/assessment-submit-ux branch from a909bd8 to f822741 Compare August 1, 2016 10:03

e-kolpakov force-pushed the ekolpakov/assessment-submit-ux branch 2 times, most recently from 289df94 to ca3f822 Compare August 1, 2016 12:46

cahrens reviewed Aug 1, 2016
View reviewed changes

e-kolpakov mentioned this pull request Aug 2, 2016

Click handlers and backend processing for assessment mode #87

Merged

4 tasks

cahrens reviewed Aug 3, 2016
View reviewed changes

e-kolpakov force-pushed the ekolpakov/assessment-submit-ux branch 3 times, most recently from f1406ec to c3b0cca Compare August 3, 2016 10:52

e-kolpakov force-pushed the ekolpakov/assessment-submit-ux branch from 343eb54 to 5754ac6 Compare August 4, 2016 10:13

Submission answer UI

dd94251

[#SOL-1944] Review notes: responsiveness, ARIA attributes, focus after reset

haikuginger force-pushed the ekolpakov/assessment-submit-ux branch from 1037959 to dd94251 Compare August 10, 2016 17:29

haikuginger merged commit 630a8d7 into openedx:master Aug 10, 2016

bradenmacdonald deleted the ekolpakov/assessment-submit-ux branch August 10, 2016 20:09

e-kolpakov mentioned this pull request Aug 11, 2016

Prevent overlapping drop placements #92

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SOL-1944] Submit Answer UI #86

[SOL-1944] Submit Answer UI #86

e-kolpakov commented Jul 27, 2016 •

edited by bradenmacdonald

Loading

openedx-webhooks commented Jul 27, 2016

e-kolpakov commented Jul 28, 2016 •

edited

Loading

mtyaka Jul 28, 2016

mtyaka Jul 28, 2016

mtyaka commented Jul 31, 2016 •

edited

Loading

e-kolpakov commented Aug 1, 2016

e-kolpakov commented Aug 1, 2016

mtyaka commented Aug 1, 2016

e-kolpakov commented Aug 1, 2016 •

edited

Loading

cahrens commented Aug 1, 2016

cahrens commented Aug 1, 2016

cahrens commented Aug 1, 2016

cahrens commented Aug 1, 2016 •

edited

Loading

cahrens Aug 1, 2016 •

edited

Loading

e-kolpakov Aug 2, 2016

e-kolpakov commented Aug 2, 2016

e-kolpakov commented Aug 2, 2016

cahrens commented Aug 3, 2016 •

edited

Loading

cahrens commented Aug 3, 2016 •

edited

Loading

cahrens Aug 3, 2016 •

edited

Loading

e-kolpakov commented Aug 3, 2016

staubina commented Aug 3, 2016

cahrens commented Aug 3, 2016

e-kolpakov commented Aug 3, 2016

e-kolpakov commented Aug 3, 2016

cahrens commented Aug 3, 2016

e-kolpakov commented Aug 4, 2016 •

edited

Loading

cahrens commented Aug 4, 2016 •

edited

Loading

cahrens commented Aug 4, 2016

e-kolpakov commented Aug 4, 2016

sstack22 commented Aug 10, 2016

bradenmacdonald commented Aug 10, 2016 •

edited

Loading

[SOL-1944] Submit Answer UI #86

[SOL-1944] Submit Answer UI #86

Conversation

e-kolpakov commented Jul 27, 2016 • edited by bradenmacdonald Loading

openedx-webhooks commented Jul 27, 2016

e-kolpakov commented Jul 28, 2016 • edited Loading

mtyaka Jul 28, 2016

Choose a reason for hiding this comment

mtyaka Jul 28, 2016

Choose a reason for hiding this comment

mtyaka commented Jul 31, 2016 • edited Loading

e-kolpakov commented Aug 1, 2016

e-kolpakov commented Aug 1, 2016

mtyaka commented Aug 1, 2016

e-kolpakov commented Aug 1, 2016 • edited Loading

cahrens commented Aug 1, 2016

cahrens commented Aug 1, 2016

cahrens commented Aug 1, 2016

cahrens commented Aug 1, 2016 • edited Loading

cahrens Aug 1, 2016 • edited Loading

Choose a reason for hiding this comment

e-kolpakov Aug 2, 2016

Choose a reason for hiding this comment

e-kolpakov commented Aug 2, 2016

e-kolpakov commented Aug 2, 2016

cahrens commented Aug 3, 2016 • edited Loading

cahrens commented Aug 3, 2016 • edited Loading

cahrens Aug 3, 2016 • edited Loading

Choose a reason for hiding this comment

e-kolpakov commented Aug 3, 2016

staubina commented Aug 3, 2016

cahrens commented Aug 3, 2016

e-kolpakov commented Aug 3, 2016

e-kolpakov commented Aug 3, 2016

cahrens commented Aug 3, 2016

e-kolpakov commented Aug 4, 2016 • edited Loading

cahrens commented Aug 4, 2016 • edited Loading

cahrens commented Aug 4, 2016

e-kolpakov commented Aug 4, 2016

sstack22 commented Aug 10, 2016

bradenmacdonald commented Aug 10, 2016 • edited Loading

e-kolpakov commented Jul 27, 2016 •

edited by bradenmacdonald

Loading

e-kolpakov commented Jul 28, 2016 •

edited

Loading

mtyaka commented Jul 31, 2016 •

edited

Loading

e-kolpakov commented Aug 1, 2016 •

edited

Loading

cahrens commented Aug 1, 2016 •

edited

Loading

cahrens Aug 1, 2016 •

edited

Loading

cahrens commented Aug 3, 2016 •

edited

Loading

cahrens commented Aug 3, 2016 •

edited

Loading

cahrens Aug 3, 2016 •

edited

Loading

e-kolpakov commented Aug 4, 2016 •

edited

Loading

cahrens commented Aug 4, 2016 •

edited

Loading

bradenmacdonald commented Aug 10, 2016 •

edited

Loading