Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

Adding Health in Dashboard #342

Merged
merged 1 commit into from
Apr 15, 2019
Merged

Conversation

AyushAmbastha
Copy link
Contributor

Added the health folder in dashboard so that it's generic and can be used across components. Working remains the same.

src/components/Dashboard/Health/strings.js Outdated Show resolved Hide resolved
src/components/Dashboard/Health/HealthItem.js Outdated Show resolved Hide resolved
src/components/Dashboard/Health/HealthItem.js Outdated Show resolved Hide resolved
@AyushAmbastha
Copy link
Contributor Author

@rawagner Agreed, Will amend the PR. Thanks!

src/components/Dashboard/Health/HealthItem.js Outdated Show resolved Hide resolved
src/components/Dashboard/Health/HealthItem.js Outdated Show resolved Hide resolved
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2019
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2019
@coveralls
Copy link

coveralls commented Apr 15, 2019

Pull Request Test Coverage Report for Build 1462

  • 26 of 26 (100.0%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.06%) to 87.272%

Totals Coverage Status
Change from base Build 1223: -0.06%
Covered Lines: 3356
Relevant Lines: 3682

💛 - Coveralls

@AyushAmbastha
Copy link
Contributor Author

@rawagner @suomiy , I've made the requested changes and amended the PR. Please review.

BEFORE-

Old-health

AFTER-

New-health

The changes I've made are-

  1. Made health card generic by adding Heath in the Dashboard folder. HealthItem now receives icon, message and classname. These values are passed after being mapped specific for Cluster or Storage Overview.
  2. Made a change in the dashoard.scss file to remove the gap between the Health and the Compliance card.
  3. Wrote fixtures and tests for compliance card by adding new file 'ComplianceBody.js' and removed complianceData from the index.js file.
  4. Seperate 'strings.js' file for Cluster and Storage Overview which is used when Health card is rendered.

Reference Doc - https://docs.google.com/presentation/d/15nDG3VuQRy6vJbMX4yq6ZvJ5G2Qawaz9glnLT8GEvX4/edit#slide=id.g4fa43fba25_0_0

src/components/Dashboard/Health/HealthItem.js Outdated Show resolved Hide resolved
src/components/Dashboard/Health/HealthItem.js Outdated Show resolved Hide resolved
</DashboardCardBody>
</DashboardCard>
);
const HealthStatus = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not change data format from the previous one, we are not using the same as in OCS

Copy link
Contributor

Choose a reason for hiding this comment

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

I would split the PR to two - first one would contain new generic HealthBody/Item for Dashboard + Health in StorageOverview. I will create second one which changes ClusterOverview health to use HealthBody/Item from Dashboard - as this will probably require changes in web-ui too

src/components/Dashboard/Health/HealthItem.js Outdated Show resolved Hide resolved
src/components/Dashboard/Health/HealthItem.js Outdated Show resolved Hide resolved
@AyushAmbastha
Copy link
Contributor Author

AyushAmbastha commented Apr 15, 2019

@rawagner Rolled back the changes for the cluster overview and made the requested changes.

Note: Build failing with error - Invalid component selector ".kubevirt-dashboard-health__body"
kubevirt-health-... is already used in Cluster Overview.

@rawagner rawagner merged commit 8534d65 into kubevirt:master Apr 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants