[PATCH v2 3/5] patch-detail: add label and button for comment addressed status
Daniel Axtens
dja at axtens.net
Wed Aug 11 09:47:46 AEST 2021
Raxel Gutierrez <raxel at google.com> writes:
> Add new label to show the status of whether a comment is addressed and a
> button to change the status of the comment. Only users that can edit
> the patch (submitter, delegate, project maintainers) can change the
> status of a comment. Clean up submission.html to have hyphen delimited
> id and class selector names as well. Before [1] and after [2] images
> for reference.
>
So another thing you do in this patch is change the show related and
show headers from links to buttons.
Please could you split this up. I think there are 3 things going on,
each of which should be in its own patch:
- rename things/refactor
- change show related/show header links to buttons
- add UI elements for comment addressed status
But if there's a different, more logical or simpler split feel free to
do that.
Skimming the diff I don't have many review comments. I'll do a deeper
dive when it's split.
> [1] https://imgur.com/NDfcPJy
> [2] https://imgur.com/kIhohED
>
> Signed-off-by: Raxel Gutierrez <raxel at google.com>
> ---
> htdocs/css/style.css | 46 +++++-
> patchwork/templates/patchwork/submission.html | 146 +++++++++++-------
> patchwork/views/patch.py | 1 +
> 3 files changed, 134 insertions(+), 59 deletions(-)
>
> diff --git a/htdocs/css/style.css b/htdocs/css/style.css
> index 243caa0..ff34ff5 100644
> --- a/htdocs/css/style.css
> +++ b/htdocs/css/style.css
> @@ -1,3 +1,9 @@
> +:root {
> + --light-color: #F7F7F7;
> + --warning-color: #f0ad4e;
> + --success-color: #5cb85c;
> +}
> +
> h2 {
> font-size: 25px;
> margin: 18px 0 18px 0;
> @@ -192,7 +198,7 @@ table.patchmeta tr th, table.patchmeta tr td {
> vertical-align: top;
> }
>
> -.submissionlist ul {
> +.submission-list ul {
> list-style-type: none;
> padding: 0;
> margin: 0;
> @@ -277,12 +283,18 @@ table.patchmeta tr th, table.patchmeta tr td {
> color: #f7977a;
> }
>
> -.comment .meta {
> +.patch-message .meta {
> + display: flex;
> + align-items: center;
> background: #f0f0f0;
> padding: 0.3em 0.5em;
> }
>
> -.comment .content {
> +.patch-message .message-date {
> + margin-left: 8px;
> +}
> +
> +.patch-message .content {
> border: 0;
> }
>
> @@ -294,6 +306,34 @@ table.patchmeta tr th, table.patchmeta tr td {
> font-family: "DejaVu Sans Mono", fixed;
> }
>
> +#comment-status-bar {
> + display: flex;
> + flex-wrap: wrap;
> + align-items: center;
> +}
> +
> +#comment-status-label {
> + margin: 0px 8px;
> +}
> +
> +#comment-action-addressed, #comment-action-unaddressed {
> + background-color: var(--light-color);
> + border-radius: 4px;
> +}
> +
> +#comment-action-addressed {
> + border-color: var(--success-color);
> +}
> +
> +#comment-action-unaddressed {
> + border-color: var(--warning-color);
> +}
> +
> +#comment-action-addressed:hover, #comment-action-unaddressed:hover {
> + background-color: var(--success-color);
> + color: var(--light-color);
> +}
> +
> .quote {
> color: #007f00;
> }
> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
> index 978559b..4109442 100644
> --- a/patchwork/templates/patchwork/submission.html
> +++ b/patchwork/templates/patchwork/submission.html
> @@ -1,3 +1,4 @@
> +
You probably don't need the extra newline here?
> {% extends "base.html" %}
>
> {% load humanize %}
> @@ -32,7 +33,7 @@ function toggle_div(link_id, headers_id, label_show, label_hide)
> <h1>{{ submission.name }}</h1>
> </div>
>
> -<table class="patchmeta">
> +<table class="patch-meta">
> <tr>
> <th>Message ID</th>
> {% if submission.list_archive_url %}
> @@ -61,12 +62,14 @@ function toggle_div(link_id, headers_id, label_show, label_hide)
> {% endif %}
> <tr>
> <th>Headers</th>
> - <td><a id="togglepatchheaders"
> - href="javascript:toggle_div('togglepatchheaders', 'patchheaders')"
> - >show</a>
> - <div id="patchheaders" class="patchheaders" style="display:none;">
> - <pre>{{submission.headers}}</pre>
> - </div>
> + <td>
> + <button
> + id="toggle-patch-headers"
> + onclick="javascript:toggle_div('toggle-patch-headers', 'patch-headers')"
> + >show</button>
> + <div id="patch-headers" class="patch-headers" style="display:none;">
> + <pre>{{submission.headers}}</pre>
> + </div>
> </td>
> </tr>
> {% if submission.series %}
> @@ -75,11 +78,12 @@ function toggle_div(link_id, headers_id, label_show, label_hide)
> <td>
> <a href="{% url 'patch-list' project_id=project.linkname %}?series={{ submission.series.id }}">
> {{ submission.series.name }}
> - </a> |
> - <a id="togglepatchseries"
> - href="javascript:toggle_div('togglepatchseries', 'patchseries', 'expand', 'collapse')"
> - >expand</a>
> - <div id="patchseries" class="submissionlist" style="display:none;">
> + </a>
> + <button
> + id="toggle-patch-series"
> + onclick="javascript:toggle_div('toggle-patch-series', 'patch-series', 'expand', 'collapse')"
> + >expand</button>
> + <div id="patch-series" class="submission-list" style="display:none;">
> <ul>
> {% with submission.series.cover_letter as cover %}
> <li>
> @@ -114,36 +118,38 @@ function toggle_div(link_id, headers_id, label_show, label_hide)
> <tr>
> <th>Related</th>
> <td>
> - <a id="togglerelated"
> - href="javascript:toggle_div('togglerelated', 'related')"
> - >show</a>
> - <div id="related" class="submissionlist" style="display:none;">
> - <ul>
> - {% for sibling in related_same_project %}
> - <li>
> - {% if sibling.id != submission.id %}
> - <a href="{% url 'patch-detail' project_id=project.linkname msgid=sibling.url_msgid %}">
> - {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> - </a>
> - {% endif %}
> - </li>
> - {% endfor %}
> - {% if related_different_project %}
> - <a id="togglerelatedoutside"
> - href="javascript:toggle_div('togglerelatedoutside', 'relatedoutside', 'show from other projects')"
> - >show from other projects</a>
> - <div id="relatedoutside" class="submissionlist" style="display:none;">
> - {% for sibling in related_outside %}
> + <button
> + id="toggle-related"
> + onclick="javascript:toggle_div('toggle-related', 'related')"
> + >show</button>
> + <div id="related" class="submission-list" style="display:none;">
> + <ul>
> + {% for sibling in related_same_project %}
> <li>
> - <a href="{% url 'patch-detail' project_id=sibling.project.linkname msgid=sibling.url_msgid %}">
> + {% if sibling.id != submission.id %}
> + <a href="{% url 'patch-detail' project_id=project.linkname msgid=sibling.url_msgid %}">
> {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> - </a> (in {{ sibling.project }})
> + </a>
> + {% endif %}
> </li>
> - {% endfor %}
> - </div>
> - {% endif %}
> - </ul>
> - </div>
> + {% endfor %}
> + {% if related_different_project %}
> + <button
> + id="toggle-related-outside"
> + onclick="javascript:toggle_div('toggle-related-outside', 'related-outside', 'show from other projects')"
> + >show from other projects</button>
> + <div id="related-outside" class="submission-list" style="display:none;">
> + {% for sibling in related_outside %}
> + <li>
> + <a href="{% url 'patch-detail' project_id=sibling.project.linkname msgid=sibling.url_msgid %}">
> + {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> + </a> (in {{ sibling.project }})
> + </li>
> + {% endfor %}
> + </div>
> + {% endif %}
> + </ul>
> + </div>
> </td>
> </tr>
> {% endif %}
> @@ -277,14 +283,14 @@ function toggle_div(link_id, headers_id, label_show, label_hide)
> {% else %}
> <h2>Message</h2>
> {% endif %}
> -<div class="comment">
> -<div class="meta">
> - <span>{{ submission.submitter|personify:project }}</span>
> - <span class="pull-right">{{ submission.date }} UTC</span>
> -</div>
> -<pre class="content">
> -{{ submission|commentsyntax }}
> -</pre>
> +<div class="patch-message">
> + <div class="meta">
> + <span>{{ submission.submitter|personify:project }}</span>
> + <span class="message-date">{{ submission.date }} UTC</span>
> + </div>
> + <pre class="content">
> + {{ submission|commentsyntax }}
> + </pre>
> </div>
>
> {% for item in comments %}
> @@ -293,15 +299,43 @@ function toggle_div(link_id, headers_id, label_show, label_hide)
> {% endif %}
>
> <a name="{{ item.id }}"></a>
> -<div class="comment">
> -<div class="meta">
> - <span>{{ item.submitter|personify:project }}</span>
> - <span class="pull-right">{{ item.date }} UTC | <a href="{% url 'comment-redirect' comment_id=item.id %}"
> - >#{{ forloop.counter }}</a></span>
> -</div>
> -<pre class="content">
> -{{ item|commentsyntax }}
> -</pre>
> +<div class="patch-message">
> + <div class="meta">
> + {{ item.submitter|personify:project }}
> + <span class="message-date">{{ item.date }} UTC |
> + <a href="{% url 'comment-redirect' comment_id=item.id %}">#{{ forloop.counter }}</a>
> + </span>
> + {% if item.addressed %}
> + <div id="comment-status-bar">
> + <div id="comment-status-label" class="text-success mx-3">
> + <span id="comment-status-icon" class="glyphicon glyphicon-ok-circle" aria-hidden="true"></span>
> + Addressed
> + </div>
> + {% if editable %}
> + <button id="comment-action-unaddressed" class="text-warning">
> + <span id="comment-action-icon" class="glyphicon glyphicon-warning-sign" aria-hidden="true"></span>
> + Unaddressed
> + </button>
> + {% endif %}
> + </div>
> + {% else %}
> + <div id="comment-status-bar">
> + <div id="comment-status-label" class="text-warning mx-3">
> + <span id="comment-status-icon" class="glyphicon glyphicon-warning-sign" aria-hidden="true"></span>
> + Unaddressed
> + </div>
> + {% if editable %}
> + <button id="comment-action-addressed" class="text-success">
> + <span id="comment-action-icon" class="glyphicon glyphicon-ok-circle" aria-hidden="true"></span>
> + Addressed
> + </button>
> + {% endif %}
> + </div>
> + {% endif %}
> + </div>
> + <pre class="content">
> + {{ item|commentsyntax }}
> + </pre>
> </div>
> {% endfor %}
>
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index 3e6874a..ac9cb44 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -128,6 +128,7 @@ def patch_detail(request, project_id, msgid):
> patch.check_set.all().select_related('user'),
> )
> context['submission'] = patch
> + context['editable'] = editable
> context['patchform'] = form
> context['createbundleform'] = createbundleform
> context['project'] = patch.project
> --
> 2.32.0.554.ge1b32706d8-goog
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list