-
Notifications
You must be signed in to change notification settings - Fork 29
Emphasize student reflection in Step 2 #137
base: master
Are you sure you want to change the base?
Conversation
ubcpi/static/html/ubcpi.html
Outdated
| <ul class="ubcpi-other-answers"> | ||
|
|
||
| <li> | ||
| <b><div class="other-answer" translate>"{{options[rc.answer_original].text}}" chosen by you because:</div></b> |
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.
please correct the indentation
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 have done this and updated the pull request.
ubcpi/static/html/ubcpi.html
Outdated
| </div><!-- .revised-answers --> | ||
| </div> | ||
| </div></div> | ||
| <p id="decision-prompt" style="text-align: center; display: none;">What would you like to do?</p> |
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.
please add translation tag for the new texts
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 have done this and updated the pull request.
|
Since this is related to the workflow and UI, I'm adding @lenglund as additional reviewer. |
|
Justin said I'd need to set up local dev? Can someone assist me with that? |
|
@lenglund Alternatively, I remember that we do have a sandbox http://edx.ctlt.ubc.ca/, but @xcompass will have to update it with the latest change from this branch that is to be examined. |
73d9147 to
28bd41c
Compare
|
@xcompass - Let me know when the sandbox is updated. Thanks! |
|
@lenglund sorry for the delay. Sandbox is updated. @jleong-openedx could you do a quick check to see if the changes are there? |
|
lenglund
left a comment
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.
Changes as noted.
|
"Add spacing so that the answer button choices are vertically centered in the grey box they appear in (right now there is far more spacing at the top)" @lenglund I'm not sure that I'm seeing this/looking in the correct spot; which step does this apply to? |
|
When the two button options for either updating or submitting the answer appear, these two blue buttons are not vertically centred in the surrounding box, at least on my screen. (I can show you tomorrow if that helps more...) |
|
Okay I see now, thanks @lenglund! |
28bd41c to
79b59c4
Compare
|
I have made the changes; however the build is breaking due to the contextify package--I have so far been unable to locate/identify the package(s) that has/have contextify as a dependency. |
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.
79b59c4 to
e470157
Compare
|
@xcompass I have included the fix for the build-breaking issue; thanks. |
|
@jleong-openedx Did you address @lenglund's suggestion? Also, could you resolve the merge conflicts? |
In Step 1, hide the rationale area before an option is selected.
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.
Closes #62.