[PATCH 6 of 7] frontend: change comment workflow
Mads Kiilerich
mads at kiilerich.com
Fri Apr 24 13:21:28 EDT 2015
On 04/14/2015 06:03 PM, Jan Heylen wrote:
> # HG changeset patch
> # User Jan Heylen <heyleke at gmail.com>
> # Date 1428986395 -7200
> # Tue Apr 14 06:39:55 2015 +0200
> # Node ID b1b3c859230ce5997f7820788dd331748e277525
> # Parent 48a64173a2187e3ea2822df54b3d0096176e76e3
> frontend: change comment workflow
>
> This changes the workflow from one-comment-at-a-time to giving-draft-comments-and-a-final-judgement.
> Only after the final judgement, the drafts are changed into comments and an email notification is send.
> The internal notification model is untouched.
>
> diff -r 48a64173a218 -r b1b3c859230c kallithea/config/routing.py
> --- a/kallithea/config/routing.py Sat Mar 28 08:12:49 2015 +0100
> +++ b/kallithea/config/routing.py Tue Apr 14 06:39:55 2015 +0200
> @@ -662,6 +662,16 @@
> controller='changeset', action='preview_comment',
> conditions=dict(function=check_repo, method=["POST"]))
>
> + rmap.connect('changeset_comment_draft',
> + '/{repo_name:.*?}/changeset-comment-draft/{revision}',
> + controller='changeset', revision='tip', action='draft',
> + conditions=dict(function=check_repo, method=["POST"]))
> +
> + rmap.connect('pullrequest_comment_draft',
> + '/{repo_name:.*?}/pullrequest-comment-draft/{pull_request_id}',
> + controller='changeset', action='draft',
> + conditions=dict(function=check_repo, method=["POST"]))
It could make sense to hook these up when they are implemented on the
server side, leaving this changeset to be a client side change.
> +
> rmap.connect('changeset_comment_delete',
> '/{repo_name:.*?}/changeset-comment-delete/{comment_id}',
> controller='changeset', action='delete_comment',
> diff -r 48a64173a218 -r b1b3c859230c kallithea/public/js/base.js
> --- a/kallithea/public/js/base.js Sat Mar 28 08:12:49 2015 +0100
> +++ b/kallithea/public/js/base.js Tue Apr 14 06:39:55 2015 +0200
> @@ -706,6 +706,7 @@
> $('#edit-btn_'+lineno).show();
> $('#preview-container_'+lineno).show();
> $('#preview-btn_'+lineno).hide();
> + $('#draft-btn_'+lineno).hide();
>
> var url = pyroutes.url('changeset_comment_preview', {'repo_name': REPO_NAME});
> var post_data = {'text': text};
> @@ -714,11 +715,37 @@
> $('#preview-box_'+lineno).removeClass('unloaded');
> })
> })
> + $('#draft-btn_'+lineno).click(function(e){
> + var text = $('#text_'+lineno).val();
> + if(!text){
> + return
> + }
> + $('#preview-box_'+lineno).addClass('unloaded');
> + $('#preview-box_'+lineno).html(_TM['Loading ...']);
> + $('#edit-container_'+lineno).hide();
> + $('#edit-btn_'+lineno).show();
> + $('#preview-container_'+lineno).show();
> + $('#draft-btn_'+lineno).hide();
> + $('#preview-btn_'+lineno).hide();
> +
> + var url = AJAX_DRAFT_URL;
> + var post_data = {
> + 'text': text,
> + 'f_path': f_path,
> + 'line': lineno
> + };
> + ajaxPOST(url, post_data, function(json_data){
> + $tr.removeClass('form-open');
> + $form.remove();
> + _renderInlineComment(json_data);
> + })
> + })
> $('#edit-btn_'+lineno).click(function(e){
> $('#edit-container_'+lineno).show();
> $('#edit-btn_'+lineno).hide();
> $('#preview-container_'+lineno).hide();
> $('#preview-btn_'+lineno).show();
> + $('#draft-btn_'+lineno).show();
> })
>
> setTimeout(function(){
> diff -r 48a64173a218 -r b1b3c859230c kallithea/templates/changeset/changeset.html
> --- a/kallithea/templates/changeset/changeset.html Sat Mar 28 08:12:49 2015 +0100
> +++ b/kallithea/templates/changeset/changeset.html Tue Apr 14 06:39:55 2015 +0200
> @@ -28,6 +28,7 @@
> var _GROUPS_AC_DATA = ${c.user_groups_array|n};
> AJAX_COMMENT_URL = "${url('changeset_comment',repo_name=c.repo_name,revision=c.changeset.raw_id)}";
> AJAX_COMMENT_DELETE_URL = "${url('changeset_comment_delete',repo_name=c.repo_name,comment_id='__COMMENT_ID__')}";
> + AJAX_DRAFT_URL = "${url('changeset_comment_draft',repo_name=c.repo_name,revision=c.changeset.raw_id)}";
Many things could be draft. It would be nice if the name made it clear
it was a draft comment.
> </script>
> <div class="table">
> <div class="diffblock">
> diff -r 48a64173a218 -r b1b3c859230c kallithea/templates/changeset/changeset_file_comment.html
> --- a/kallithea/templates/changeset/changeset_file_comment.html Sat Mar 28 08:12:49 2015 +0100
> +++ b/kallithea/templates/changeset/changeset_file_comment.html Tue Apr 14 06:39:55 2015 +0200
> @@ -24,14 +24,22 @@
> ${_('Status change from pull request')}
> <a href="${co.pull_request.url()}">"${co.pull_request.title or _("No title")}"</a>:
> %else:
> - ${_('Comment from pull request')}
> + %if co.draft:
> + ${_('Draft from pull request')}
./whitespacecleanup.sh will fix the indentation here ;-)
Anyway, I guess this touches upon how wrong the datamodel is with PR
comments being duplicated for each changeset. It is sad to do that
already for the draft comments which probably are likely to change more
often ... and be draft. I guess it is hard to make it any cleaner
without changing the datamodel. Which is why I would like to get the
datamodel cleaned up before making it worse by building more on top of it.
> + %else:
> + ${_('Comment from pull request')}
> + %endif
> <a href="${co.pull_request.url()}">"${co.pull_request.title or _("No title")}"</a>
> %endif
> %else:
> %if co.status_change:
> ${_('Status change on changeset')}:
> %else:
> - ${_('Comment on changeset')}
> + %if co.draft:
> + ${_('Draft on changeset')}
> + %else:
> + ${_('Comment on changeset')}
> + %endif
> %endif
> %endif
> </span>
> @@ -85,8 +93,9 @@
> <div class="submitting-overlay">${_('Submitting ...')}</div>
> <input type="hidden" name="f_path" value="{0}">
> <input type="hidden" name="line" value="{1}">
> - ${h.submit('save', _('Comment'), class_='btn btn-small save-inline-form')}
> + <!-- ${h.submit('save', _('Comment'), class_='btn btn-small save-inline-form')} -->
???
> ${h.reset('hide-inline-form', _('Cancel'), class_='btn btn-small hide-inline-form')}
> + <div id="draft-btn_{1}" class="draft-btn btn btn-small">${_('Save')}</div>
> <div id="preview-btn_{1}" class="preview-btn btn btn-small">${_('Preview')}</div>
> <div id="edit-btn_{1}" class="edit-btn btn btn-small" style="display:none">${_('Edit')}</div>
> </div>
> @@ -132,6 +141,16 @@
> </div>
> %endfor
> %endfor
> + %for path, lines in c.drafts:
> + % for line,comments in lines.iteritems():
> + <div style="display:none" class="inline-comment-placeholder" path="${path}" target_id="${h.safeid(h.safe_unicode(path))}">
> + %for co in comments:
> + ${comment_block(co)}
> + %endfor
> + </div>
> + %endfor
> + %endfor
> +
>
> </%def>
>
> @@ -144,7 +163,11 @@
> </div>
>
> %for co in c.comments:
> + %if co.draft:
> + <div id="draft-tr-${co.comment_id}">
> + %else:
> <div id="comment-tr-${co.comment_id}">
> + %endif
> ${comment_block(co)}
> </div>
> %endfor
> @@ -202,7 +225,7 @@
> </div>
>
> <div class="comment-button">
> - ${h.submit('save', _('Comment'), class_="btn")}
> + ${h.submit('save', _('Commit all Comments'), class_="btn")}
Is "commit" the right term to use?
How do you describe this whole functionality? How to do you talk about
it or explain it to users? I guess I would say that "your comments will
be draft until you save them".
Somewhat related: Should there also be a "save as draft" option? I don't
think so. I think it should save as draft regularly and when leaving the
input fields.
Or ... perhaps the inline save button should be renamed 'Save Draft' ...
> <div id="preview-btn" class="preview-btn btn">${_('Preview')}</div>
> <div id="edit-btn" class="edit-btn btn" style="display:none">${_('Edit')}</div>
> </div>
> @@ -239,7 +262,7 @@
> }
> var post_data = {'text': _text};
> $('#preview-box').addClass('unloaded');
> - $('#preivew-box').html(_TM['Loading ...']);
> + $('#preview-box').html(_TM['Loading ...']);
> $('#edit-container').hide();
> $('#edit-btn').show();
> $('#preview-container').show();
> diff -r 48a64173a218 -r b1b3c859230c kallithea/templates/pullrequests/pullrequest_show.html
> --- a/kallithea/templates/pullrequests/pullrequest_show.html Sat Mar 28 08:12:49 2015 +0100
> +++ b/kallithea/templates/pullrequests/pullrequest_show.html Tue Apr 14 06:39:55 2015 +0200
> @@ -22,7 +22,11 @@
> <div class="title">
> ${self.breadcrumbs()}
> </div>
> -
> + <script>
> + AJAX_COMMENT_URL = "${url('changeset_comment',repo_name=c.repo_name,pull_request=c.pull_request.pull_request_id)}";
> + AJAX_COMMENT_DELETE_URL = "${url('changeset_comment_delete',repo_name=c.repo_name,comment_id='__COMMENT_ID__')}";
> + AJAX_DRAFT_URL = "${url('pullrequest_comment_draft',repo_name=c.repo_name,pull_request_id=c.pull_request.pull_request_id)}";
Why are these lines suddenly needed here?
> + </script>
> ${h.form(url('pullrequest_post', repo_name=c.repo_name, pull_request_id=c.pull_request.pull_request_id), method='post', id='pull_request_form')}
> <div class="form pr-box" style="float: left">
> <div class="pr-details-title ${'closed' if c.pull_request.is_closed() else ''}">
/Mads
More information about the kallithea-general
mailing list