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

Jump to bottom

fix: Make review table more responsive #6053

Merged
merged 7 commits into from
Jul 27, 2023
Merged

fix: Make review table more responsive #6053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c44795d
fix: Improve layout of review table
larseggert Jul 20, 2023
20ac318
Progress
larseggert Jul 20, 2023
5019ae1
Progress
larseggert Jul 22, 2023
47e3904
Final changes
larseggert Jul 26, 2023
944bd0f
Fix tests
larseggert Jul 26, 2023
1481701
Remove fluff
larseggert Jul 26, 2023
c407449
Undo commits
larseggert Jul 26, 2023
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
4 changes: 2 additions & 2 deletions ietf/group/tests_review.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,13 @@ def test_reviewer_overview(self):
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
# review team members can see reason for being unavailable
self.assertContains(r, "Availability")
self.assertContains(r, "Available")

self.client.login(username="secretary", password="secretary+password")
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
# secretariat can see reason for being unavailable
self.assertContains(r, "Availability")
self.assertContains(r, "Available")

# add one closed review with no response and see it is visible
review_req2 = ReviewRequestFactory(state_id='completed',team=team)
Expand Down
143 changes: 78 additions & 65 deletions ietf/templates/group/reviewer_overview.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ <h2 class="mt-3">Reviewers</h2>
rotation with the next reviewer in the rotation at the top. Rows with darker backgrounds have the following meaning:
</p>
<div class="reviewer-overview">
<p class="alert alert-secondary my-3">
<p class="alert alert-info my-3">
Has already been assigned a document within the given interval.
</p>
<p class="alert alert-warning my-3">
Expand All @@ -44,89 +44,102 @@ <h2 class="mt-3">Reviewers</h2>
</div>
{% endif %}
{% if reviewers %}
<table class="table reviewer-overview tablesorter">
<table class="table table-sm table-striped reviewer-overview tablesorter">
<thead>
<tr>
<th scope="col" data-sort="num">Next</th>
<th scope="col" data-sort="next">Next</th>
<th scope="col" data-sort="reviewer">Reviewer</th>
<th scope="col" data-sort="assigned">Recent history</th>
<th scope="col" data-sort="num">Days since completed</th>
<th scope="col" data-sort="settings">Settings</th>
<th scope="col">
<div class="row">
<div class="col-md-2 me-1">Assigned</div>
<div class="col-md-2 me-1 d-none d-lg-block">Deadline</div>
<div class="col-md-2 me-1">State</div>
<div class="col-md-1 me-1 d-none d-lg-block">Days for review</div>
<div class="col">Document</div>
</div>
</th>
<th scope="col" data-sort="num">Days since review</th>
<th scope="col" class="d-none d-xl-table-cell w-25">Settings</th>
</tr>
</thead>
<tbody>
{% for person in reviewers %}
<tr {% if person.completely_unavailable %}class="table-danger" title="Is not available to do reviews at this time." {% elif person.busy %}class="table-secondary" title="Has already been assigned a document within the given interval." {% elif person.settings.skip_next %}class="table-warning" title="Will be skipped the next time at the top of rotation." {% endif %}>
<tr {% if person.completely_unavailable %}class="table-danger" title="Is not available to do reviews at this time." {% elif person.busy %}class="table-info" title="Has already been assigned a document within the given interval." {% elif person.settings.skip_next %}class="table-warning" title="Will be skipped the next time at the top of rotation." {% endif %}>
<td>{{ forloop.counter }}</td>

<td>
{% person_link person %}
{% if person.settings_url %}
<a href="{{ person.settings_url }}" class="btn btn-primary btn-small float-end"
title="{{ person.settings.expertise }}">Edit
</a>
{% endif %}
{% person_link person with_email=False %}

<div class="text-nowrap">
{% if person.settings_url %}
<a href="{{ person.settings_url }}" class="btn btn-primary btn-sm"
aria-label="Change settings for {{ person.name }}">
<i class="bi bi-gear"></i>
</a>
{% endif %}
<button type="button"
class="btn btn-primary btn-sm d-xl-none"
aria-label="View settings for {{ person.name }}"
data-bs-toggle="modal"
data-bs-target="#modal{{ forloop.counter }}">
<i class="bi bi-search"></i>
</button>
</div>
</td>

<td>
{% if person.latest_reqs %}
<table class="table table-sm table-borderless">
<thead>
<tr>
<th scope="col">Assigned</th>
<th scope="col">Deadline</th>
<th scope="col">State</th>
<th scope="col">Review time</th>
<th scope="col">Document</th>
</tr>
</thead>
<tbody>
{% for assn_pk, req_pk, doc_name, reviewed_rev, assigned_time, deadline, state, assignment_to_closure_days in person.latest_reqs %}
<tr>
<td>
{{ assigned_time|date }}
</td>
<td>
<a href="{% url 'ietf.doc.views_review.review_request' name=doc_name request_id=req_pk %}">{{ deadline|date }}</a>
</td>
<td>
<span class="badge rounded-pill bg-{% if state.slug == 'completed' or state.slug == 'part-completed' %}success{% elif state.slug == 'no-response' %}danger{% elif state.slug == 'overtaken' %}warning{% elif state.slug == 'requested' or state.slug == 'accepted' %}primary{% else %}secondary{% endif %}">{{ state.name }}</span>
</td>
<td>
{% if assignment_to_closure_days != None %}
{{ assignment_to_closure_days }} day{{ assignment_to_closure_days|pluralize }}
{% endif %}
</td>
<td class="text-nowrap">
{{ doc_name }}
{% if reviewed_rev %}-{{ reviewed_rev }}{% endif %}
</td>
</tr>
{% endfor %}
</tbody>
</table>
{% for assn_pk, req_pk, doc_name, reviewed_rev, assigned_time, deadline, state, assignment_to_closure_days in person.latest_reqs %}
<div class="row {% if not forloop.last %}border-bottom mb-1 pb-1{% endif %}">
<div class="col-md-2 me-1">
{{ assigned_time|date|split:"-"|join:"-<wbr>" }}
</div>
<div class="col-md-2 me-1 d-none d-lg-block">
<a href="{% url 'ietf.doc.views_review.review_request' name=doc_name request_id=req_pk %}">{{ deadline|date|split:"-"|join:"-<wbr>" }}</a>
</div>
<div class="col-md-2 me-1">
<span class="badge rounded-pill bg-{% if state.slug == 'completed' or state.slug == 'part-completed' %}success{% elif state.slug == 'no-response' %}danger{% elif state.slug == 'overtaken' %}warning{% elif state.slug == 'requested' or state.slug == 'accepted' %}primary{% else %}secondary{% endif %} text-wrap">{{ state.name }}</span>
</div>
<div class="col-md-1 me-1 text-end d-none d-lg-block">
{% if assignment_to_closure_days != None %}
{{ assignment_to_closure_days }}
{% endif %}
</div>
<div class="col">
{{ doc_name }}{% if reviewed_rev %}-{{ reviewed_rev }}{% endif %}
</div>
</div>
{% endfor %}
{% endif %}
</td>
<td>

<td class="text-end">
{% if person.days_since_completed_review != 9999 %}
{{ person.days_since_completed_review }}
{% endif %}
</td>
<td>
{% if person.settings.min_interval %}
<b>{{ person.settings.get_min_interval_display }}</b>
<br>
{% endif %}
{% if person.settings.skip_next %}
<b>Skip:</b> {{ person.settings.skip_next }}
<br>
{% endif %}
{% if person.settings.filter_re %}
<b>Filter:</b> <code title="{{ person.settings.filter_re }}">{{ person.settings.filter_re|truncatechars:15 }}</code>
<br>
{% endif %}
{% if person.unavailable_periods %}
{% include "review/unavailable_table.html" with unavailable_periods=person.unavailable_periods %}
{% endif %}

<td class="d-none d-xl-table-cell w-25">
{% include "review/unavailable_table.html" with person=person unavailable_periods=person.unavailable_periods %}
<div class="modal modal-xl fade" id="modal{{ forloop.counter }}" tabindex="-1" aria-labelledby="modallabel{{ forloop.counter }}" aria-hidden="true">
<div class="modal-dialog">
<div class="modal-content">
<div class="modal-header">
<h1 class="modal-title fs-5" id="modallabel{{ forloop.counter }}">Reviewer settings</h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
</div>
<div class="modal-body">
{% include "review/unavailable_table.html" with person=person unavailable_periods=person.unavailable_periods %}
</div>
<div class="modal-footer">
<button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Close</button>
<button type="button" class="btn btn-primary">Save changes</button>
</div>
</div>
</div>
</div>
</td>

</tr>
{% endfor %}
</tbody>
Expand Down
8 changes: 4 additions & 4 deletions ietf/templates/ietfauth/review_overview.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ <h2 class="mt-4">Latest closed review assignments</h2>
{% if r.due %}<span class="badge rounded-pill bg-warning">{{ r.due }} day{{ r.due|pluralize }}</span>{% endif %}
</td>
<td>
<span class="{% if r.state_id == "completed" or r.state_id == "part-completed" %}bg-success{% endif %}">{{ r.state.name }}</span>
<span class="badge rounded-pill {% if r.state_id == "completed" or r.state_id == "part-completed" %}bg-success{% endif %}">{{ r.state.name }}</span>
</td>
<td>
{% if r.result %}{{ r.result.name }}{% endif %}
Expand Down Expand Up @@ -186,10 +186,10 @@ <h2 class="mt-4">
<table class="table table-sm table-striped tablesorter">
<thead>
<tr>
<th scope="col">
<th scope="col" data-sort="setting">
Setting
</th>
<th scope="col">
<th scope="col" data-sort="value">
Value
</th>
</tr>
Expand All @@ -208,7 +208,7 @@ <h2 class="mt-4">
Skip next assignments
</th>
<td>
{{ t.reviewer_settings.skip_next }}
{{ t.reviewer_settings.skip_next|yesno }}
</td>
</tr>
<tr>
Expand Down
51 changes: 40 additions & 11 deletions ietf/templates/review/unavailable_table.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,54 @@
<table class="mt-2 mb-0 table table-sm table-borderless">
{% load origin %}
{% origin %}
<table class="m-0 p-0 table table-sm table-borderless">
<thead class="d-nones">
<tr>
<th scope="row" class="p-0 col-3"></th>
<th scope="row" class="p-0 col"></th>
</tr>
</thead>
<tbody>
{% if person %}
{% if person.settings.min_interval %}
<tr>
<th class="bg-transparent p-0 pe-1" scope="row">Can review:</th>
<td class="bg-transparent p-0">{{ person.settings.get_min_interval_display }}</td>
</tr>
{% endif %}
{% if person.settings.skip_next %}
<tr>
<th class="bg-transparent p-0 pe-1" scope="row">Skip next:</th>
<td class="bg-transparent p-0">{{ person.settings.skip_next|yesno }}</td>
</tr>
{% endif %}
{% if person.settings.filter_re %}
<tr>
<th class="bg-transparent p-0 pe-1" scope="row">Filter:</th>
<td class="bg-transparent p-0"><code title="{{ person.settings.filter_re }}">{{ person.settings.filter_re }}</code></td>
</tr>
{% endif %}
{% endif %}
{% for p in unavailable_periods %}
<tr class="unavailable-period-{{ p.state }}">
<th scope="row">Period:</th>
<td>{{ p.state }}</td>
<th class="bg-transparent p-0 pe-1" scope="row">Period:</th>
<td class="bg-transparent p-0">{{ p.state }}</td>
</tr>
<tr class="unavailable-period-{{ p.state }}">
<th scope="row">Dates:</th>
<td>
{% if p.start_date or p.end_date %}{{ p.start_date|default:"&infin;" }} -{% endif %}
{{ p.end_date|default:"&infin;" }}
<th class="bg-transparent p-0 pe-1" scope="row">Dates:</th>
<td class="bg-transparent p-0">
{% if p.start_date or p.end_date %}{{ p.start_date|default:"&infin;" }}-{% endif %}{{ p.end_date|default:"&infin;" }}
</td>
</tr>
<tr class="unavailable-period-{{ p.state }}">
<th scope="row">Availability:</th>
<td>{{ p.get_availability_display }}</td>
<th class="bg-transparent p-0 pe-1" scope="row">Available:</th>
<td class="bg-transparent p-0">{{ p.get_availability_display }}</td>
</tr>
{% if p.reason %}
<tr class="unavailable-period-{{ p.state }}">
<th scope="row">Reason:</th>
<td>{{ p.reason }}</td>
<th class="bg-transparent p-0 pe-1" scope="row">Reason:</th>
<td class="bg-transparent p-0">{{ p.reason }}</td>
</tr>
{% endif %}
{% endfor %}
</tbody>
</table>
Loading