Skip to content
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

#144 improve visual consistency #146

Closed
wants to merge 4 commits into from

Conversation

kitsook
Copy link
Contributor

@kitsook kitsook commented Apr 16, 2018

Based on branch for #62,

  • Revise UI to improve visual consistency
  • Fix problem of response missing when switching between units
  • Add test cases for new chart format
  • Generate and return peer answers on-the-fly if nothing persisted

Closes #144 , closes #33, closes #148

jleong-openedx and others added 3 commits November 28, 2017 11:25
Add front-end changes to focus the user on reflecting on their initial
answer. This includes presenting them with an explicit choice to either
update or submit their response. Also, this adds a jump to their
initial response on the page if they choose to update their initial
response.

Clarify wording and vertically align button options. Remove focus from
submit button after advancing to Step 2. Correct anchoring functionality
when advancing steps and jumping to class breakdown.

Update packages to handle outdated dependency and security
vulnerability.
In Step 1, hide the rationale area before an option is selected.
@kitsook kitsook added this to the 1.0 milestone Apr 16, 2018
@lenglund
Copy link
Collaborator

@xcompass - How best to review?

@kitsook kitsook force-pushed the #144-improve-visual-consistency branch from cb1a7b9 to a770f95 Compare April 20, 2018 18:26
@coveralls
Copy link

coveralls commented Apr 20, 2018

Coverage Status

Coverage decreased (-0.4%) to 96.871% when pulling fa5a9de on #144-improve-visual-consistency into ff69bbb on master.

@kitsook kitsook force-pushed the #144-improve-visual-consistency branch 3 times, most recently from ab48668 to d7cd89a Compare April 25, 2018 23:24
@lenglund
Copy link
Collaborator

lenglund commented Jun 1, 2018

Thanks! Really great to see progress on this issue.

Comments for revision (most of which are me rethinking the original mock-ups a bit):

  • I couldn’t seem to get more than one explanation per answer to show? Even after increasing the # of answers seen and switching between random/simple? Is there a hard-coded limit to one explanation per answer somewhere?
  • I think we can remove the yellow warning alert now (i.e., “Note: In order to move to…”) in Step 1.
  • The “Next Step” button briefly flashes to “Submit Answer” sometimes after I click it in Step 1 – is this preventable?
  • Let’s make person icons bigger in Step 2 at 130%.
  • For the sample answers, let’s display:block, line-height:140%, and padding-left:1em.
  • Also for “What would you like to do?” text, let’s font-weight:normal, font-size: 20%, and padding-right:4.8em.
  • Can the bottom border that separates the student’s own answer from the peer answer above it sit with the peer answer, so when the revise option is selected the border remains (instead of moving to be between the revise explanation text box and submit button)? And this border should exist between multiple peer explanations as well (but not apply to the last one in each answer box)? (If that makes sense…)
  • Wondering if we need an exit option when the student chooses revise? Is it feasible to include a “Cancel” link (which would replace the revise button after revise is selected in Step 2) that if clicked would revert the student’s answer back to the original selection and explanation text (with text not editable and button options again revise or submit, i.e., revert to the first view of Step 2)?
  • Explanation text in Step 3 (both instructor and student) seems unusually large. Let’s keep that size consistent with the other screens (.85 em).
  • I’d also like to try bolding the “Correct Answer” text and limit the green colouring to this text (not include the answer text itself).
  • Can we increase the bar graph text to 12px size? Maybe with the bars at 20px height?
  • I also feel like the wording I suggested for the bar graph text is unclear. Maybe change “initial choice” -> “chose initially” and “after revision” -> “chose after revision”?

Copy link
Collaborator

@lenglund lenglund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted

@kitsook kitsook force-pushed the #144-improve-visual-consistency branch from d7cd89a to 185061f Compare June 8, 2018 23:43
@kitsook
Copy link
Contributor Author

kitsook commented Jun 11, 2018

Changes deployed to the testing environment. Besides the style changes, here are the logical changes:

I couldn’t seem to get more than one explanation per answer to show? Even after increasing the # of answers seen and switching between random/simple? Is there a hard-coded limit to one explanation per answer somewhere?

ubcpi shows max one explanation per answer. The "Answers Students See - Number Selected" option is to control the total number of explanation displayed in Step 2.

Edit: Scratch that. Seems that it is possible to display more that one peer explanation per answer. Probably problems with the frontend logic. Will check.

I think we can remove the yellow warning alert now (i.e., “Note: In order to move to…”) in Step 1.

Removed those “In order to…”. Keeping the warnings on character count. Please see if needed to re-word them:
"Note: Your rationale must be a minimum of x characters."
"Note: Your rationale must be a maximum of y characters."

The “Next Step” button briefly flashes to “Submit Answer” sometimes after I click it in Step 1 – is this preventable?

Suppressed.

Can the bottom border that separates the student’s own answer from the peer answer above it sit with the peer answer, so when the revise option is selected the border remains (instead of moving to be between the revise explanation text box and submit button)? And this border should exist between multiple peer explanations as well (but not apply to the last one in each answer box)? (If that makes sense…)

Moved the line to the bottom of peer explanation, before student’s own answer / edit box. No line is displayed if there is no explanation shown.

Wondering if we need an exit option when the student chooses revise? Is it feasible to include a “Cancel” link (which would replace the revise button after revise is selected in Step 2) that if clicked would revert the student’s answer back to the original selection and explanation text (with text not editable and button options again revise or submit, i.e., revert to the first view of Step 2)?

Added a "Cancel" button when student is in edit mode of Step 2. The "Cancel" button will revert the state to the beginning of Step 2 with student's original answer and explanation.

@kitsook kitsook force-pushed the #144-improve-visual-consistency branch from 185061f to b6e6d5c Compare June 12, 2018 18:16
@kitsook
Copy link
Contributor Author

kitsook commented Jun 12, 2018

@lenglund Issue of multiple peer explanations fixed. Testing environment updated and ready for review.

@lenglund
Copy link
Collaborator

lenglund commented Jun 13, 2018

Great, thanks! Just a few more tweaks:

  • Can those bottom borders for the sample explanations NOT appear under the last explanation in each answer box?
  • Can you re-word warnings to:
    "Note: Your explanation must be at least x characters long. Please explain further."
    "Note: Your explanation must contain fewer than y characters. Please trim the text."
  • As per the mock-ups, if no student explanations are selected for a given answer, can we please state that (i.e., (no student explanations were randomly selected for this answer)), with matching styling (i.e., matching margin)?
  • When no student explanations are selected for an answer, the answer does not enable a radio button when I select “Revise Answer”. This should still be a selectable option.

@kitsook kitsook force-pushed the #144-improve-visual-consistency branch from b6e6d5c to 38dc217 Compare June 13, 2018 22:46
@kitsook
Copy link
Contributor Author

kitsook commented Jun 15, 2018

@lenglund Testing environment updated with the fixes except the one below. Couldn't simulate it in testing environment or my local dev machine. Wondering if it was a cache issue or browser issue. Please let me know if it happens again.

When no student explanations are selected for an answer, the answer does not enable a radio button when I select “Revise Answer”. This should still be a selectable option.

@kitsook kitsook force-pushed the #144-improve-visual-consistency branch 3 times, most recently from dfa231f to 80e2a58 Compare June 27, 2018 21:57
@kitsook kitsook force-pushed the #144-improve-visual-consistency branch from 80e2a58 to 7adbde9 Compare August 8, 2018 21:39
- Revise UI to improve visual consistency
- Fix problem of response missing when switching between units
- Add test cases for new chart format
- Generate and return peer answers on-the-fly if nothing persisted
@kitsook kitsook force-pushed the #144-improve-visual-consistency branch from 7adbde9 to fa5a9de Compare August 10, 2018 17:59
Copy link
Member

@xcompass xcompass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kitsook
Copy link
Contributor Author

kitsook commented Nov 4, 2019

closing the PR. will rebase the change and submit another PR

@kitsook kitsook closed this Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants