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

Revert "Revert "Assessments: Add color to multiple choice assessments table"" #23553

Merged

Conversation

caleybrock
Copy link
Contributor

@caleybrock caleybrock commented Jul 6, 2018

Reverts #23552

Summary

For summary see original PR: #23532
Additionally this PR fixes some bugs detailed below.
Here's the main work from that PR:
screen shot 2018-07-05 at 12 54 40 pm

Pair programmed: Caley / @nkiruka

Bugs before

screen shot 2018-07-06 at 1 44 17 pm

screen shot 2018-07-06 at 1 44 30 pm

Bugs after

screen shot 2018-07-06 at 2 21 20 pm

screen shot 2018-07-06 at 2 21 34 pm


const opacity = calculateOpacity(percentValue);

const backgroundCSS = (isCorrectAnswer || isSurvey) ? {backgroundColor: `rgba(159, 212, 159, ${opacity})`} :
Copy link
Contributor

Choose a reason for hiding this comment

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

you could pull the logic on lines 42-45 into a function like this:

getBackgroundColor = (percentValue) => {
  const {isCorrectAnswer, isSurvey} = this.props;
  const opacity = calculateOpacity(percentValue);
  return (isCorrectAnswer || isSurvey) ? `rgba(159, 212, 159, ${opacity})` : `rgba(255, 99, 71, ${opacity})`;
};

then plug that into: const backgroundCSS = {backgroundColor: getBackgroundColor(percentValue}; to clarify the logic here a little. not a blocker, but something to consider

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea, will follow up.

Copy link
Contributor

@maddiedierker maddiedierker left a comment

Choose a reason for hiding this comment

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

left an inline comment with a suggestion, but otherwise this looks great!

@caleybrock caleybrock merged commit 3a7755b into staging Jul 6, 2018
@caleybrock caleybrock deleted the revert-23552-revert-23532-add-color-assessments-table branch July 6, 2018 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants