saros-project / saros

Open Source IDE plugin for distributed collaborative software development
https://www.saros-project.org
GNU General Public License v2.0
160 stars 52 forks source link

[UI][I] Add border to selection and contribution annotations #1128

Closed tobous closed 1 year ago

tobous commented 3 years ago

Adds a border to selection and contribution annotations by default. This border will be visible even when the rest of the annotation is overshadowed by other IDE range highlighters most of the time (as bordered highlighters are rare as far as I can tell).

Contribution annotations look a bit weird with the borders as they show that each added character is its own range highlighter and therefore will be displayed as its own border "box". This, however, will only be visible when the main annotation (the background color) is overshadowed (or if the user changes the border color).

This border can be disabled by the user in the editor color scheme settings for Saros if they don't like it.

Visual Comparison

Without Borders

Default contribution & selection annotations: annotation

Annotation on active line (i.e. the line in which the local caret it located): annotation-active-line

Annotation under selection: annotation-selected

With Borders

Default contribution & selection annotations: annotation-with-blocks

Annotations on active line: annotation-with-blocks-active-line

Annotation under selection: annotation-with-blocks-selected

Note Regarding the Active Line Highlighter

To explain the behavior on the active line in more details: IntelliJ IDEA highlights the line the local caret currently resides in. This helps figuring out where you are currently typing at a glance. As I find this feature helpful, I did not want Saros overshadowing it. Therefore, I put Saros annotations below the layer the active line is highlighted in with #1126. As a result, the default Saros contribution and selection annotations (displayed by a colored background) are overshadowed by the active line highlighter.

Drakulix commented 3 years ago

Note Regarding the Active Line Highlighter

To explain the behavior on the active line in more details: IntelliJ IDEA highlights the line the local caret currently resides in. This helps figuring out where you are currently typing at a glance. As I find this feature helpful, I did not want Saros overshadowing it. Therefore, I put Saros annotations below the layer the active line is highlighted in with #1126. As a result, the default Saros contribution and selection annotations (displayed by a colored background) are overshadowed by the active line highlighter.

So if I understand that correctly, in Saros/I the current line is never highlighted by Saros? Isn't that what the current behavior of Saros/E is as well?

I assumed you wanted Saros/E to match Saros/I here, so I might misunderstand something. Could you add some screenshots how Saros/I looks currently compared to this? I am in favor of providing a similar experience where possible.

That said, I personally think it is absolutely hideous and looks very busy, If our annotation were not one character long, it would probably look a lot better. I see value in the additional information, but maybe this should be an opt-in feature instead of opt-out?

tobous commented 3 years ago

So if I understand that correctly, in Saros/I the current line is never highlighted by Saros? Isn't that what the current behavior of Saros/E is as well?

No, in Saros/E, the Saros annotations overshadow the highlighter of the current line. But there, it isn't as much of an issue. I don't know whether it is an inherent difference between Eclipse and IntelliJ IDEA or whether it is due to the specific Saros/E annotation implementation, but for Saros/E, empty space in the editor is not highlighted.

You can't see the difference in the screenshots as they are cut of to the right, but for Saros/I, adding a newline causes the entire line to be marked with a contribution annotation. As a result, the highlighter for the current line is completely overshadowed. As Saros/E only highlights the actual characters and not the rest of the line after the newline, at least part of the current line highlighter is still visible (except for lines that span the entire screen).

I assumed you wanted Saros/E to match Saros/I here, so I might misunderstand something. Could you add some screenshots how Saros/I looks currently compared to this? I am in favor of providing a similar experience where possible.

The case "Without Border" is how Saros/I looks currently.

That said, I personally think it is absolutely hideous and looks very busy, If our annotation were not one character long, it would probably look a lot better. I see value in the additional information, but maybe this should be an opt-in feature instead of opt-out?

Merging annotations of neighboring annotations is too much of a hassle in my opinion. Maintaining the different annotations blocks requires a lot of complicated logic and has to cover many corner cases. The previous Saros/E implementation did so, but it never worked correctly. As a result, Kelvin also replaced it with single-character annotations at some point.

Without this PR, the option is technically 'opt-in' as the user can modify the highlighter settings however they want. But I don't think most user know about the specifics of how highlighter overshadowing works in IntelliJ IDEA. Adding in a specific option to set this highlighter setup would require a lot of additional work and is not worth it in my opinion. But I could add a section in the documentation specifying how to set this up in the options.

Drakulix commented 3 years ago

No, in Saros/E, the Saros annotations overshadow the highlighter of the current line. But there, it isn't as much of an issue. I don't know whether it is an inherent difference between Eclipse and IntelliJ IDEA or whether it is due to the specific Saros/E annotation implementation, but for Saros/E, empty space in the editor is not highlighted. You can't see the difference in the screenshots as they are cut of to the right, but for Saros/I, adding a newline causes the entire line to be marked with a contribution annotation. As a result, the highlighter for the current line is completely overshadowed. As Saros/E only highlights the actual characters and not the rest of the line after the newline, at least part of the current line highlighter is still visible (except for lines that span the entire screen).

Alright, that is a big difference, noted.

Without this PR, the option is technically 'opt-in' as the user can modify the highlighter settings however they want. But I don't think most user know about the specifics of how highlighter overshadowing works in IntelliJ IDEA. Adding in a specific option to set this highlighter setup would require a lot of additional work and is not worth it in my opinion. But I could add a section in the documentation specifying how to set this up in the options.

In that case, I am in favor of adding the borders, but with clear instructions somewhere how to turn them off.