-
Notifications
You must be signed in to change notification settings - Fork 191
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
Show number of successful repos instead of completed repos in variant analysis results view header #2349
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We're always passing in a non-undefined number anyway, so this optional value isn't giving us any benefit.
elenatanasoiu
approved these changes
Apr 19, 2023
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.
Looks good, had one small suggestion for readability, but it's not blocking.
extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx
Outdated
Show resolved
Hide resolved
robertbrignull
added a commit
that referenced
this pull request
Apr 28, 2023
3 tasks
robertbrignull
added a commit
that referenced
this pull request
Apr 28, 2023
…changelog Add missing changelog entry for #2349
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2239
This PR aims to address the bug spotted in the issue linked above. We were showing the number of completed analyses instead of the number of successful analyses, and this caused confusion when actually looking through the results. This isn't how our original designs were so I think this is a bug and we didn't implement our designs correctly.
I try to generally improve this area by surfacing as many useful states as we can, and adding labels to the icons to explain what they mean. I also rewrite the text that previously said "Succeeded warnings" with something more descriptive and grammatically correct.
To see it in action try running some variant analyses, and to see all the possible states check out the storybook.
Checklist
ready-for-doc-review
label there.