[PATCH v4 8/9] patch-detail: add label and button for comment addressed status

Daniel Axtens dja at axtens.net
Mon Aug 23 22:13:30 AEST 2021


Raxel Gutierrez <raxel at google.com> writes:

> Add new label to patch and cover comments to show the status of whether
> they are addressed or not and add an adjacent button to allow users to
> change the status of the comment. Only users that can edit the patch
> (i.e. patch author, delegate, project maintainers) as well as comment
> authors can change the status of a patch comment. For cover comments,
> there are no delegates, so only maintainers and cover/cover comment
> authors can edit the status of the cover comment. Before [1] and after
> [2] images for reference.
>
> Use new comment detail REST API endpoint to update the addressed field
> value when "Addressed" or "Unaddressed" buttons are clicked. After a
> successful request is made, the appearance of the comment status label
> and buttons are toggled appropriately. For unsuccessful requests (e.g.
> network errors prevents reaching the server), the error message is
> populated to the page. A future improvement on this behavior is to add
> a spinner to the button to provide a feedback that the request is in a
> pending state until it's handled.

My only complaint with the current design is by the time I've scrolled
down far enough to see the comments, I can no longer see the top bar
where the messages appear. But hopefully we can sort that out later.

>
> [1] https://imgur.com/3ZKzgjN
> [2] https://imgur.com/hWZrrnM
>
> Signed-off-by: Raxel Gutierrez <raxel at google.com>
> ---
>  htdocs/css/style.css                          | 38 ++++++++++++
>  htdocs/js/submission.js                       | 20 +++++++
>  patchwork/templates/patchwork/submission.html | 60 +++++++++++++++----
>  patchwork/views/patch.py                      |  4 +-
>  4 files changed, 110 insertions(+), 12 deletions(-)
>
> diff --git a/htdocs/css/style.css b/htdocs/css/style.css
> index a2a2e3c3..9156aa6e 100644
> --- a/htdocs/css/style.css
> +++ b/htdocs/css/style.css
> @@ -1,4 +1,5 @@
>  :root {
> +    --light-color:rgb(247, 247, 247);
>      --success-color:rgb(92, 184, 92);
>      --warning-color:rgb(240, 173, 78);
>      --danger-color:rgb(217, 83, 79);
> @@ -27,6 +28,10 @@ pre {
>      top: 17em;
>  }
>  
> +.hidden {
> +    visibility: hidden;
> +}
> +
>  /* Bootstrap overrides */
>  
>  .navbar-inverse .navbar-brand > a {
> @@ -315,6 +320,39 @@ table.patch-meta tr th, table.patch-meta tr td {
>      font-family: "DejaVu Sans Mono", fixed;
>  }
>  
> +div[class^="comment-status-bar-"] {
> +    display: flex;
> +    flex-wrap: wrap;
> +    align-items: center;
> +}
> +
> +.comment-status-label {
> +    margin: 0px 8px;
> +}
> +
> +button[class^=comment-action] {
> +    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 {
> +    background-color: var(--success-color);
> +    color: var(--light-color);
> +}
> +
> +.comment-action-unaddressed:hover {
> +    background-color: var(--warning-color);
> +    color: var(--light-color);
> +}
> +
>  .quote {
>      color: #007f00;
>  }
> diff --git a/htdocs/js/submission.js b/htdocs/js/submission.js
> index 9676f348..47cffc82 100644
> --- a/htdocs/js/submission.js
> +++ b/htdocs/js/submission.js
> @@ -1,4 +1,7 @@
> +import { updateProperty } from "./rest.js";
> +
>  $( document ).ready(function() {
> +    const patchMeta = document.getElementById("patch-meta");
>      function toggleDiv(link_id, headers_id, label_show, label_hide) {
>          const link = document.getElementById(link_id)
>          const headers = document.getElementById(headers_id)
> @@ -14,6 +17,23 @@ $( document ).ready(function() {
>          }
>      }
>  
> +    $("button[class^='comment-action']").click((event) => {
> +        const submissionType = patchMeta.dataset.submissionType;
> +        const submissionId = patchMeta.dataset.submissionId;
> +        const commentId = event.target.parentElement.dataset.commentId;
> +        const url = `/api/${submissionType}/${submissionId}/comments/${commentId}/`;
> +        const data = {'addressed': event.target.value} ;
> +        const updateMessage = {
> +            'error': "No comments updated",
> +            'success': "1 comment(s) updated",
> +        };
> +        updateProperty(url, data, updateMessage).then(isSuccess => {
> +            if (isSuccess) {
> +                $("div[class^='comment-status-bar-'][data-comment-id='"+commentId+"']").toggleClass("hidden");
> +            }
> +        })
> +    });
> +
>      // Click listener to show/hide headers
>      document.getElementById("toggle-patch-headers").addEventListener("click", function() {
>          toggleDiv("toggle-patch-headers", "patch-headers");
> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
> index 36b15d0e..2238e82e 100644
> --- a/patchwork/templates/patchwork/submission.html
> +++ b/patchwork/templates/patchwork/submission.html
> @@ -5,6 +5,7 @@
>  {% load person %}
>  {% load patch %}
>  {% load static %}
> +{% load utils %}
>  
>  {% block headers %}
>    <script type="module" src="{% static "js/submission.js" %}"></script>
> @@ -19,7 +20,12 @@
>    <h1>{{ submission.name }}</h1>
>  </div>
>  
> -<table class="patch-meta">
> +<table
> +  id="patch-meta"
> +  class="patch-meta"
> +  data-submission-type={{submission|verbose_name_plural|lower}}
> +  data-submission-id={{submission.id}}
> +>
>   <tr>
>    <th>Message ID</th>
>    {% if submission.list_archive_url %}
> @@ -271,18 +277,50 @@
>  {% if forloop.first %}
>  <h2>Comments</h2>
>  {% endif %}
> -
> +{% is_editable item user as comment_is_editable %}
>  <a name="{{ item.id }}"></a>
>  <div class="submission-message">
> -<div class="meta">
> - <span>{{ item.submitter|personify:project }}</span>
> - <span class="message-date">{{ 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="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 class="comment-status-bar-addressed" data-comment-id={{item.id}}>
> +    {% else %}
> +      <div class="comment-status-bar-addressed hidden" data-comment-id={{item.id}}>
> +    {% endif %}
> +        <div class="comment-status-label text-success mx-3">
> +          <span class="glyphicon glyphicon-ok-circle" aria-hidden="true"></span>
> +          Addressed
> +        </div>
> +        {% if editable or comment_is_editable %}
> +          <button class="comment-action-unaddressed text-warning" value="false">
> +            <span class="glyphicon glyphicon-warning-sign" aria-hidden="true"></span>
> +            Mark Unaddressed
> +          </button>
> +        {% endif %}
> +      </div>
> +    {% if item.addressed %}
> +      <div class="comment-status-bar-unaddressed hidden" data-comment-id={{item.id}}>
> +    {% else %}
> +      <div class="comment-status-bar-unaddressed" data-comment-id={{item.id}}>
> +    {% endif %}
> +        <div class="comment-status-label text-warning mx-3">
> +          <span class="glyphicon glyphicon-warning-sign" aria-hidden="true"></span>
> +          Unaddressed
> +        </div>
> +        {% if editable or comment_is_editable %}
> +          <button class="comment-action-addressed text-success" value="true">
> +            <span class="glyphicon glyphicon-ok-circle" aria-hidden="true"></span>
> +            Mark Addressed
> +          </button>
> +        {% endif %}
> +      </div>
> +  </div>
> +  <pre class="content">
> +  {{ item|commentsyntax }}
> +  </pre>
>  </div>
>  {% endfor %}
>  
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index 3e6874ae..00b0147f 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -109,7 +109,8 @@ def patch_detail(request, project_id, msgid):
>  
>      comments = patch.comments.all()
>      comments = comments.select_related('submitter')
> -    comments = comments.only('submitter', 'date', 'id', 'content', 'patch')
> +    comments = comments.only('submitter', 'date', 'id', 'content', 'patch',
> +                             'addressed')
>  
>      if patch.related:
>          related_same_project = patch.related.patches.only(
> @@ -128,6 +129,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.33.0.rc2.250.ged5fa647cd-goog
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork


More information about the Patchwork mailing list