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

fix: view management, don't show trust & connect when not needed #377

Conversation

bastiandoetsch
Copy link
Contributor

@bastiandoetsch bastiandoetsch commented Sep 27, 2023

Description

Previously, during plugin startup, "trust & connect" was shown in the welcome dialog. Now, during startup, a message is shown that the plugin is still starting. Views are now initialized with reasonable defaults and only displayed when needed.

Also removed the "feature select" view that was not shown anymore anyway. Boy Scouts Rule!

Checklist

  • Tests added and all succeed
  • Linted
  • CHANGELOG.md updated
  • README.md updated, if user-facing

Screenshots / GIFs

Before:
https://github.com/snyk/vscode-extension/assets/20150761/fdeb9a93-1f4c-4831-95c2-1d0c2eeb8eb2

Screenrecording After:
https://github.com/snyk/vscode-extension/assets/20150761/d2ae2c58-0721-45ac-9690-7083b9ec7d47

@bastiandoetsch bastiandoetsch marked this pull request as ready for review September 27, 2023 13:39
@bastiandoetsch bastiandoetsch requested review from a team as code owners September 27, 2023 13:39
@bastiandoetsch bastiandoetsch requested a review from a team September 27, 2023 13:39
@bastiandoetsch bastiandoetsch force-pushed the fix/HEAD-655_VS-Code-Extension-Trustworkspace-and-connect-message-persists branch from 4e58bed to 6396f6b Compare September 27, 2023 13:47
@bastiandoetsch bastiandoetsch force-pushed the fix/HEAD-655_VS-Code-Extension-Trustworkspace-and-connect-message-persists branch from 6396f6b to fb90097 Compare September 29, 2023 06:41
Comment on lines +9 to +11
// eslint-disable-next-line @typescript-eslint/no-unused-vars,@typescript-eslint/no-explicit-any
declare const acquireVsCodeApi: any;

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it correct to say that this package could bring the types we need?

DefinitelyTyped/DefinitelyTyped#53370
https://www.npmjs.com/package/@types/vscode-webview

If so, do you think we could start using them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, let's do that, but put that into a different PR

Comment on lines -104 to +107
lineOnly: !!lineOnly,
lineOnly: lineOnly,
Copy link
Contributor

Choose a reason for hiding this comment

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

This operator forces the conversion of truthy/falsy values to primitive booleans. Do we need to work with loose truthy/falsy values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be a guaranteed boolean, so we don't need the forced conversion.

Copy link
Contributor

@cat2608 cat2608 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 :)

@bastiandoetsch bastiandoetsch merged commit f158194 into main Sep 29, 2023
3 checks passed
@bastiandoetsch bastiandoetsch deleted the fix/HEAD-655_VS-Code-Extension-Trustworkspace-and-connect-message-persists branch September 29, 2023 09:25
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