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

details/charts #383

Merged
merged 10 commits into from
Apr 25, 2019
Merged

details/charts #383

merged 10 commits into from
Apr 25, 2019

Conversation

matthewcarleton
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Apr 18, 2019

Pull Request Test Coverage Report for Build 1659

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • 12 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.1%) to 86.469%

Files with Coverage Reduction New Missed Lines %
src/selectors/prometheus/storage.js 2 4.0%
src/components/Details/VmDetails/VmDetails.js 2 88.15%
src/components/StorageOverview/TopConsumers/TopConsumers.js 8 35.0%
Totals Coverage Status
Change from base Build 1396: 0.1%
Covered Lines: 3645
Relevant Lines: 4040

💛 - Coveralls

@matthewcarleton
Copy link
Contributor Author

@rawagner just need some help on the class I'm adding on the dl

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2019
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2019
@@ -29,9 +28,5 @@
.kubevirt-capacity__graph {
display: flex;
flex-direction: column;

svg {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rawagner did you say we needed this overflow?

Copy link
Contributor

@rawagner rawagner Apr 24, 2019

Choose a reason for hiding this comment

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

Yes, otherwise tooltips are cut see Capacity CPU chart
svg_tooltip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@rawagner
Copy link
Contributor

I see a few issues

  1. if Health is showing multiple errors with additional message it makes height of Health and Compliance card different
    health_css

  2. Storage dashboard - Capacity and Data Resiliency cards are not aligned anymore
    storage_size

Arent those Capacity charts too small ? Ie CPU - 12% Used - the text is really tiny

@matthewcarleton
Copy link
Contributor Author

I see a few issues

  1. if Health is showing multiple errors with additional message it makes height of Health and Compliance card different
    health_css
  2. Storage dashboard - Capacity and Data Resiliency cards are not aligned anymore
    storage_size

Arent those Capacity charts too small ? Ie CPU - 12% Used - the text is really tiny
@rawagner
The height issue should be fixed now. I have a react issue that I'll need some help with :)
I made the charts a bit wider, it's a tricky balance of keeping them narrow enough so they fit on one line. We could adjust the one capacity chart for storage to be larger but that might be a little strange.

@rawagner
Copy link
Contributor

@matthewcarleton fixed the build

  1. and 2. are resolved
    I see that Events card has horizontal scroll now which is not what we want right ? It can even be seen on this screenshot details/charts #383 (comment) - Chrome 72.0.3626.109 Full HD display

@rawagner
Copy link
Contributor

@rawagner
The height issue should be fixed now. I have a react issue that I'll need some help with :)
I made the charts a bit wider, it's a tricky balance of keeping them narrow enough so they fit on one line. We could adjust the one capacity chart for storage to be larger but that might be a little strange.

I agree, they should be the same size, otherwise it would be a bit weird. I think that previous size was ok ( maybe scale down to like 80%) but the issue was that the items didn't wrap very well - you could end up with 3 up and 1 down - which can still happen
cap_wrap

I think that if we make the items bigger but make sure that they wrap only to 2 up 2 down
cap_wrap_ok

or 1 for a row
cap_wrap_ok2

it would be good

@matthewcarleton
Copy link
Contributor Author

@rawagner I agree there is more work to be done here for responsive behaviour. The charts, the column breakdowns in top consumers, and just generally how card content reacts at different viewport sizes. I think it's out of scope for this PR but something that should definitely be addressed post Summit :) Happy to help out with that.

@rawagner rawagner merged commit c64524c into kubevirt:master Apr 25, 2019
rawagner pushed a commit to rawagner/web-ui-components that referenced this pull request Apr 25, 2019
rawagner added a commit that referenced this pull request Apr 25, 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.

4 participants