Navigation Menu

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

Gutter icon background size limited #6553

Merged
merged 1 commit into from Jun 17, 2016
Merged

Conversation

kisstkondoros
Copy link
Contributor

To be able to show images with arbitrary size on the gutter margin,
the css property 'background-size' has been added.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @egamma and @alexandrudima to be potential reviewers

@msftclas
Copy link

Hi @kisstkondoros, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@kisstkondoros
Copy link
Contributor Author

Motivation:
I would like to create an extension which would show image previews on the glyph margin

image

The only thing which prevents this is that there is no way to define the background size property.
The proposed change should cover most of the practical use cases.

@kisstkondoros
Copy link
Contributor Author

@alexdima
Copy link
Member

@kisstkondoros This proposed change affects existing gutter decorations, such as the one used for breakpoints:

image

I think it would also affect other extensions that use the gutter margin decorations.

I really like the idea, but maybe the hover would be a better place to preview the image, as the gutter area is so limited and would only work for icon-type images:

https://code.visualstudio.com/docs/extensionAPI/vscode-api#_languages
image

I am not sure if our current MarkedString supports images, but maybe @jrieken can point you to how that could be added.

@kisstkondoros
Copy link
Contributor Author

How about making it configurable?
I'll check what can I do with MarkedString... Thanks for the hint!

@kisstkondoros
Copy link
Contributor Author

kisstkondoros commented May 22, 2016

I've updated the extension with the mentioned HoverProvider, MarkedString indeed supports images.
(is it intended? :) )
I still think it would be useful to have the image preview on the gutter.
(of course it makes sense only for Icon type images...)

@jrieken
Copy link
Member

jrieken commented May 23, 2016

I've updated the extension with the mentioned HoverProvider, MarkedString indeed supports images.
(is it intended? :) )

Yes, that's the intend. MarkedString should be full markdown minus embedded html

@alexdima
Copy link
Member

@kisstkondoros Just a suggestion: maybe you can ship your extension with a generic image icon and render the same one in the gutter where images appear. That would cover nicely all image types.

Do you still plan to do something in this PR? You could make it configurable by adding/adopting a new option in:

@kisstkondoros
Copy link
Contributor Author

About the customization: i did something already in 3708e01, but I haven't changed the vscode.d.ts (i thought it's generated in a way), also I don't have the proper environment for building a version... This week I'll have a look.

I appreciate Your suggestions, but I don't really like the idea to show just a generic icon.

To be able to show images with arbitrary size on the gutter margin,
the css property 'background-size' has been made configurable.
@kisstkondoros
Copy link
Contributor Author

@alexandrudima Please have a look :)

@msftclas
Copy link

@kisstkondoros, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@alexdima alexdima modified the milestones: June 2016, Backlog May 26, 2016
@alexdima alexdima merged commit 65c7bd2 into microsoft:master Jun 17, 2016
alexdima added a commit that referenced this pull request Jun 17, 2016
Gutter icon background size limited
@alexdima
Copy link
Member

@kisstkondoros Looks great, thank you! Extra kudos for the tests! ❤️

@kisstkondoros
Copy link
Contributor Author

@alexandrudima
You are welcome 😄 (those tests are not the ones I'm most proud of actually 😃 ❤️ toString ❤️ )

also please note that I have used URI.parse instead of URI.file to support some special requirements for the mentioned plugin, I think it is not a breaking change but I'm not sure.

@alexdima
Copy link
Member

Oh, it might be breaking, I'll write a quick plugin to double check...

@alexdima
Copy link
Member

Ok, I checked for both Windows and Linux that absolute gutter paths still work fine even with URI.parse

@jrieken
Copy link
Member

jrieken commented Sep 27, 2016

might be related to #12111

@kisstkondoros Can you detail on the special requirements? That change actually is breaking: Uri#file makes sure that path with # et al get parsed correctly (/my/c#files -> { path: /my/c#files } vs { path: /my/c, fragment: files }) and also it normalises backslashes before calling parse internally.

@kisstkondoros
Copy link
Contributor Author

kisstkondoros commented Sep 27, 2016

@jrieken I think I've thought about having support also for http/https uri's. With the proposed api change (string | URI) it would be even better 👍

@kisstkondoros
Copy link
Contributor Author

@jrieken Thanks for giving a hint about the API change 🙇
I hope I have successfully prepared the extension for the API change (kisstkondoros/gutter-preview@2ac1f6f)

@kisstkondoros kisstkondoros deleted the patch-1 branch February 25, 2017 20:11
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants