Skip to content

Handle viewLoaded message#1574

Merged
koesie10 merged 6 commits intomainfrom
koesie10/view-loaded-message
Oct 12, 2022
Merged

Handle viewLoaded message#1574
koesie10 merged 6 commits intomainfrom
koesie10/view-loaded-message

Conversation

@koesie10
Copy link
Copy Markdown
Member

@koesie10 koesie10 commented Oct 7, 2022

This will handle the viewLoaded message by sending the variant analysis to the webview. It will also clean up some things that are not relevant anymore after this change.

It's probably easiest to review this PR commit-by-commit.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

When the `viewLoaded` message is received by the view, it will now
retrieve the variant analysis from the manager and send it to the
view. This will allow the view to display the variant analysis.
The `VariantAnalysis` component will now only receive values from the
`VariantAnalysisView`. We still allow setting defaults to support
Storybook.
The mock variant analysis view would only show the loading message. This
completely removes it since it does not provide value anymore.
@koesie10 koesie10 added the secexp label Oct 7, 2022
@koesie10 koesie10 marked this pull request as ready for review October 7, 2022 13:46
@koesie10 koesie10 requested review from a team as code owners October 7, 2022 13:46
Base automatically changed from koesie10/request-repo-results-message to main October 11, 2022 14:48
Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

Looking really good, just a couple of comments.


const variantAnalysis = await this.manager.getVariantAnalysis(this.variantAnalysisId);

if (!variantAnalysis) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we show a notification to the user to tell them something went wrong?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think that would indeed be useful, I'll add a showAndLogWarningMessage call here.

Comment thread extensions/ql-vscode/package.json
@koesie10 koesie10 merged commit 908abb4 into main Oct 12, 2022
@koesie10 koesie10 deleted the koesie10/view-loaded-message branch October 12, 2022 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants