Tweak Git blame UI/UX
Created by: philipp-spiess
Closes #41314 (closed) Closes #38377 (closed)
This PR tweaks the UI/UX of the newly added migrated Git blame extension based on @jjinnii's feedback. I posted some comments on the original issue but will re-iterate them here as well:
⚠ ️ "Git blame column with Codecov line decorations line background on hover conflict (Loom). This kind of conflict may happen with any extension which changes line colors. Customers with enableLegacyExtensions experimental feature flag enabled are affected." As this only impacts customers with the legacy extensions enabled and have to enable both CodeCov and blame, I think a minimal effort solution is best here. I think your first suggestion that blame highlight takes precedence over other extensions is sufficient, as it's our top-used extension based on our current data.
Just to clarify with that you mean the behavior that is already shown in Tara's video?
I suppose this is already how it works (because of the video) but I have a hard time checking this since I don't really understand how to enable the codecov extension. Do you have it running by any chance @jjinnii and can test this on dot-com?
TL;DR: I think we don't need to make any changes but this needs verification.
✅ "There is a gap between the blame decoration and the popover. When there are multiple lines in a row with decorations and the user hovers over one of them and then tries to hover over the popover, they should move the cursor over the code area, but not over the blame column, as the previous line decoration will trigger opening its popover. Moving the cursor over the code area may trigger showing the code-intel popover, so we may end up with two popovers (Loom)." This is less an edge case, but poor UX of how the blame popover interacts with the popover. I'm seeing that there's a latency in the blame popover disappearing when a user's cursor has moved outside of the blame column, which is why there's an overlap with the code intel popover. Is it possible for us to instantly close the blame popover if a user's cursor moves outside the blame column?
I don't think this would be a good fix UX wise. If you look at Tara's video again, all he's doing is to try and click the button on the popover which causes his mouse to move a bit to the center of the popover. I don't think it would be intended in this case to close the git blame popover in favor of code intel.
I've played around with this feature now as well and what's really unexpected for me is that when you are not super fast, moving the mouse to the content of the bit blame window will hover over another line and thus change the git blame info to the one on the line above/below.
I think what is really unexpected is that somehow if you hover over a new blame column, the popup will switch instantly but if you hover from a blame column to the code to show a code intel column, this is not the case.
However, I think this is less of an issue after one of the other changes below that changes the position of the git blame popup. Since the popup is now no longer shifted to the right, you don't need to move the mouse outside the blame column but simply upwards/downwards which prevents any interferences with the code intel popup.
TL;DR: I recommend we keep this as-is since a behavior change here is not trivial and the changed horizontal offset of the popover patches over the most noticeable issue when moving the mouse outside the blame column
✅ The blame popover still looks large when used alongside the code intel popover. Having trouble inspecting the font sizes of the popover using the inspect tool☹ ️ , could someone double-check the styling?
I fixed this as good as possible. I needed to make changes to the paddings and the content text size. See the screnshot below.
@jjinnii Here's a trick that I use to make the inspect tool work with popovers. Paste the following snipped into the console:
setTimeout(() => { debugger; }, 2000);After pressing enter, you have two seconds to move the mouse to the popover and then the debugger breakpoint will kick in and freeze the JavaScript and DOM mutations. You can then use the inspector tool however often you want
TL;DR: Fixed
✅ You can also see in the above screenshots that the positioning of the popover is different from the code intel popover. The code intel popover is anchored directly above the hovered token, whereas our blame popover has an ambiguous relationship to the line being hovered. It's standard for popovers on Sourcegraph to be positioned directly alongside the content; could we adjust the positioning?
I've changed this but I think I know the reason why it was added in the first place. Without it, you can't see any of the blame lines behind the popover. It's not too big a deal I think since you can always just move the mouse away again to get more context.
What I really like about this change though is that it causes you to no longer need to move the mouse into the content area if you want to click the link and thus it kind of works the second issue Taras raised.
TL;DR: Fixed
✅ On a similar note on font, the row being hovered should transition to being Label/small-underline.
TL;DR: Not an issue
✅ (Open to feedback on this) I transitioned to using absolute dates for better formatting, but open to feedback on this!
My proposal is to make a mix of the relative dates in the blame column for easier scanning and relative comparisons between the rows and to show the full timestamp in the overlay since we have more space there and need precision (the relative date was already shown before).
TL;DR: Awaiting feedback
Test plan
App preview:
Check out the client app preview documentation to learn more.