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

Fix-76141 Add border around image in image preview #76387

Merged
merged 2 commits into from Jul 3, 2019

Conversation

@skprabhanjan
Copy link
Contributor

commented Jul 1, 2019

@mjbvz , Added the border property accordingly to fix #76141 and kept it default as I could not find the panel color that was mentioned in this comment

I will change it if you can provide me the color code.

This is how it looks for default border 1px solid

image

pkoushik
@dbaeumer

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

@bpasero FYI. Could be you as well :-)

@mjbvz mjbvz requested a review from aeschli Jul 2, 2019

@mjbvz

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Thanks @skprabhanjan.

We probably want to use a border color from one of our existing theme colors, such as panel.border perhaps. Adding @aeschli for ideas on which theme color to use.

Here's an example of how write a style that uses a theme color:

const backgroundColor = theme.getColor(TERMINAL_BACKGROUND_COLOR);

Just add a call to registerThemingParticipant in resourceViewer and then move the css border rule into a string within it

@aeschli

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

reusing panel.border is not really 100% proper, but fine with me. You could also add a new color: imagePreview.border

@skprabhanjan

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

@mjbvz and @aeschli
Added imagePreview.border which has same values as panel.border and used that in registerThemingParticipant in resourceViewer .
Please check and review this :)

PS : This is how it looks now.

image

@mjbvz mjbvz added this to the July 2019 milestone Jul 3, 2019

@mjbvz mjbvz merged commit bc7f5a7 into microsoft:master Jul 3, 2019

3 of 5 checks passed

VS Code Build #20190703.42 failed
Details
VS Code (macOS) macOS failed
Details
VS Code (Linux) Linux succeeded
Details
VS Code (Windows) Windows succeeded
Details
license/cla All CLA requirements met.
Details
@mjbvz

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

Thanks! This will be in the next VS Code 1.37 insiders build and is scheduled to go out in the July release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.