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.

Sign up for GitHub

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

feat: Revamp the UX of exercise page #382

Merged
merged 2 commits into from
Mar 29, 2024
Merged

feat: Revamp the UX of exercise page #382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9a832d
feat: Revamp the UX of exercise page
yammesicka Mar 29, 2024
9541036
fix: Semantics
yammesicka Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 31 additions & 14 deletions lms/lmsdb/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ class SolutionAssessment(BaseModel):
name = CharField()
icon = CharField(null=True)
color = CharField()
active_color = CharField()
active_color = CharField(default="#fff")
order = IntegerField(default=0, index=True)
course = ForeignKeyField(Course, backref='assessments')

Expand Down Expand Up @@ -718,6 +718,19 @@ def ordered_versions(self) -> Iterable['Solution']:
def test_results(self) -> Iterable[dict]:
return SolutionExerciseTestExecution.by_solution(self)

@staticmethod
def _get_summary(solution: 'Solution') -> dict:
exercise = {}
exercise['solution_id'] = solution.id
exercise['is_checked'] = solution.is_checked
exercise['comments_num'] = len(solution.staff_comments)
Comment on lines +723 to +726
Copy link

Choose a reason for hiding this comment

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

suggestion (code-quality): Merge dictionary assignment with declaration [×3] (merge-dict-assign)

Suggested change
exercise = {}
exercise['solution_id'] = solution.id
exercise['is_checked'] = solution.is_checked
exercise['comments_num'] = len(solution.staff_comments)
exercise = {
'solution_id': solution.id,
'is_checked': solution.is_checked,
'comments_num': len(solution.staff_comments),
}

if solution.is_checked and solution.checker:
exercise['checker'] = solution.checker.fullname
if solution.assessment:
exercise['assessment'] = solution.assessment.name
exercise['grade_color'] = solution.assessment.color
return exercise

@classmethod
def of_user(
cls, user_id: int, with_archived: bool = False,
Expand All @@ -737,15 +750,10 @@ def of_user(
.order_by(cls.submission_timestamp.desc())
)
for solution in solutions:
exercise = exercises[solution.exercise_id]
if exercise.get('solution_id') is None:
exercise['solution_id'] = solution.id
exercise['is_checked'] = solution.is_checked
exercise['comments_num'] = len(solution.staff_comments)
if solution.is_checked and solution.checker:
exercise['checker'] = solution.checker.fullname
if solution.assessment:
exercise['assessment'] = solution.assessment.name
id_ = solution.exercise_id
if exercises[id_].get('solution_id') is None:
exercises[id_].update(cls._get_summary(solution))

return tuple(exercises.values())

@property
Expand Down Expand Up @@ -1244,11 +1252,20 @@ def create_basic_roles() -> None:

def create_basic_assessments() -> None:
assessments_dict = {
'Excellent': {'color': 'green', 'icon': 'star', 'order': 1},
'Nice': {'color': 'blue', 'icon': 'check', 'order': 2},
'Try again': {'color': 'red', 'icon': 'exclamation', 'order': 3},
'Excellent': {
'color': 'green', 'icon': 'star', 'order': 1,
},
'Nice': {
'color': 'blue', 'icon': 'check', 'order': 2,
},
'Try again': {
'color': 'red', 'icon': 'exclamation', 'order': 3,
},
'Invalid': {
'color': 'red', 'icon': 'ban', 'order': 4,
},
'Plagiarism': {
'color': 'black', 'icon': 'exclamation-triangle', 'order': 4,
'color': 'black', 'icon': 'exclamation-triangle', 'order': 5,
},
}
courses = Course.select()
Expand Down
142 changes: 87 additions & 55 deletions lms/static/my.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,16 +124,18 @@ a {

.exercise {
align-items: center;
background: #f8f8f8;
border-radius: 5px;
border: 1px solid;
background: #f8f9fa;
border-radius: 10px;
box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1);
display: flex;
flex-direction: row;
justify-content: space-between;
margin-bottom: 0.5rem;
padding: 1rem;
text-align: center;
margin-bottom: 1rem;
overflow: auto;
padding: 0.75rem;
text-align: center;
border-right: #00000000 8px solid;
border-left: #00000000 8px solid;
}

.centered {
Expand All @@ -146,26 +148,34 @@ a {
align-items: stretch;
}

.right-side {
justify-content: start;
}

.left-side {
justify-content: end;
}

.exercise-number {
align-items: center;
border-radius: 100%;
border: 1px solid;
display: flex;
height: 3rem;
justify-content: center;
overflow: hidden;
text-align: center;
width: 3rem;
font-size: 1.2rem;
margin-right: 1rem;
color: #495057;
padding: 0.25rem 0.75rem;
border-radius: 15px;
min-width: 4rem;
justify-content: start;
}

.exercise-name {
align-items: center;
display: flex;
font-size: 1.5em;
font-size: 1.4rem;
line-height: 1.3;
height: 3rem;
justify-content: center;
text-align: center;
Expand Down Expand Up @@ -224,65 +234,62 @@ button#share-action:hover {
border-bottom-right-radius: 0;
}

.go-send {
background: #247ba0;
color: #ebf3f6;
}

.go-send:hover {
color: #ebf3f6;
}

.go-view {
background: #ffe066;
color: #18150a;
}

.go-view:hover {
color: #18150a;
#back-to-exercises {
width: 100%;
margin-top: 5vh;
}

.go-checked {
background: #70c1b3;
color: #0b1211;
.our-button {
align-items: center;
background-color: #ffffff;
border-radius: 20px;
border: 1px solid #dee2e6;
box-shadow: 0 1px 2px rgba(0, 0, 0, 0.2);
color: #495057;
cursor: pointer;
display: flex;
font-size: 0.9rem;
font-weight: 500;
justify-content: space-between;
margin: 0.5em;
width: 8em;
padding: 0.55rem 1rem;
text-align: center;
transition: background-color 0.2s, transform 0.2s, box-shadow 0.2s;
}

.go-checked:hover {
color: #0b1211;
.our-button:hover {
background-color: #0d6efd;
color: white;
box-shadow: 0 2px 4px rgba(0,0,0,0.15);
transform: translateY(-2px);
}

.go-grader {
background: #b2dbbf;
color: #1f2d3d;
.our-button:active {
transform: translateY(1px);
box-shadow: 0 1px 2px rgba(0,0,0,0.15);
}

.go-grader:hover {
color: #1f2d3d;
.our-button.active, .our-button:checked {
background-color: #28a745;
color: white;
}

#back-to-exercises {
width: 100%;
margin-top: 5vh;
.our-button.active:hover, .our-button:checked:hover {
background-color: #218838; /* Slightly darker green on hover */
}

.our-button {
border-radius: 8px;
display: flex;
margin-right: 0.5em;
outline: none;
padding: 0.55rem 1rem;
white-space: nowrap;
justify-content: space-between;
align-items: center;
height: 3em;
width: 8em;
@keyframes pop {
0% { transform: scale(0.9); }
50% { transform: scale(1.1); }
100% { transform: scale(1); }
}

.our-button:hover {
text-decoration: none;
.our-button:active {
animation: pop 0.5s;
}

.our-button-narrow {
button.our-button-narrow, a.our-button-narrow {
width: 2em;
display: flex;
align-items: center;
Expand Down Expand Up @@ -887,19 +894,44 @@ code .grader-add .fa {
}

.which-notebook {
align-items: center;
align-self: center;
display: flex;
margin-inline-end: 1rem;
}

.fa-book {
color: #495057;
font-size: 1rem;
margin-inline-end: 0.25rem;
vertical-align: middle;
}

.comments-count {
display: flex;
align-items: center;
font-size: 0.9rem;
color: #495057;
display: flex;
align-self: center;
align-items: center;
font-size: 1.25rem;
font-family: "Segoe UI", Tahoma, Geneva, Verdana, sans-serif;
margin-inline-end: 1rem;
}

.comments-count .our-badge {
background-color: #e9ecef; /* Neutral background */
color: #495057; /* Dark text color for readability */
padding: 0.25rem 0.5rem;
font-size: 0.75rem;
border-radius: 15px;
display: inline-flex;
align-items: center;
justify-content: center;
margin-inline-end: 0.5rem;
box-shadow: none; /* Remove the shadow if you prefer a flat design */
}

#courses-list {
overflow-y: auto;
max-height: 10em;
Expand Down
11 changes: 1 addition & 10 deletions lms/templates/exercises.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,12 @@ <h1 id="exercises-head">{{ _('Exercises') }}</h1>
</div>
<div id="exercises">
{%- for exercise in exercises %}
<div class="exercise">
<div class="exercise" {%- if exercise.grade_color %} style="border-right: 8px solid {{ exercise.grade_color }};" {%- endif -%}>
<div class="right-side {{ direction }}-language">
<div class="exercise-number me-3">{{ exercise['exercise_number'] }}</div>
<div class="exercise-name"><div class="ex-title">{{ exercise['exercise_name'] | e }}</div></div>
</div>
<div class="left-side">
<div class="comments-count">
<span class="badge bg-secondary">{{ exercise['comments_num'] }}</span>
<span class="visually-hidden">{{ _("Comments for the solution") }}</span>
</div>
{%- if exercise['notebook'] %}
<div class="which-notebook">
<i class="fa fa-book" aria-hidden="true"></i>
Expand All @@ -39,11 +35,6 @@ <h1 id="exercises-head">{{ _('Exercises') }}</h1>
<i class="fa fa-{{ details['icon'] }}" aria-hidden="true"></i>
</a>
{% endif -%}
{% if is_manager %}
<a class="our-button our-button-narrow go-grader" href="check/{{ exercise['exercise_id'] }}">
<i class="fa fa-graduation-cap" aria-hidden="true"></i>
</a>
{% endif %}
</div>
</div>
{% endfor -%}
Expand Down
12 changes: 12 additions & 0 deletions tests/test_solutions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,3 +686,15 @@ def test_solutions_of_user(
exercises = solution.of_user(student_user.id, from_all_courses=True)
assert exercises[0].get('assessment') is None
assert exercises[1].get('assessment') == 'Try again'

@staticmethod
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Missing test cases for edge conditions in test_get_solution_summary.

It would be beneficial to add test cases covering scenarios where a solution has comments, an assigned checker, and an assessment. This ensures that the summary method accurately reflects all aspects of a solution's state.

Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Add a test case for the new 'Invalid' assessment type.

Given the addition of a new 'Invalid' assessment type in the model changes, it's important to ensure that this assessment type is correctly handled in the solution summaries. A specific test case for this scenario would validate the integration.

def test_get_solution_summary(solution: Solution, staff_user: User):
summary = Solution._get_summary(solution)
assert summary.get('assessment') is None
assert summary['solution_id'] == solution.id
assert not summary['is_checked']

solution.mark_as_checked(staff_user)
solution = Solution.get_by_id(solution.id)
summary = Solution._get_summary(solution)
assert summary['is_checked']
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Consider adding more comprehensive assertions for test_get_solution_summary.

The test currently checks if the solution is marked as checked, but it could be enhanced by verifying other fields in the summary, such as comments_num, checker, and grade_color when applicable. This would ensure the summary method works as expected in various scenarios.

Loading