Assignment summary v2#7559
Conversation
merge is necessary to add new tests
for more information, see https://pre-commit.ci
need to reconcile divergent branches
Repeat
Repeat file, accidentally pressed sync on GitHub instead of using command-line
Pull Request Test Coverage Report for Build 15721616170Details
💛 - Coveralls |
david-yz-liu
left a comment
david-yz-liu
left a comment
There was a problem hiding this comment.
@chickenwaddle77 you're definitely on the right track. I left one comment; but also, please add an additional sentence (in a new paragraph) in the section saying "For detailed information, see Annotations", where "Annotations" is a link to the assignment annotations tab.
| let annotation_summary = ( | ||
| <div className="distribution-graph"> | ||
| <h3>{I18n.t("assignments.annotation_summary")}</h3> | ||
| <p>{I18n.t("assignments.average_annotations", {average_annotations: 2.75})}</p> |
There was a problem hiding this comment.
This number should not be hard-coded
There was a problem hiding this comment.
Sorry David, was debugging and forgot to make it dynamic 😂
@david-yz-liu Hi David, do you want me to put that as a key in |
| <h3>{I18n.t("assignments.annotation_summary")}</h3> | ||
| <p> | ||
| {I18n.t("assignments.average_annotations", { | ||
| average_annotations: this.state.summary.average_annotations, |
There was a problem hiding this comment.
This isn't quite going to work if the state is null; you can use || to set a default value of 0
| --- | ||
| en: | ||
| assignments: | ||
| annotation: Annotation |
There was a problem hiding this comment.
You don't need to define a new key for this. Instead, you can use the existing key found in models/annotations/en.yml, which is also what the menu label uses. Use the plural form.
|
Hi @chickenwaddle77, one other comment. Because I've now merged in #7560, you'll need to do a merge from upstream/master, and then also add back in the localized string for displaying the average number of annotations (see the "Files Changed" tab of that PR to see what was removed). |
| this.props.assessment_id | ||
| )} | ||
| > | ||
| {I18n.t("activerecord.models.annotation.one")} |
…y-v2 merging to sync with upstrem/master
david-yz-liu
left a comment
david-yz-liu
left a comment
There was a problem hiding this comment.
@chickenwaddle77 nice work 🎉
3 participants
Proposed Changes
(Describe your changes here. Also describe the motivation for your changes: what problem do they solve, or how do they improve the application or codebase? If this pull request fixes an open issue, use a keyword to link this pull request to the issue.)
Added the average annotations section in the bottom of Assignment Summary -> Summary Statistics.
This was done by creating a new JSX expression that used the function
assignment.average_annotations.AssignmentController is modified to call
assignment.average_annotationsand return that when fetched to the front-end.Screenshots of your changes (if applicable)
Associated documentation repository pull request (if applicable)
Type of Change
(Write an
Xor a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]into a[x]in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request:
Questions and Comments
(Include any questions or comments you have regarding your changes.)