Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in

Issue 6815077: Press "d" to mark comment as done (Closed)

Can't Edit
Can't Publish+Mail
Start Review
1 year, 5 months ago by danakj
1 year, 4 months ago
Andi Albrecht, Andi
codereview-list_googlegroups.com, backer_chromium.org, piman


Press "d" to mark comment as done

Adding a new hotkey for replying to comments. Currently, you have to use
the mouse in order to reply "Done", or type it out in full. Now, you can press
"d" and it does the same as if you clicked "Done".

A demo is here: http://danakj-rietveld2.appspot.com/1

Patch Set 1 #

Patch Set 2 : Prevent double post, remove console.logs #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+91 lines, -33 lines) Patch
M static/script.js View 1 4 chunks +87 lines, -32 lines 2 comments Download
M templates/base.html View 1 chunk +3 lines, -0 lines 0 comments Download
M templates/inline_comment.html View 1 chunk +1 line, -1 line 0 comments Download


Total messages: 3
Please take a look, thanks.
1 year, 5 months ago #1
LGTM I've already changed the two style nits (see comments) when applying your change as ...
1 year, 5 months ago #2
1 year, 5 months ago #3
Thanks :)

On Sun, Nov 4, 2012 at 3:13 AM,  <albrecht.andi@gmail.com> wrote:
> I've already changed the two style nits (see comments) when applying
> your change as rev f36d73ca8476.
> This version is now live too. The JS file is cached, maybe you need to
> reload the page to get the newest version.
> Thanks for this patch!
> https://codereview.appspot.com/6815077/diff/3002/static/script.js
> File static/script.js (right):
> https://codereview.appspot.com/6815077/diff/3002/static/script.js#newcode1930
> static/script.js:1930: // Go through this tr and try responding to the
> last comment. The general
> I think that the comment about responding to the last comment belongs to
> the markAsDone function below.
> https://codereview.appspot.com/6815077/diff/3002/static/script.js#newcode1991
> static/script.js:1991: M_isElementVisible(this.win,
> hooks[this.hookPos].cells[0]));
> Tabs instead of spaces.
> https://codereview.appspot.com/6815077/
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5