LX-309 Modify CAPA choicegroup problems to make custom radio/checkboxes possible

[pomegranited]

The LabXchange project requires custom styling of the radio/checkbox problems, which, due to the element structure, could not be done with CSS alone. We made some changes to the DOM to facilitate these customizations, which this PR submits upstream:

• Moves the <input> items out of their <label>, to be siblings, as is already done in Paragon.
  This is required so that CSS selectors which target the dynamic attributes of the input element, e.g.:

  input:hover + label
  input:checked + label 
  

  This is not possible without using sibling selectors, until browsers begin supporting :has yet. See stackoverflow thread for more info.
• Moves the .status icon out of the radio button label.
  This makes the status display more consistent with the checkbox multiple choice items (see screenshot below).
• Adds a submitted class to mark radio/checkbox items which were submitted as answers during the previous attempt.
  This allows us to style these options differently in LabXchange.
• Addresses some a11y changes requested by @wittjeff , cf https://github.com/edx/edx-platform/pull/21215#issuecomment-515595848

JIRA tickets: OSPR-3760

Screenshots:

Status icon for radio button problems moved below, to make consistent with checkbox multiple choice items:

Sandbox: Redeploying

• LMS: https://pr21215.sandbox.opencraft.hosting/
• Studio: https://studio.pr21215.sandbox.opencraft.hosting/

Merge deadline: ASAP

Testing instructions:

1. Sign in to the sandbox as a demo user, e.g. honor@example.com / edx
2. Navigate to the CAPA Demo course, and enrol.
3. Complete the problems.
   Ensure that you can answer all the questions using the keyboard alone, and that the radio button and checkbox problems are all labeled clearly.
   Ensure that the correct/incorrect/submitted status is conveyed clearly to all users.

Reviewers

• @symbolist
• edX reviewer[s] TBD

CC @ormsbee

[openedx-webhooks]

Thanks for the pull request, @pomegranited! I've created OSPR-3760 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

• supporting documentation
• edx-code email threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

[natabene]

@pomegranited Thank you for your contribution. Please let me know once it is ready for our review.

[pomegranited]

Thanks @natabene -- I still need to get the tests passing again, but could you run this change past the a11y team (e.g. @wittjeff ) to ensure it meets their accessibility standards?

[natabene]

@wittjeff Can you give this a look as soon as you can?

[wittjeff]

Hi @pomegranited ! What you have submitted looks OK but there are a few additional things to consider:

1. There is a WCAG 2.0 accessibility guideline that warns against use of color alone to indicate state. https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-without-color.html . So by that criterion, I think there needs to be a word somewhere in the block saying "submitted" (visible to all). I'm not sure where in the UI would be ideal though.
   1a) I wonder if it might make sense to change the action button text to Resubmit once the selection state has changed (along with the removal of "submitted" state).
   1b) I'm of the opinion that it would be better to have text/graphic redundancy with the Correct (check) and Incorrect (X) graphics. Further I think it might make most sense to combine the state info there. So perhaps it would say "Submitted: [check] Correct". Under this scheme the checkmark and X should have alt="" to avoid text output repetition.
2. Incidentally, I have been reviewing the platform for WCGA 2.1 conformance and there are some related changes that I would like to suggest. Maybe you would like to pick them up now? Or they could be folded in later of course.
   a) I think it would be good if the radio and checkbox sizes were a bit bigger, for better visual noticeability. Perhaps 22x22px?
   b) WCAG 2.1 requires a luminance contrast ratio of 3:1 or better for the borders of active elements. So #949494 would be better for the borders of the radio and checkbox inputs.
   c) WCAG 2.1 also has a 3:1 minimum luminance contrast requirement for state info. I suggest adding a 1px box-shadow (in all 4 directions) to the border, instead of the browser default, for visual consistency across browsers.
   d) Related to c), it would be ideal if i) the grey used for "selected"+"submitted", and ii) the blue used for "selected"+"unsubmitted", each had minimum 3:1 contrast vs the (presumed white) background, AND vs. each other. This might require a darker grey (#949494) and a darker blue. I think the new blue being used in edx.org discovery pages may work for this. But I need to confirm the intended scope of use for that blue.
   If the items in 2) are confusing I can make a demo page just to illustrate.
   Thanks!

[pomegranited]

Awesome, thank you for your reply @wittjeff ! I was mainly worried about the <input><label> change, but glad it won't be a problem. Will see how many of your suggestions I can get through here too, and will tackle at least #1, if not most of #2. Of the changes under #2, are any higher priority than the others?

[pomegranited]

@wittjeff

1a) I wonder if it might make sense to change the action button text to Resubmit once the selection state has changed (along with the removal of "submitted" state).

Totally agree. Added with 4f5836d.

1b) I'm of the opinion that it would be better to have text/graphic redundancy with the Correct (check) and Incorrect (X) graphics. Further I think it might make most sense to combine the state info there. So perhaps it would say "Submitted: [check] Correct".

I agree that we should be showing "Correct" or "Incorrect" text along with the status icon -- the icon by itself is ambiguous.

EDIT But I think that adding "Submitted:" starts making it pretty wordy. I've done as you requested though, so you can have a look. What do you think, is it ok to drop the "Submitted:" ?
I'm afraid I've had to pull out the added Submitted: text from the status bar, because it was messing with the Show Answer functionality as shown below:

Under this scheme the checkmark and X should have alt="" to avoid text output repetition.

Currently, the text that's shown on hover is a "tooltip", not an "alt" text, and they read rather nicely:

• This answer is correct.
• This answer is incorrect.

Do you want these tooltips removed?

2a) I think it would be good if the radio and checkbox sizes were a bit bigger, for better visual noticeability. Perhaps 22x22px?

Done with 47d30ef -- see screenshots in PR description.

If the items in 2) are confusing I can make a demo page just to illustrate.

Could you? I'm getting confused about which borders you mean, and want to make sure I get the colours right. Thank you!

[pomegranited]

This is ready for your review @symbolist .

Just waiting on a couple of things before I can complete @wittjeff 's requests above:

• Should I remove the "tooltips" from the status text?
• Demo page requested to demonstrate colors/borders for item 2.

[symbolist]

@pomegranited Thanks! I have tested this out and left a few comments.

[symbolist]

@pomegranited Thanks!

• I tested this by working through various capa problems with a keyboard and screenreader.
• I read through the code
• I tested that the UI can be used with a keyboard only (tab order, keyboard controls).
• Includes documentation. N/A

[pomegranited]

@natabene This is ready for edX review.

As noted above, I still have a couple of outstanding changes requested by @wittjeff , but am waiting on his reply there.

[natabene]

@marcotuts Do you want to have a look before this goes to engineering?

[wittjeff]

It all looks good. Thanks!

[wittjeff]

1. Yes I think you can remove the tooltips but I don't think it is urgent.
2. Thanks for the icon size nudge. Re the colors, I have Arbisoft people working on this and some related ones now so there is no need to bother with it now.
   Thanks!

[pomegranited]

Thanks @wittjeff ! I went ahead and removed the tooltips.

[wittjeff]

@marcotuts The switch to symbol+text is not a strict accessibility requirement though I think it is stronger for understanding, so really a usability thing. Is there a problem with i18n?

22px has been suggested as a minimum touch target size in other contexts so that was just a nudge in that direction (not strictly needed here because the whole input+label region is active, but made with future consistency in mind).

I see your meeting request, can wait til then to discuss if you'd prefer.

edit: discussion of icons & text:
http://edwardsanchez.me/blog/13589712
https://uxdesign.cc/when-should-i-be-using-icons-63e7448202c4
https://ux.stackexchange.com/questions/94839/is-a-plus-symbol-without-button-text-sufficient-to-denote-adding-of-an-item

If there is a screen real estate scarcity concern, I'm intrigued by the notion of "progressive reduction."

[ormsbee]

@pomegranited: Nice! I'm starting code review now, but it looks like the sandbox isn't working. Is it possible to get it restarted?

As is always the case when we're playing in Capa-land, my main concern is what this means for compatibility with existing content. Are there corresponding Studio docs that need to be updated?

Thank you!

FYI @pdpinch, @Colin-Fredericks

[pomegranited]

Thanks @ormsbee !

it looks like the sandbox isn't working. Is it possible to get it restarted?

Yep, I've built a new one, so those links should work now.

As is always the case when we're playing in Capa-land, my main concern is what this means for compatibility with existing content.

The sandbox has the standard DemoX course on it, so you can see that it works with old imported problems.

Are there corresponding Studio docs that need to be updated?

There's no XML changes here, just GUI, so there may be some out-of-date screenshots in the docs? But no change to the author instructions or anything like that.

[Colin-Fredericks]

I am way too swamped to test this week.

If you'd like to try out my usual set of courses, here's the Tarballs for Testing folder. I would especially check the more unusual assessment tricks in Grape Ape and the "better basic problems" bit in Studio Advanced.

[pomegranited]

Hi @natabene @ormsbee Do you know when we might get this reviewed? Please let me know if I should rebase on latest master and deploy a new sandbox for testing. Thanks!

[ormsbee]

Sorry for the delay. I was looking at a couple of things last week on my local but ran into some devstack issues. Please merge/squash/put-awesome-commit-message, and I'll merge.

[marcotuts]

Hello! It looks like my comment was never submitted here after @wittjeff and I met. :(
My apologies for that.

For now, the summary I would give is that we should revert the visual changes identified in the comment above: https://github.com/edx/edx-platform/pull/21215#issuecomment-515595848

(For example, we should not add / show the additional "Correct" / "Incorrect" text to the right of the per-input option feedback check marks / x marks.) In other words, there should be no visual change in this PR relating to this part of the CAPA experience other than behind the scenes div / layout changes necessary to support styling.

If this summary doesn't make the list of required change reversions clear let me know! If it makes things easier I am happy to jump on a call / time block to discuss this and make sure we speed up response times for this change.

[pomegranited]

@marcotuts I've removed the "Correct" / "Incorrect" text added here, so the only remaining visual changes are:

• Status icon for radio problems is moved down below the problem, to the same place it is shown for checkbox problems.
• Radio buttons and checkbox problems are slightly larger, 22x22px.
• "Submit" button changes to "Resubmit" when attempts > 0.

I've provisioned a fresh sandbox, so you can preview these changes. The Capa Demo course has samples of all the permutations I could think of -- show answer, reset, show/hide correctness -- and has the usual demo accounts:

https://pr21215.sandbox.opencraft.hosting

@ormsbee Have rebased on latest master and squashed down to a single commit.

We have some LX frontend changes that are hinging on this, and so the timing is a little tricky. We'd really appreciate if this could be merged and deployed to stage.edx.org ASAP, and if all goes ok there, to edx.org before Monday's LX launch. Do you think that's possible?

CC @symbolist

[marcotuts]

Hi Jill thanks for the details on what has changed, I left a few thoughts below:

1.) Status icon for radio problems is moved down below the problem, to the same place it is shown for checkbox problems.
So that I make sure I'm looking at the right thing, you are saying that the X icon here is moved down, where it was previously shown in-context within the input element:

If that is the case I think this is fine, though I'm not actually sure if this is necessarily a step forward here? I'm curious if in-context is actually better than the messages below, and that the status below is best used for when there is both in-context and overall scores (especially if the status is different as with partial credit cases). I'm ok with this change though, no action necessary just calling out my thought process on this one.

2.) Radio buttons and checkbox problems are slightly larger, 22x22px.
This is also ok I think, though generally I wouldn't override defaults and would rely on the same size of the text to the right of the input? No changes needed here.

3.) "Submit" button changes to "Resubmit" when attempts > 0.
This should not change I believe as part of this work. We should keep the current behavior that shows you the attempts to the right of the button with the Submit language.

[ormsbee]

I'm re-reviewing after each code commit to try to save roundtrip time. From the code perspective, this is still good. @marcotuts: So as long as the "Resubmit" text is change, we're good to merge?

[pomegranited]

@marcotuts CC @ormsbee

1.) Status icon for radio problems is moved down below the problem, to the same place it is shown for checkbox problems.
So that I make sure I'm looking at the right thing, you are saying that the X icon here is moved down, where it was previously shown in-context within the input element:

Yes, that's the change made here.

I'm curious if in-context is actually better than the messages below, and that the status below is best used for when there is both in-context and overall scores (especially if the status is different as with partial credit cases).

The aim here was for consistency, and we couldn't change the checkbox problems to use in-context status markers without exposing partial correctness of those answers, which would dramatically change the way learners interact with checkbox problems. (E.g., if I have 2 attempts available, I'd tick all the options first, and then it shows me which ones are correct.)

The per-problem status area is the same even for multi-problem partial credit cases -- partial correctness is shown at the very bottom (see Show Correctness > Checkbox & Radio & Numeric for a working example):

3.) "Submit" button changes to "Resubmit" when attempts > 0.
This should not change I believe as part of this work. We should keep the current behavior that shows you the attempts to the right of the button with the Submit language.

I've reverted this change with
fb981bf , see diff. (Force-pushed to keep this PR in a one-commit merge-able state).

The sandbox is also updated, so you can verify the change.

[edx-status-bot]

Your PR has finished running tests. There were no failures.

[marcotuts]

Thank you so much @pomegranited for the changes here! I can commit to having an internal T&L review / check on the items you backed out of this PR to see what it would take to reconsider items such as the resubmit button text, captured in the diff above. This looks good to go from my standpoint!

[openedx-webhooks]

@pomegranited 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

[pomegranited]

Thanks for your help and advice here @wittjeff @marcotuts @ormsbee !
From my perspective, I thought that discussing these changes with @wittjeff as the a11y expert would be the best way to ensure we weren't going the wrong way, and we took a little time to apply his recommendations. So it was pretty confusing and counter-productive to have to back them out. We'd appreciate guidance in the future on how best to get UI changes like these through.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Thursday, September 12, 2019.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.