-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #43465 #44006
Fix #43465 #44006
Conversation
@aushakou Thank you for the PR. This does not fix the issue, although it comes pretty close. The shadow element is placed before the minimap element in the DOM, and thus, the minimap is painted on top of the shadow element: |
@alexandrudima I would like to continue working on this issue if it is possible. But I need some clarification about how the shadow element should be placed. |
We could rearrange the dom nodes in https://github.com/Microsoft/vscode/blob/ab7f6e871d1af714ae454a06e2745a6ec0521704/src/vs/editor/browser/view/viewImpl.ts#L227:L234 But now I am having second thoughts if we should do this change at all: Here is the case where scrolling to the right, the minimap also drops a shadow, and the top shadow and the right shadow form a nice corner at the minimap boundary. With this change things would not line up: |
@alexandrudima From my understanding, if the minimap is on the right side, everything seems to be fine. If the minimap is on the left side, there are two issues. This PR seems to fix issue 1 (the top shadow) by increasing the width of the shadow. I think we should add the shadow on the right side by sticking it to the vertical scrollbar. And I think it might require a separate issue. |
@aushakou Sorry about that. I somehow misunderstood the issue, and didn't place the minimap to the left. May I suggest we do the following: When the minimap is to the left, we make the right-hand side area of the editor behave as if the minimap is disabled. Namely:
|
The top shadow extends over the scrollbar when minimap is on the left side or when minimap is disabled.
The scrollbar is transparent when minimap is on the left side.
@alexandrudima I tried to fix this issue by following your suggestions and it seems to work fine. When the minimap is on the right side: When the minimap is disabled: |
Nice! Thank you for your contribution! ❤️ |
@alexandrudima Thanks for your guidance and suggestions. |
This fixes #43465, removing shadow glitch under tab bar when minimap on the left side.
By the way, I wrote a blog post about fixing this issue.
I would appreciate any comments and suggestions
Thanks.