Kallithea issues archive

I think main reason I made it that way is to avoid using horizontal space on every line for something that usually not is relevant.

Design and implementation of a better way to do it would be appreciated.

To be clear: I'm not questioning the fact that it only displays when hovering the line. I'm just saying that when hovering the line number, the same behavior should happen. Do you know by heart where in the code this is handled?

The problem is that it is shown on top of the line numbers so we can't show both.

It was a minimal fix on top of the old behaviour where clicking (or highlighting) anywhere in the diff opened the comment box.

the current behaviour was implemented in 5f883a5be2d1 . Depending on exactly which alternative you would prefer, it might be possible to do it just by tweaking css.

Replacing the relative offset of -32px to 0 solves the main usability problem: hovering at the place where the button normally is works as expected. In the current state, starting hovering there (in the line number column) does not work, unless your mouse pointer comes from the right.

Only disadvantage of the offset 0 is that on code lines without indentation, the first letters are not visible on the hovered line. Not sure if that is a big problem though. For one, in the programming languages I know, it is very common for +90% of lines to be indented. Secondly, the visual collision only happens on the hovered line.

The alternative is to create an empty column, but you already indicated you wanted to avoid that.

What are your thoughts on the 0 relative offset?

I did intentionally put it outside the source area. I do not think offset 0 is an option. The source area is for source and it should be possible to select things there freely without anything getting in the way.

I think the "click to add comment" option somehow has to be out there to the left where we also have the line numbers. The actual line numbers are however rarely relevant, if only we easily can link to a specific line and also can see the actual line number when hovering. So we could perhaps do with just having shown-when-hovering icons for linking to line number and adding comment instead of the line numbers.

Alternatively, the 'add comment' option could perhaps show up overlapping with the source area when hovering over the line number. That would however be less explorable.

@kiilerix: with "having shown-when-hovering icons for linking to line number and adding comment instead of the line numbers.", do you mean to have a blank space, which is filled with two icons when a line is hovered? Line numbers itself would not be displayed at all? But what about the difference between old and new line numbers?

I assume that at least some people will expect line numbers to be shown, so leaving that out may be counter-intuitive.

I can also follow your reasoning that the code should not be crippled with an icon.

At this point, my feeling is that it would be more logical to provide a dedicated area for the icon. The icon would be visible when either hovering the code line, or hovering the icon area. The line number handling would be left untouched, and would be to the left of the code review icon area. The area to be provided is not very large. The font size of the icon is 14px.

(I really think such discussions would be better to have on the mailing list where more can chime on ... and where we can do proper quoting ;-) )

It must of course remain clear what is "context", "+" and "-" - especially for color blind. But I don't see much use for the actual line numbers when reviewing through the web. Just like most editors (presumably for good reasons) don't show line numbers next to each line but can show you (in a status line) what line you are at. The ordinary 'diff' format do fine without line numbers on each line ... and we are in a graphical browser and have the option of making something that is even better. I doubt that leaving the line numbers out would be surprising, counter-intuitive or a real problem.

I find it essential that I can mark code for cut'n'paste everywhere in the code window, without accidentally opening the 'comment' field. (Lesson learned from the annoying RhodeCode behaviour.) But I think it is OK that it overlays the code window once I have "activated" it somewhere outside the code area, either by clicking or hovering somewhere.

A bare 14px x 14px icon is very small and hard to click. That is why we in most places have a clickable area that is significantly bigger than the icon itself ... at least in one dimension (such as underlined text or clicking a line). Notice how big the current clickable area for the bubble is. I do thus not feel it is a good idea to dedicate a whole column to the icon - it would take up too much space for no good reason. (The line numbers do at least have something slightly relevant to show).

Hmm ... we could perhaps keep showing the line numbers, but for the current line replace them with 'link to this line' and 'comment on this line' links ...

I am having great trouble following this discussion on bitbucket. Could we talk about this on the mailing list instead?

(And I just noticed that I had misread and added a negation in "I can also follow your reasoning that the code should not be crippled with an icon." So my answer might be a response to something you didn't say ;-) )

I'm starting a mail thread for this, let's continue the discussion there...

css: add text +/- markers to the diff to improve readability for colour blind (fixes #77)

→ <<cset a06804c28d74>>