Deletion mark in gutter is partially covered when the mark's bottom row is selected
Closed this issue · 8 comments
In the GIF below, something was deleted between row 16 and 17, so the icon is shown. Notice how when I click in the gutter for row 16 the upper part of the triangle is still visible, but when I click in the gutter for row 17 the lower part of the triangle is no longer visible.
Atom 1.0.8-a87dd05 in safe mode, on OSX.
Some 📟: @simurai @as-cii @nathansobo
Tiling strikes again. Forcing synchronous scroll events may allow us to take a totally different approach and avoid the need for it.
Seems that some themes add a background to selected line numbers, which then covers the icon.
We could do
atom-text-editor::shadow .gutter .line-number.git-line-removed {
z-index: 1;
}But there is still the edge case of deleting every other line so that there are multiple removed icons after each other. Then only the last one wouldn't be cut off.
To also solve that:
Option (1): Would it be possible to reverse the order of the .line-numbers in the DOM? Like
3. Line
2. Line
1. Line
They would still be positioned ok, but just the stacking order would change so that the line after is shown on top of the previous one.
Another option (2) would be to move the .git-line-removed icon to the line below the removed line (and then positioned the icon to the top):
// instead of
Line ( ▶ has removed icon at the bottom)
---- Removed line
Line
// do
Line
---- Removed line
Line ( ▶ has removed icon at the top)
Or option (3): Make the removed icon its own div element that has a z-index: 1 and is above other line numbers. Maybe if a line gets deleted, the .line-number element could still be kept, just empty.
Line
---- Removed line ( ▶ has removed icon, still in DOM, empty, z-index:1)
Line
Tiling strikes again. Forcing synchronous scroll events may allow us to take a totally different approach and avoid the need for it.
@nathansobo: agreed that tiling is flawed for certain use cases, but this problem seems to be present not only at the boundaries between tiles, but also within a single tile.
@simurai: would the options you proposed work e.g. between two tiles? If yes, reversing line numbers in the DOM seems okay: similarly, would manually assigning an higher z-index to the rows below in the document do the trick? (the reason is that I'd prefer to avoid to deal with sorting, etc.).
agreed that tiling is flawed for certain use cases, but this problem seems to be present not only at the boundaries between tiles, but also within a single tile.
Yeah, it already happens if themes add a background color to selected lines, regardless of tiles.
would the options you proposed work e.g. between two tiles?
Sorry, I just realized that the order of the tiles in the DOM are not necessarily the same order as they visually appear. Initially they are, but when scrolling it could change. So I think option 1 + 2 wouldn't work (unless the DOM order or z-index could be somehow managed).
Option 3 should still work because it seems that tiles have 0 width. So the backgrounds of tiles are actually not seen and shouldn't cover the red icon.
Option 3 should still work because it seems that tiles have 0 width. So the backgrounds of tiles are actually not seen and shouldn't cover the red icon.
As far as I understand, Option 3 would require us to keep an extra element around, and I'd prefer to avoid to manage the lifetime of a node which solely exists for this graphical glitch.
Sorry, I just realized that the order of the tiles in the DOM are not necessarily the same order as they visually appear. Initially they are, but when scrolling it could change. So I think option 1 + 2 wouldn't work (unless the DOM order or z-index could be somehow managed).
We are already using a similar trick for tiles within .lines, so I'd be down to implement something similar for the gutter. I am not sure there is any other implication in doing so, but code-wise it should be quite straightforward.
@simurai: that said, what do you think if we proceed with Option 1?
As far as I understand, Option 3 would require us to keep an extra element around, and I'd prefer to avoid to manage the lifetime of a node which solely exists for this graphical glitch.
Fair enough. 😁
what do you think if we proceed with Option 1?
Yeah, I think the .line-numbers would need to be reversed in order + the .tiles would need a z-index that grows from bottom to top.
Option 4: If there are complications, the visual could also be changed to not overlap a .line-number. For example have a red vertical bottom border. Too big of a change?
I am reopening this because, for performance reasons, we couldn't exploit the z-index trick anymore (more details in atom/atom#8730) and this seemed quite minor compared to the gainings of that PR.
Quoting that thread, I'd be down to have the git deletion marker styled as a "red vertical bottom border" because:
[...] So long as two lines overlap (in this case, because of the git deletion maker crossing the line container) and one of them has an opaque background, we'll always face this kind of problem.
In other words, I feel like the most resilient (and straightforward) solution would be to attack this problem from a UI angle. If we want to keep the visual effect, the only long term solution I could think of would be to have an inline decoration, but that's a whole different abstraction which doesn't exist yet.
This issue was moved to atom/atom#17995