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

feat: add doctor visibility provider & extract doctor #930

Merged
merged 2 commits into from
Mar 31, 2022

Conversation

kpodsiad
Copy link
Member

src/doctor.ts Outdated
Comment on lines 39 to 43
this.doctor.onDidDispose(() => {
this.client.sendNotification(doctorNotification, { visible: false });
this.isVisible = false;
this.doctor = undefined;
});
Copy link
Member

Choose a reason for hiding this comment

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

There is also an option to change visibility if this tab isn't focused.

      this.doctor.onDidChangeViewState(() => {
        this.doctor.visible
      })

Copy link
Member Author

Choose a reason for hiding this comment

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

onDidChangeViewState fires when webview isn't focused or stops being visible?

I assumed that if the webview is open then we should always treat it as "visible" - user may come to it anytime and we should display them actual status.

Copy link
Member

Choose a reason for hiding this comment

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

I assumed that if the webview is open then we should always treat it as "visible" - user may come to it anytime and we should display them actual status.

Haha. Actually, this visible word usage attracted my attention to that and I decided to check if it really about visibility or not.

Won't it be equal to the just opening a doctor page?
I think if we introduce some kind of cache it won't take a lot of time to refresh it after focusing on it back.

@dos65
Copy link
Member

dos65 commented Mar 31, 2022

@kpodsiad There is some issue with build and test.

@kpodsiad kpodsiad merged commit 571f2dc into scalameta:main Mar 31, 2022
@kpodsiad kpodsiad deleted the feat/doctor-visibility-provider branch March 31, 2022 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants