launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Pushed the requested changes. This should be good to go now.

Diff comments:

> diff --git a/lib/lp/answers/browser/question.py b/lib/lp/answers/browser/question.py
> index 9d4986d..52cff40 100644
> --- a/lib/lp/answers/browser/question.py
> +++ b/lib/lp/answers/browser/question.py
> @@ -1195,13 +1195,11 @@ class QuestionMessageDisplayView(LaunchpadView):
>          return check_permission('launchpad.Moderate', self.context)
>  
>      def getBoardCommentCSSClass(self):
> -        css_classes = ["boardComment"]
> +        css_classes = ["boardComment", "editable-message"]
>          if not self.context.visible:
>              # If a comment that isn't visible is being rendered, it's being
>              # rendered for an admin or registry_expert.
>              css_classes.append("adminHiddenComment")
> -        if self.can_edit:
> -            css_classes.append("editable-message")

The displaying of the pencil icon is already guarded by `tal:condition="view/can_edit"`.

We need to add `editable-message` class on every message so we can add the link to the revision history list. Everyone is able to see such list, regardless on having or not permission to edit the message itself.

>          return " ".join(css_classes)
>  
>      @property
> diff --git a/lib/lp/services/messages/javascript/messages.edit.js b/lib/lp/services/messages/javascript/messages.edit.js
> index 3c4c0e5..912a24c 100644
> --- a/lib/lp/services/messages/javascript/messages.edit.js
> +++ b/lib/lp/services/messages/javascript/messages.edit.js
> @@ -168,6 +179,74 @@ YUI.add('lp.services.messages.edit', function(Y) {
>          elements.update_btn.getDOMNode().disabled = false;
>      };
>  
> +    module.fillMessageRevisions = function(elements, revisions) {
> +        // Position the message revision list element.
> +        revisions = revisions.reverse();
> +        var revisions_container = elements.container.one(
> +            ".message-revision-container");
> +        var last_edit_el = elements.last_edit.getDOMNode();
> +        var target_position = last_edit_el.getBoundingClientRect();
> +        var nodes_holder = revisions_container.one(".message-revision-list");
> +        var template = revisions_container.one(
> +            "script[type='text/template']").getDOMNode().innerHTML;
> +
> +        revisions_container.setStyle('left', target_position.left);
> +        revisions_container.setStyle('display', 'block');
> +        revisions_container.one(".message-revision-close").on(
> +            "click", function() {
> +                nodes_holder.getDOMNode().innerHTML = '';
> +                revisions_container.setStyle('display', 'none');
> +            });
> +
> +        var content = "";
> +        revisions.forEach(function(rev) {
> +            var attrs = rev.getAttrs();
> +            var date_created = new Date(attrs.date_created);
> +            attrs.date_created = (
> +                date_created.toISOString().substr(0,10) + ", " +
> +                date_created.toLocaleTimeString());

It makes sense. I'll use toLocaleString here.

I thought also about implementing something in JS to display it as a relative date (like 'x days ago', 'y months ago', etc), but I guess this can go in another MP.

> +            attrs.content = module.htmlify_msg(attrs.content);
> +            content += Y.Lang.sub(template, attrs);
> +        });
> +
> +        nodes_holder.getDOMNode().innerHTML = content;
> +        nodes_holder.all(".message-revision-item").each(function(rev_item) {
> +            rev_item.one(".message-revision-title").on('click', function() {
> +                nodes_holder.all('.message-revision-body').setStyle(
> +                    'display', 'none');
> +                var body = rev_item.one('.message-revision-body');
> +                var current_display = body.getStyle('display');
> +                body.setStyle(
> +                    'display', current_display === 'block'? 'none' : 'block');
> +                nodes_holder.all(".message-revision-item").removeClass(
> +                    'active');
> +                rev_item.addClass('active');
> +            });
> +        });
> +    };
> +
> +    module.onLastEditClick = function(elements, baseurl) {
> +        // Hide all open revision containers.
> +        Y.all('.message-revision-container').each(function(container) {
> +            container.setStyle('display', 'none');
> +        });
> +
> +        var url = "/api/devel" + baseurl + "/revisions";
> +        var config = {
> +            on: {
> +                 success: function(response) {
> +                    module.fillMessageRevisions(elements, response.entries);
> +                 },
> +                 failure: function(err) {
> +                    alert("Error fetching revisions.");
> +                 }
> +             },
> +             // XXX pappacena 2021-05-19: Pagination will be needed here.
> +             size: 100
> +        };
> +        this.lp_client.get(url, config);
> +    };
> +
>      module.wireEventHandlers = function(container) {
>          var node = container.getDOMNode();
>          var baseurl = node.dataset.baseurl;


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403213
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-ui-list-revisions.