Skip to content

Commit

Permalink
fix: Make review table more responsive (#6053)
Browse files Browse the repository at this point in the history 
* fix: Improve layout of review table

* Progress

* Progress

* Final changes

* Fix tests

* Remove fluff

* Undo commits
  • Loading branch information
@larseggert
larseggert authored Jul 27, 2023
1 parent 593bdb4 commit 04df797
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 82 deletions.
4 changes: 2 additions & 2 deletions ietf/group/tests_review.py
View file
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
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
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
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>

0 comments on commit 04df797

Please sign in to comment.