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.

Sign up for GitHub

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

Jump to bottom

All plots sections visible #2145

Merged
merged 8 commits into from
Aug 5, 2022
Merged

All plots sections visible #2145

merged 8 commits into from
Aug 5, 2022

Conversation

sroy3
Copy link
Contributor

@sroy3 sroy3 commented Aug 4, 2022 •
edited
Loading

Part of #1933
Screen Shot 2022-08-04 at 3 06 09 PM
Screen Shot 2022-08-04 at 3 05 30 PM
Screen Shot 2022-08-04 at 3 04 32 PM

@sroy3 sroy3 added the product PR that affects product label Aug 4, 2022
@sroy3 sroy3 self-assigned this Aug 4, 2022
sroy3
sroy3 commented Aug 4, 2022
View reviewed changes
webview/package.json
@@ -47,7 +47,6 @@
"@storybook/manager-webpack5": "6.5.9",
"@storybook/preset-scss": "1.0.3",
"@storybook/react": "6.5.9",
"@storybook/testing-library": "0.0.13",
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 is there twice in the list

@sroy3 sroy3 marked this pull request as ready for review August 4, 2022 19:08
mattseddon
mattseddon approved these changes Aug 4, 2022
View reviewed changes
Copy link
Member

@mattseddon mattseddon left a comment

Choose a reason for hiding this comment

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

For checkpoint plots, I think we need to differentiate between no metrics being selected and there being no plots. Extending that we should probably have different text based on whether or not the project has those types of plots.

@mattseddon
Copy link
Member

@sroy3 could be unrelated to this particular change but virtualized plots seems broken in the storybook

@sroy3
Copy link
Contributor Author

sroy3 commented Aug 5, 2022

@sroy3 could be unrelated to this particular change but virtualized plots seems broken in the storybook

The only difference is the Images section being added between the two other sections.

@sroy3
Copy link
Contributor Author

sroy3 commented Aug 5, 2022

For checkpoint plots, I think we need to differentiate between no metrics being selected and there being no plots. Extending that we should probably have different text based on whether or not the project has those types of plots.

Changed it for when there is no data for checkpoint plots

@codeclimate
Copy link

codeclimate bot commented Aug 5, 2022

Code Climate has analyzed commit 58e5326 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.8% (0.0% change).

View more on Code Climate.

@sroy3 sroy3 merged commit 5f98437 into main Aug 5, 2022
@sroy3 sroy3 deleted the visible-sections branch August 5, 2022 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julieg18 julieg18 julieg18 approved these changes

@mattseddon mattseddon mattseddon approved these changes

@wolmir wolmir Awaiting requested review from wolmir

Assignees

@sroy3 sroy3

Labels
product PR that affects product
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sroy3 @mattseddon @julieg18