-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Extension divergence help information in the UI #2185
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something went wrong with the diff here. I think you need to rebase your branch and clean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE: code-server has it's own marketplace of open-source extensions due to
<a href="https://github.com/cdr/code-server/blob/master/doc/FAQ.md#differences-compared-to-vs-code" target="_blank">legal distribution restrictions</a>.
VSIX assets from the official marketplace can be
<a onClick="MAKE THIS EXECUTE THE UPLOAD VSIX COMMAND">uploaded<a/>.
Thoughts on something like this?
@kylecarbs I do think a link to the official marketplace is worth a lot.
Oh yeah I'm down to include that.
Just tryna reduce the size of the copy as much as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, +1 from me on reducing the copy size as much as possible. Should we store the dismissal state for subsequent page loads or would it be more beneficial have it show more often?
My nitpick here is use the same caution dialogue indicator used in Git when you're exceeding the 50-character limit so the user will see it immediately, it will add a sense of visual priority of what to read first.
Hmm, that's an interesting idea, it looks like it pops up below the textbox as you type. I lean toward keeping the current method because that popup floats and would probably cover an extension or two in the search results which could be frustrating. We could only show it if there weren't any results, but I think having results but not the right ones might be a common case (just a guess though) and those cases would never get the help text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. BTW if we still want to add a link for jumping directly to the upload we can do it with something like:
uploadButton.addEventListener("click", () => {
this.instantiationService.createInstance(InstallVSIXAction, InstallVSIXAction.ID, InstallVSIXAction.LABEL).run()
});
Closing this PR as it has become clear that linking to or referencing the existence of the official marketplace is a no-go. Suggesting users to upload via the dropdown has the same problem. Keeping the helper text without including instructions for resolution will cause more confusion than it's worth. Eventually, if we ever move to open-source our extension marketplace, an overlay of this nature will be appropriate.
😢 I'll open a new PR with a revised issue template for extension requests.
Uh oh!
There was an error while loading. Please reload this page.
Adds the following UI to the extension search view:
Screen Shot 2020年10月13日 at 12 06 47 PM
Closes #2132