Assignment view students#7533
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 15480154988Details
💛 - Coveralls |
david-yz-liu
left a comment
There was a problem hiding this comment.
@chickenwaddle77 good work, I just left a few inline comments. Please also do a merge from upstream master, and fix the merge conflict in the Changelog (I don't anticipate this being a large conflict).
Changelog.md
Outdated
| ### 🚨 Breaking changes | ||
|
|
||
| ### ✨ New features and improvements | ||
| - Fix front-end Assignment view for students (#7533) |
There was a problem hiding this comment.
The word "fix" is a bit misleading, as there wasn't an error before. You can use the verb "Improve" instead. Also, "front-end" and "view" are redundant; I think you can just say "Assignment view" here.
app/views/assignments/show.html.erb
Outdated
| <% end %> | ||
|
|
||
| <% content_for :title, "#{Assignment.model_name.human} #{@assignment.short_identifier}" %> | ||
| <% content_for :title, "#{@assignment.short_identifier}: #{@assignment.description}" %> |
There was a problem hiding this comment.
This is on the right track, but loses out on distinguishing the "peer review" assignments (see code you deleted that was replaced by "Assignment information". This title should be consistent with that code.
| shuffle: Randomly select one top level file or directory from each starter file group | ||
| simple: Assign the default starter file group to all students | ||
| title: Starter Files | ||
| title: Starter files |
There was a problem hiding this comment.
This change is good, but a bit tricky because now the word "files" is no longer capitalized in the download button, which isn't good. You might need to do some case conversion in the front-end to make this work.
app/views/assignments/_read.html.erb
Outdated
| <ul> | ||
| <li><%= link_to t('submissions.student.files_submitted', count: @num_submitted_files), | ||
| file_manager_course_assignment_submissions_path(course_id: @assignment.course_id, | ||
| assignment_id: @assignment.id)%></li> |
There was a problem hiding this comment.
add a space before the %>
david-yz-liu
left a comment
There was a problem hiding this comment.
@chickenwaddle77 good work, I just left two small inline comments.
app/views/assignments/_read.html.erb
Outdated
| <ul> | ||
| <li><%= link_to t('submissions.student.files_submitted', count: @num_submitted_files), | ||
| file_manager_course_assignment_submissions_path(course_id: @assignment.course_id, | ||
| assignment_id: @assignment.id)%> |
There was a problem hiding this comment.
I think you misunderstood my previous comment: you should add a space before the %>
There was a problem hiding this comment.
I see what you mean 👍
Changelog.md
Outdated
|
|
||
| ### ✨ New features and improvements | ||
| - Improved layout and labeling in the assignment settings form for both standard and timed assessments. (#7531) | ||
| - Improve Assignment view for students (#7533) |
There was a problem hiding this comment.
This is better, but make sure the verb "Improve" is in the past tense to match the style of other entries
david-yz-liu
left a comment
There was a problem hiding this comment.
Thanks, @chickenwaddle77!
1. Changed from "Assignment <short identifier>" to "<short identifier>: <description>".
2. Changed text in "Policies" section to be written in second-person ("You must complete this assignment individually.").
3. Make "Required Files" only show when there are files needed.
4. Moved "Starter Files" subsection to immediately above the "Submissions" subsection.
5. Modified title so "Files" in "Starter Files" is not capitalised.
6. Added period to the end of first line in starter files.
7. Moved to make "<X> files submitted" the first bullet point in "Submissions" and made it link to the "Submissions" tab.
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.)
Changes implements various updates to the user interface in the Assignments tab when viewed as a Student.
Changes Include:
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.)