Skip to content
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

Fixes top and bottom border cut off on status bar item border #101656

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jeanp413
Copy link
Contributor

@jeanp413 jeanp413 commented Jul 3, 2020

For the bottom border:

  • The window border wasn't taken into account when calculating the layout
    image

For the top border:

This PR fixes #101525

@isidorn
Copy link
Contributor

isidorn commented Jul 27, 2020

Thanks for the PR. I think this makes sense, however also assigning to @sbatten so he gives it a review since he owns the layout code

@isidorn isidorn requested a review from sbatten July 27, 2020 08:38
isidorn
isidorn previously approved these changes Jul 27, 2020
@isidorn
Copy link
Contributor

isidorn commented Nov 5, 2020

@sbatten friendly ping on this? Do you pan to review this?

@jeanp413 for the slow response this got out of date. If you are still interested in this is it possible that you get rid of merge conflicts and then I can look into merging this? Thanks a lot for great PRs!

@isidorn
Copy link
Contributor

isidorn commented Nov 5, 2020

@jeanp413 thanks! I plan to review this week. Let's give some time for @sbatten to responde before that though.

@isidorn
Copy link
Contributor

isidorn commented Nov 6, 2020

I tried this PR and it looks good and the changes make sense.
However the issue seems to be fixed already in vscode insdiers.
That is in insiders with HC theme the bottom border already gets nicely rendered. At least on my machine
Let me know if I am missing something here

Screenshot 2020-11-06 at 17 30 46

@sbatten
Copy link
Member

sbatten commented Nov 6, 2020

@isidorn I looked at the linked issue and it seems this fix the hover border case, not the focus case. I can reproduce the original issue in insiders and the behavior is different with the PR. However, I don't like that we don't show the top border. I think we should, in both focus and hover, show the top and bottom border.

@jeanp413
Copy link
Contributor Author

jeanp413 commented Nov 8, 2020

@isidorn I can also reproduce in insiders, in your screenshot the status bar does not have a white bottom border, for that the window should not be maximized.

@isidorn
Copy link
Contributor

isidorn commented Nov 9, 2020

@jeanp413 thanks for clarifying
However based on @sbatten will not merge this in, since he owns this code.

@isidorn isidorn assigned sbatten and unassigned isidorn Nov 9, 2020
@isidorn isidorn removed this from the November 2020 milestone Nov 9, 2020
@joaomoreno joaomoreno changed the base branch from master to main February 15, 2021 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Top and bottom border cut off on status bar item border
4 participants