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

StudentFeedbackResultsPage: separate self-responses from others' responses #8497 #8690

Closed

Conversation

davel37
Copy link
Contributor

@davel37 davel37 commented Mar 20, 2018

Fixes #8497

Outline of Solution

  • Change of text to help distinguish between our own feedback responses, and those from our teammates.

  • Bootstrap change to the color of the panel for our own responses.

screenshot 12

@craaaa craaaa requested review from craaaa and removed request for craaaa March 21, 2018 11:08
@craaaa craaaa self-assigned this Mar 21, 2018
@craaaa craaaa added the s.ToReview The PR is waiting for review(s) label Mar 22, 2018
Copy link
Contributor

@craaaa craaaa left a comment

Choose a reason for hiding this comment

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

A good start! Some changes need to be made to ensure that all the right responses get highlighted.

@@ -179,7 +179,7 @@ private FeedbackResultsResponseTable createResponseTable(FeedbackQuestionAttribu
if (question.giverType == FeedbackParticipantType.TEAMS && isUserPartOfGiverTeam) {
displayedGiverName = "Your Team (" + giverName + ")";
} else if (isUserGiver) {
displayedGiverName = "You";
displayedGiverName = "(Your own response)";
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think From: (Your own response) makes sense when displayed in the UI. From: Yourself might make more sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points. Actually I've just noticed a new pull request created today offers a better solution to this issue #6290 so this pull request can be closed now.

@@ -6,7 +6,7 @@

<c:choose>
<c:when test="${responseTable.giverNameYou}">
Copy link
Contributor

Choose a reason for hiding this comment

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

This check does not cover all cases. For instance, in the screenshot below, the response that is immediately relevant to the student is grayed out while responses that are intended for others are in blue (blue implied as the more relevant color).

You may want to consider the possible permutations of senders and receivers of a response, and decide which ones you think a student may find relevant.

image

@craaaa craaaa added s.Ongoing The PR is being worked on by the author(s) and removed s.ToReview The PR is waiting for review(s) labels Mar 22, 2018
@craaaa
Copy link
Contributor

craaaa commented Mar 24, 2018

Closing this PR as work on #8700 covers a similar purview.

However, I think that a separate issue #8690 touches on is ensuring that responses directed at the student (i.e. To: You responses) are visually separate from responses directed at others (for instance through dark blue vs light blue headers), and that the visual distinction is consistent across different question types. If that's something you're keen on working on, do reopen this PR or open a new one :)

@craaaa craaaa closed this Mar 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s.Ongoing The PR is being worked on by the author(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants