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
3 years ago by danakj
3 years 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.
3 years ago (2012-11-03 16:35:56 UTC) #1
LGTM I've already changed the two style nits (see comments) when applying your change as ...
3 years ago (2012-11-04 08:13:57 UTC) #2
3 years ago (2012-11-04 18:19:44 UTC) #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 80a51fa-tainted