Closed Bug 1246769 Opened 5 years ago Closed 5 years ago

Make double-click to comment a per-user setting

Categories

(MozReview Graveyard :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mconley, Unassigned)

References

Details

Attachments

(1 file)

In bug 1101601, we made it possible to comment on single-lines by double-clicking on them.

That originally landed in core Review Board, but got backed out once enough users started complaining that they couldn't double-click in order to select text.

We should probably make this feature a per-user setting, which should be pretty straight-forward.
For what it's worth this is probably the #1 UI annoyance I ran into with mozreview today.  It made a review take easily 2x as long as it should have; wasted at least 20-30 minutes on it.  :(
Comment on attachment 8718927 [details]
MozReview Request: mozreview: make double-clicking to open a comment optional per user (bug 1246769); r?mdoglio

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34805/diff/1-2/
Attachment #8718927 - Attachment description: MozReview Request: mozreview: [WIP] make double-clicking to open a comment optional per user (bug 1246769); r?mdoglio → MozReview Request: mozreview: make double-clicking to open a comment optional per user (bug 1246769); r?mdoglio
Attachment #8718927 - Flags: review?(mdoglio)
Duplicate of this bug: 1239905
Attachment #8718927 - Flags: review?(mdoglio) → review+
Comment on attachment 8718927 [details]
MozReview Request: mozreview: make double-clicking to open a comment optional per user (bug 1246769); r?mdoglio

https://reviewboard.mozilla.org/r/34805/#review31537

Thanks :mconley, this is awesome! Now I have a framework to save custom user settings :-)

::: pylib/mozreview/mozreview/templates/mozreview/review-scripts-js.html:11
(Diff revision 2)
> +<div id="mozreview-dblclick-comment" data-value="{{ user_profile|dblclick_comment|yesno:'true,false' }}"></div>

Can you please add a TODO comment refering to https://bugzilla.mozilla.org/show_bug.cgi?id=1248364 ? I will soon put all the invisible template fragments needed for the diff viewer in a separate template.
Comment on attachment 8718927 [details]
MozReview Request: mozreview: make double-clicking to open a comment optional per user (bug 1246769); r?mdoglio

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34805/diff/2-3/
Thank you, Mike!

I wonder whether this should default to off.
I get the feeling more people now are just quoting one line in review comments instead of the code that they are commenting on.
Priority: -- → P1
Comment on attachment 8718927 [details]
MozReview Request: mozreview: make double-clicking to open a comment optional per user (bug 1246769); r?mdoglio

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34805/diff/3-4/
This landed:

https://hg.mozilla.org/hgcustom/version-control-tools/rev/d6d96bde19fd

(In reply to Karl Tomlinson (ni?:karlt) from comment #7)
> Thank you, Mike!
> 
> I wonder whether this should default to off.
> I get the feeling more people now are just quoting one line in review
> comments instead of the code that they are commenting on.

I've filed bug 1249297 for if we decide to make this off by default.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
There was significant opposition to this feature in the last MozReview meeting, mainly because it is either annoying to some people if defaulted to on, or very undiscoverable if defaulted to off.

We're thinking that we should just back out the double-click thing altogether; it seems the pain caused by people who are trying to highlight a word is greater than those who expect double-click to work.

A better general solution would be just making the comment action more obvious.  We filed bug 1249647 about this, to use something like what GitHub does--a big button appears when you hover over a line.
Where should I look for this setting?

I don't see it in these locations:
https://reviewboard.mozilla.org/account/preferences/#settings
https://reviewboard.mozilla.org/account/preferences/#profile

Is there another bug that needs to be fixed before this setting is available?
(In reply to Karl Tomlinson (ni?:karlt) from comment #11)
> Where should I look for this setting?
> 
> I don't see it in these locations:
> https://reviewboard.mozilla.org/account/preferences/#settings
> https://reviewboard.mozilla.org/account/preferences/#profile
> 
> Is there another bug that needs to be fixed before this setting is available?

The latest version of the MozReview extension hasn't been pushed to production yet.

However, due to comment 10, I've started hacking on bug 1249297 to remove the double-click to comment feature entirely, which I hope to have pushed to production early next week.
(In reply to Boris Zbarsky [:bz] from comment #1)
> For what it's worth this is probably the #1 UI annoyance I ran into with
> mozreview today.  It made a review take easily 2x as long as it should have;
> wasted at least 20-30 minutes on it.  :(

Out of curiosity, how did this not bother you with Splinter?
(In reply to Botond Ballo [:botond] from comment #13)
> Out of curiosity, how did this not bother you with Splinter?

This was one of the main reasons I didn't use Splinter.  The other was that I didn't find a way to select or determine which changes would be quoted.  However, even Splinter doesn't suffer from bug 1239905.

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #12)
> The latest version of the MozReview extension hasn't been pushed to
> production yet.
> 
> However, due to comment 10, I've started hacking on bug 1249297 to remove
> the double-click to comment feature entirely, which I hope to have pushed to
> production early next week.

OK.  Thanks, Mike.  Just wanted to check I wasn't missing out unnecessarily.
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.