ericmckean / rietveld

Automatically exported from code.google.com/p/rietveld
Apache License 2.0
0 stars 0 forks source link

Arrow scroll has disconcerting jump #440

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Enter a multi-line comment.
2. Use the down-arrow to scroll the blue line past the commment.

What is the expected output? What do you see instead?

The line [and therefore the whole page] jumps when it hits the comment.  Normal 
arrow-based scrolling on a web page always moves the page by the same amount.

What browser are you using?  What version? On what operating system?
Version 27.0.1453.47 beta on Debian Wheezy.

At what URL are you accessing Rietveld?  codereview.chromium.org

*** If you are a Google employee please say so or mail rietveld-admins@
directly. ***

Yep, I'm at Google.  Same guy who logged 382, 398, 439.  Apparently I'm your 
only arrow-key user ;'>.

Please provide any additional information below.

This is yet more fallout from issue 382.  Any chance you could just drop the 
whole misfeature?  Did anybody actually request this functionality?

Original issue reported on code.google.com by ericu@chromium.org on 18 Apr 2013 at 1:46

GoogleCodeExporter commented 9 years ago
Just hit this today, and even worse: when you have a whole series of 
comment-and-reply-comment stuff together, this skips over it completely.  If 
it's longer than a page, that means there's no way to scroll through it via 
arrow keys.  Very annoying.

Original comment by ericu@chromium.org on 24 Apr 2013 at 11:14

GoogleCodeExporter commented 9 years ago
I was against this arrow feature too. I believe the Guido is the main 
proponent. Probably because he knows how it should work. I don't. If somebody 
can show me a video of how it should behave, I can think about how to deal with 
that.

Original comment by techtonik@gmail.com on 25 Apr 2013 at 4:11

GoogleCodeExporter commented 9 years ago
I don't know if I'm the only proponent.  IIRC Mondrian at Google worked the 
same way and over the 6 years it was in use I don't recall getting significant 
complaints.

I can imagine there might be a way to fix it, but I'm no JavaScript wizard, so 
I don't know.  The *import* UI feature that this enables, BTW, is complete 
keyboard-driven entering of review comments -- just hit down until the blue 
line is at the line where you want to enter a comment, hit Enter to open a new 
comment dialog, type away, and hit control-Enter to save.

If you can implement a way to preserve this UI feature while making the 
scrolling smoother, I'm all for it, but so far this bug is just whining.

Original comment by gvanrossum@gmail.com on 25 Apr 2013 at 4:22

GoogleCodeExporter commented 9 years ago
1. Does Mondrian also jump above/after the comments?
2. Does the position of line on screen surface is always constant in Mondrian 
screen during scrolling?

The main showstopper for any further JavaScript fixes is that all code is made 
with custom JavaScript - no jQuery or any other advanced tools makes this work 
really unpleasant.

Original comment by techtonik@gmail.com on 25 Apr 2013 at 4:27

GoogleCodeExporter commented 9 years ago
I voted for this issue too and implemented it. And for sure we can work on 
sorting the issues out. Eventually it's just the easiest solution to add an 
option to the user settings to enable/disable this feature in the UI since not 
all users seem to feel comfortable with it.

techtonik, the JavaScript code is pretty well organized, understandable and 
stable for years now. I really don't understand why you call it a showstopper 
just because it doesn't use a library.

Original comment by albrecht.andi on 25 Apr 2013 at 5:43

GoogleCodeExporter commented 9 years ago
Guido:

I don't know if Mondrian worked the same way.  I haven't used Mondrian in 
years, but I don't recall them breaking scrolling.

I just tried the current internal tool, and it does override arrows for 
scrolling.  However, it doesn't have the other bugs that make this 
implementation of the feature so annoying: resetting the arrow-scroll position 
to the top of the page after every comment and failing to warp the 
scroll-position into view when you hit an arrow key.  Perhaps that's why you 
don't recall getting bug reports?

I do see the value of the feature, now that you've explained it, although I 
don't know if I'll use it.  I do like to drive entirely by keyboard when I can, 
which is ironically why this change messes with my workflow so much.  I just 
drive my keyboard differently than you drive yours, I guess.

Andi:

Thanks for your response.  I'm loathe to suggest that you add a setting; so few 
users actually use them that they're often a waste of dev time.  Is there any 
reason that it has to override the arrow keys?  They've already got a 
well-known function, and if you'd chosen e.g. ',' and '.' for your new 
comment-skipping-slow-scroll-from-the-top instead of the standard scroll keys, 
I'd never even have noticed the problems.

Alternatively, if you make the scroll position warp into view whenever an arrow 
key is hit [just like you do for 'n' and 'p', and like the internal review tool 
does] and make it stop on each line of a comment instead of skipping over them, 
it'll at least be good enough that I'll stop logging bugs for a while ;'>.

But I still think the cleaner solution is to stop breaking built-in keyboard 
shortcuts.  You picked new ones for 'p' and 'n', rather than use the arrows 
there, and this is analogous.  Your arrow keys currently jump oddly, scroll at 
the wrong speed [compared to that on other web pages], sometimes mysteriously 
fail to work [when the cursor's off-screen]...it's really off-putting.  But as 
I said, I'll take a partial solution if that'll make other users happy.

Original comment by ericu@chromium.org on 26 Apr 2013 at 10:47

GoogleCodeExporter commented 9 years ago
Andi, there is nothing wrong existing code. It is maintainable and works, but 
modification takes more time than it is currently acceptable for JS projects. 
There are now more advanced methods of DOM query thanks to jQuery selectors. 
Effects and transition easily made by UI libraries. Libraries like AngularJS 
even allow to do complex UI logic in declarative manner. There are also ways to 
define namespaces for JavaScript functionality and make it modular and 
pluggable. There is also CoffeeScript which makes JS syntax less verbose.

I'd also implement an option (turned off by default) until the cursor follows 
scrolled position, not the other way around like it is now. But I will not have 
time to implement that myself in plain JavaScript.

Original comment by techtonik@gmail.com on 27 Apr 2013 at 6:00