-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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: plugin doesn't show welcome view (#12458) #12466
fix: plugin doesn't show welcome view (#12458) #12466
Conversation
The commit check view data provider after start plugin to show plugin welcom view. Signed-off-by: zhangyi <1401457589@qq.com>
e5dad4e
to
b0b697d
Compare
@@ -303,6 +303,9 @@ export class HostedPluginSupport { | |||
} | |||
await this.startPlugins(contributionsByHost, toDisconnect); | |||
|
|||
// check data provider after start plugin to show welcom view |
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.
minor:
// check data provider after start plugin to show welcom view | |
// check data provider after plugin start to show welcome view |
checkViewDataProvider(): void { | ||
for (const viewId of this.views.keys()) { | ||
if (!this.viewDataProviders.has(viewId)) { | ||
this.getView(viewId).then(async view => { | ||
if (view) { | ||
if (view.isVisible) { | ||
await this.prepareView(view); | ||
} else { | ||
const toDisposeOnDidExpandView = new DisposableCollection(this.onDidExpandView(async id => { | ||
if (id === viewId) { | ||
unsubscribe(); | ||
await this.prepareView(view); | ||
} | ||
})); | ||
const unsubscribe = () => toDisposeOnDidExpandView.dispose(); | ||
view.disposed.connect(unsubscribe); | ||
toDisposeOnDidExpandView.push(Disposable.create(() => view.disposed.disconnect(unsubscribe))); | ||
} | ||
} | ||
}); | ||
} | ||
} | ||
} |
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.
This code duplicates a lot of what registerViewDataProvider
already does, I don't think it is the proper solution as is.
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.
May I ask if the meaning here is that I should try to reuse code? Or should we solve the problem in a different way
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.
Sorry, I mean that we should not have duplication, and it looks like the current solution duplicates a lot of what registerViewDataProvider
already does without any clear reason as to why it does. We should find a solution which does not duplicate the code, either by seeing what can be reused once we better understand the problem, or a completely different solution if necessary.
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.
Okay, I understand. In Theia,widgets should be opened only when view data providers are registered, but some vscode plugins only have some welcomeViews and do not register a viewDataProvider. So I want to check if the plugin view where the welcome view is located has been registered a viewDataProvider after the plugin is launched, and if not, give a default
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.
Thank you for your previous suggestion, but I can't find a simpler solution. I will create a new PR to remove the commit messages and wait for other results
The commit remove some duplicate code. Signed-off-by: zhangyi <1401457589@qq.com>
What it does
Closes: #12458
The pull-request checks the view data provider after the start of a plugin to show the plugin's welcome view.
How to test
isntall from vsix...
Before:
After:
Review checklist
Reminder for reviewers