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

Views: Support contributing welcome-views to empty tree view #8678

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

EstherPerelman
Copy link
Contributor

@EstherPerelman EstherPerelman commented Oct 28, 2020

Signed-off-by: Esther Perelman [email protected]

Welcome-views can be contributed by extensions in order to be applied to empty tree views, A view is considered empty if the tree has no children.
see vscode doc: vs-code-contributes.viewsWelcome

What it does

Partially solves #7178 #8667 (For now supports contributing to custom-views and 'explorer' view but not the other built-in views: 'scm', 'debugger', 'test').

How to test

  1. Deploy test extension run-configuration.zip
    When opening it from the left menu - you should see the contributed welcome-view:

Screenshot 2020-10-28 155722

2. You can now contribute more welcome-views to the run-configuration view by adding a viewWelcome to any extension package.json under "contributes" key, For example:

"viewsWelcome": [{
"view": runConfigurations,
"contents": "In order to use git features, you can open a folder containing a git repository or clone from a URL.\nOpen Folder\nClone Repository\nTo learn more about how to use git and source control in VS Code read our docs.",
}]

Now it should look like this:
Screenshot 2020-10-28 161547

You can play around... (add when clause etc...)

  1. open theia without opening a workspace/folder - you should see a new welcome view in the Explorer tab, You can contribute additional views-welcome to the Explorer view by specifying "view": "explorer" .

The contentns are rendered like this:
'/n' - for new line.
'[here goes the label](here goes a command or a link)' - If there's only a command/link in a line without any other text - it will be displayed as a button, Otherwise as a link.

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added the vscode issues related to VSCode compatibility label Oct 28, 2020
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can perform a comprehensive review after the next release (tomorrow) 👍

packages/plugin-ext/src/main/browser/style/tree.css Outdated Show resolved Hide resolved
packages/plugin-ext/src/main/browser/style/tree.css Outdated Show resolved Hide resolved
@EstherPerelman EstherPerelman marked this pull request as draft October 29, 2020 08:20
@EstherPerelman EstherPerelman changed the title Views: Support contributing welcome-views to custom-views Views: Support contributing welcome-views to empty tree view Oct 29, 2020
@EstherPerelman EstherPerelman force-pushed the views-welcome branch 3 times, most recently from 3d9e3ce to a03e2fe Compare October 29, 2020 21:49
@kittaakos kittaakos self-requested a review October 30, 2020 08:00
Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it with a VS Code extension, and it works perfectly. I look forward to seeing this feature in Theia. Nice job, @EstherPerelman 👍

@@ -33,7 +33,7 @@ export const DIR_NODE_CLASS = 'theia-DirNode';
export const FILE_STAT_ICON_CLASS = 'theia-FileStatIcon';

@injectable()
export class FileTreeWidget extends TreeWidget {
export class FileTreeWidget extends TreeViewWelcomeWidget {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we enable the welcome-support in all other trees as well? It isn't FS specific. What do you think, @akosyakov?

@EstherPerelman EstherPerelman force-pushed the views-welcome branch 2 times, most recently from bf52574 to 9372a7a Compare November 1, 2020 13:50
@EstherPerelman EstherPerelman marked this pull request as ready for review November 1, 2020 14:00
@EstherPerelman
Copy link
Contributor Author

@kittaakos @vince-fugnitto Can you continue reviewing the code?

@kittaakos kittaakos added the CQ Required (deprecated) issues requiring a CQ (contributor questionnaire) label Nov 3, 2020
@amiramw
Copy link
Member

amiramw commented Nov 19, 2020

@kittaakos @vince-fugnitto other then the CQ (which is approved now) do you have more comments?

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me, I verified with the simple extension but not the additional configuration as I do not have the source code.

I'm wondering about the following https://github.com/eclipse-theia/theia/pull/8678/files#r515101313 however. I'll add reviewers who are more familiar with the plugin-system to see if they have any comments as well.

Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works nicely, very good job. I added a few comments.

One question; if Clone Repository is not supported when there is no opened workspace, why do we show it?

Screen Shot 2020-11-20 at 10 08 08

@EstherPerelman
Copy link
Contributor Author

It works nicely, very good job. I added a few comments.

One question; if Clone Repository is not supported when there is no opened workspace, why do we show it?

Screen Shot 2020-11-20 at 10 08 08

@amiramw What do you think?

@amiramw
Copy link
Member

amiramw commented Nov 22, 2020

It works nicely, very good job. I added a few comments.
One question; if Clone Repository is not supported when there is no opened workspace, why do we show it?
Screen Shot 2020-11-20 at 10 08 08

@amiramw What do you think?

It should be supported when there is no opened workspace. At least in vscode it is with vscode builtin git support. Did you test with builtin git extension?

@EstherPerelman EstherPerelman force-pushed the views-welcome branch 4 times, most recently from 3399afd to f4ec5a6 Compare November 22, 2020 10:11
@EstherPerelman
Copy link
Contributor Author

EstherPerelman commented Nov 22, 2020

It works nicely, very good job. I added a few comments.

@kittaakos Thanks for reviewing! I updated the code accordingly

@kittaakos
Copy link
Contributor

It should be supported when there is no opened workspace

There was no workspace opened:
Screen Shot 2020-11-23 at 13 34 41

Did you test with builtin git extension?

I tested with the electron example app.

@EstherPerelman
Copy link
Contributor Author

It should be supported when there is no opened workspace

There was no workspace opened:
Screen Shot 2020-11-23 at 13 34 41

Did you test with builtin git extension?

I tested with the electron example app.

Anyway, I removed it and when working with the builtin git extension - it will be contributed by it (and supported).

@amiramw
Copy link
Member

amiramw commented Nov 25, 2020

@kittaakos are there any more comments or can we merge? (after CI is working and release is over)

@kittaakos
Copy link
Contributor

are there any more comments or can we merge?

I haven't had time to look into the updates, probably it is good to merge if all remarks have been addressed, but we need to restore the CI first.

@amiramw
Copy link
Member

amiramw commented Dec 2, 2020

@kittaakos PR is rebased and build passed. could you give final approval?

Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @EstherPerelman. Nice feature👍

@amiramw, please help with the merging. I've checked and tried the changeset twice already.

Copy link
Member

@amiramw amiramw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went over the code again and tested successfully

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CQ Required (deprecated) issues requiring a CQ (contributor questionnaire) vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants