-
Notifications
You must be signed in to change notification settings - Fork 71
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
[SOL-1998] Implement Show Answer button #101
Conversation
Thanks for the pull request, @itsjeyd! 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?repo=edx-solutions%2Fxblock-drag-and-drop-v2&number=101 |
c311a88
to
a94d352
Compare
@arbrandes This is looking great! 👍
|
I am reviewing the code changes still, but functionally it looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should defer to product, and I'd recommend holding off on further review until product signs off. FYI, @sstack Things that jumped out to me-- Hide Answer, icon. Not sure what else is different. |
I will wait to look over the PR until product weighs in. |
@arbrandes - a couple comments:
|
Hi @sstack22, I've addressed your comments on adcbf1c (@itsjeyd, you may want to update the PR description). The sandbox should be updated by the time you read this. Regarding the box highlight when pressing Show Answer: that is indicative of moving the keyboard focus to the first drop zone. For now, I removed that altogether. Nevertheless, it seems from TNL-4936 that when clicking Show Answer, the keyboard focus should be moved to the problem "meta" area. The closest thing in the DnDv2 XBlock is the item bank. If we move the focus there, however, it will be highlighted similarly: is that ok? Or should the keyboard focus not be moved at all? |
Thanks @arbrandes, description updated. |
One comment on the feedback noted while testing in the Assessment mode. Other than that Looks good to me. 👍 |
Where is keyboard focus going on "Show Answer"? |
Except for my question about "Show Answer" (and AJ's question above), 👍 |
@staubina, thanks for the review! Regarding the persistence of feedback when clicking Show Answer, as far as I know it should not happen. Plus, it's a trivial fix, so I'm including it in my next push. @cahrens, thanks as well. The keyboard focus currently doesn't leave the Show Answer button after pressing it. I asked above whether I should move it to the item bank. What do you think? |
@arbrandes to be consistent with how capa problems will work, the focus should move to the top of the problem so that the learner can easily navigate through and "see" the answer. |
In assessment mode, when the learner has run out of attempts, enable a "Show Answer" button. Clicking it causes all items to move to a correct placement.
On the final attempt, misplaced items are not returned to the bank. Change the "misplaced" feedback message to reflect this.
pylint correctly surmised that `test_grade` is an event test, so move it over.
Fix padding on the reset button so that all separators on the sidebar end up with the same height, and add padding to the top of the sidebar container to distance the separators from the top edge.
👍 |
@itsjeyd 👍 on my end! |
Thanks @sstack22, merging now. |
Description
In assessment mode, when the learner has run out of attempts, enable a "Show Answer" button. Clicking it causes all items to move to a correct placement.
Author
@arbrandes
OpenCraft review via open-craft#10.
Dependencies
None
JIRA
Sandbox URLs
Latest commit
Testing instructions
Assessment mode
staff@example.com
.Staff Debug Info
,Delete Student State
, and refresh the page.Standard mode
staff@example.com
.Notes
There is an existing issue (present on master) that causes misplaced items to return to the item bank when refreshing the page after submitting the last remaining attempt for a DnDv2 problem. This PR does not include a fix for this issue. As a result, refreshing the page as described in step 9 above only produces expected results if items that were placed on the board in the final attempt are located in correct zones.
Reviewers
Also tagging @pdesjardins as a docs FYI.