Replaced PF3 charts with PF4 #1048

bond95 · 2019-06-24T14:46:46Z

Big screen

Small screen

lwrigh · 2019-06-24T15:34:35Z

Was the font changed back to Overpass? Is the color of the chart a PatternFly standard?

sjd78

Travis CI is failing on yarn test
The standard ci build failed (initially) due to needing a pre-seed on ovirt-engine-nodejs-modules

sjd78 · 2019-06-25T18:16:25Z

ci test please

bond95 · 2019-06-27T10:48:28Z

Was the font changed back to Overpass?

Good catch, I forgot to change font.

Is the color of the chart a PatternFly standard?

Yes, this is default color for Donut in Patternfly 4, but in History charts I changed color to be same as in Patternfly 3.

bond95 · 2019-06-27T11:16:03Z

ci test please

sjd78

I have a VM that uses the qemu guest agent and it isn't reporting disk partitions. This results in an empty graph with the old charts. With the new charts, the CustomLabel errors out and the GlobalErrorBoundary kicks in:

Uncaught TypeError: number must be a number, an array of numbers or an object of numbers
    at round (round.js:21)
    at additionalLabel (DiskCharts.js:124)
    at CustomLabel (BarChart.js:7)
    at mountIndeterminateComponent (react-dom.development.js:13272)
    at beginWork (react-dom.development.js:13711)
    at performUnitOfWork (react-dom.development.js:15741)
    at workLoop (react-dom.development.js:15780)
    at HTMLUnknownElement.callCallback (react-dom.development.js:100)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:138)
    at invokeGuardedCallback (react-dom.development.js:187)

error  The above error occurred in the <CustomLabel> component:
    in CustomLabel (created by BarChart)
    in g
    in VictoryBar (created by ChartBar)
    in ChartBar (created by BarChart)
    in g
    in VictoryStack (created by ChartStack)
    in ChartStack (created by BarChart)
    in svg (created by VictoryContainer)
    in div (created by VictoryContainer)
    in VictoryContainer
    in VictoryChart (created by Chart)
    in Chart (created by BarChart)
    in BarChart (created by DiskCharts)
    in div (created by DiskCharts)
    in div (created by CardBody)
    in CardBody (created by DiskCharts)
    in div (created by Card)
    in Card (created by UtilizationCard)
    in UtilizationCard (created by DiskCharts)
    in DiskCharts (created by UtilizationCard)

From what I can tell, the error happens because the DiskCharts component considers an empty diskDetails (VM.statistics.disk.usage === []). This is ugly but will work:

  const diskDetails = diskStats && diskStats.usage && diskStats.usage.datum && diskStats.usage.datum.length > 0 ? diskStats.usage.datum : false

I get the following error in the browser console when mousing over the history charts for Memory and Networking:

delaunator.js:80 Uncaught Error: No Delaunay triangulation exists for this input.
    at new Delaunator (delaunator.js:80)
    at new Delaunay (index.js:38)
    at Function.Delaunay.from (index.js:158)
    at Object.getVoronoiPoints (voronoi-helpers.js:151)
    at Object.onMouseMove (voronoi-helpers.js:279)
    at invokeFunc (debounce.js:95)
    at leadingEdge (debounce.js:105)
    at Object.debounced [as onMouseMove] (debounce.js:172)
    at Object.onMouseMove (victory-voronoi-container.js:352)
    at Object.onEvent (events.js:240)

src/components/VmDetails/cards/UtilizationCard/CpuCharts.js

src/components/VmDetails/cards/UtilizationCard/MemoryCharts.js

src/components/VmDetails/cards/UtilizationCard/NetworkingCharts.js

src/components/VmDetails/cards/UtilizationCard/MemoryCharts.js

src/components/VmDetails/cards/UtilizationCard/DiskCharts.js

src/components/VmDetails/cards/UtilizationCard/NetworkingCharts.js

src/components/VmDetails/cards/UtilizationCard/CpuCharts.js

src/components/VmDetails/cards/UtilizationCard/UtilizationCharts/DonutChart.js

src/components/VmDetails/cards/UtilizationCard/DiskCharts.js

sjd78 · 2019-06-28T06:37:43Z

CI tests appear to be failing because with the package.json files updated, jest is now 24.8.0 and we're hitting this error: jestjs/jest#8426

/projects/sjd78-ovirt-web-ui > rm -rf node_modules                            
                      
/projects/sjd78-ovirt-web-ui > yarn install                                                         
yarn install v1.16.0
[1/4] Resolving packages...
[2/4] Fetching packages...
info [email protected]: The platform "linux" is incompatible with this module.
info "[email protected]" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
warning " > [email protected]" has incorrect peer dependency "redux@^3.6.0".
warning "patternfly-react > [email protected]" has incorrect peer dependency "redux@>= 3.0.0 < 4.0.0".
warning " > [email protected]" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > [email protected]" has incorrect peer dependency "webpack@>=2.0.0 <5.0.0".
[4/4] Building fresh packages...
Done in 10.28s.

/projects/sjd78-ovirt-web-ui > node_modules/.bin/jest --version                                     
24.8.0

So in package.json, under the jest section, the transform block needs to be fixed.

    "transform": {
      ".+\\.(jsx?)$": "<rootDir>/config/jest/transform.js"
    },

bond95 · 2019-07-02T13:43:17Z

ci build please

sjd78 · 2019-07-02T18:45:48Z

ci test please

sjd78 · 2019-07-03T13:45:42Z

ci test please

sjd78 · 2019-07-03T13:55:24Z

ci build please

Thought: When we have to update the nodejs-modules, it would also be good to update the spec.in file here to require >= the new pre-seed version. That can help cut down on bad builds that happen after the pre-seed patch on nodejs-modules is merged but before the final artifact build is done and distributed as appropriate...

sjd78 · 2019-07-03T17:04:56Z

ci build please

sjd78

Tests and tooltips on the history charts are working well and without error.

From the last review, I filed #1055 with PR #1056.

As is, I think the sizing on the charts still needs some work as the labels are difficult to read:

src/components/VmDetails/cards/UtilizationCard/UtilizationCharts/BarChart.js

src/components/VmDetails/cards/UtilizationCard/UtilizationCharts/DonutChart.js

src/components/VmDetails/cards/UtilizationCard/UtilizationCharts/BarChart.js

Modified bar chart, updated version of react-charts, change size of donut charts

bond95 · 2019-07-09T12:57:52Z

@sjd78 @lwrigh Made DonutCharts a little bit bigger and BarChart more responsive for long file system names.

bond95 · 2019-07-09T13:36:23Z

Suggest to wait until they fix this issue patternfly/patternfly-react#2486 (in worth case I'll fix it), to be more flexible in title and subTitle sizes.

bond95 · 2019-07-09T15:21:49Z

Created PR to speed up fixing: patternfly/patternfly-react#2488

bond95 · 2019-07-10T15:16:47Z

Bug fix was released in version 4.4.17: patternfly/patternfly-react#2488 (comment) Current version in npm is 4.4.7.

lwrigh · 2019-07-11T18:06:47Z

@sjd78 @lwrigh Made DonutCharts a little bit bigger and BarChart more responsive for long file system names.

Increasing the text inside of the charts looks good to me in comparison with the initial pass of it. The updated bar chart looks good to me as well.

thaorell · 2019-07-15T15:29:47Z

src/components/VmDetails/cards/UtilizationCard/UtilizationCharts/AreaChart.js

+
+const AreaChart = ({ data, labels, id }) => {
+  return (
+    <div id={id}>


@sjd78 suggested this to avoid tooltips being cut off outside of div

.area-container svg { overflow: visible !important ; }

Made container overflow

bond95 · 2019-07-17T12:47:12Z

@lwrigh @sjd78 @thaorell Made title and subTitle bigger for DonutChart

sjd78 · 2019-07-18T23:17:10Z

Note: https://gerrit.ovirt.org/#/c/101875/ for the standard ci ovirt-engine-nodejs-modules pre-seed is failing CI because jenkin cannot currently clone that repo. I'll check it again later. (@bond95, @sgratch)

As of this evening, the problem persists. I raised an issue with ovirt infra so hopefully it can get fixed soon and this PR can pass standard-ci.

sjd78 · 2019-07-22T16:47:41Z

ci test please

sjd78 · 2019-07-22T17:03:26Z

@bond95 - Reviewed, pre-seed is all set, and builds properly now. You should be good to hand it off to QE now.

bond95 · 2019-08-28T10:53:37Z

ci test please

Fixes: oVirt#1031

…nd added ids

…patternfly/react-charts, changed size of DonutChart

…arts bigger

michalskrivanek · 2019-08-28T11:40:10Z

since you opened https://gerrit.ovirt.org/#/c/102906/ - you should update the requirement in ovirt-web-ui.spec

sjd78 · 2019-08-28T21:40:06Z

ci test please

bond95 · 2019-09-05T10:37:18Z

ci test please

isaranova · 2019-09-10T11:37:26Z

LGTM

- remove pre-seeds from 2018 - pre-seed for: - ovirt-web-ui 2019-06-24 (oVirt/ovirt-web-ui#1048) - ovirt-web-ui 2019-06-25 (oVirt/ovirt-web-ui#1047) - ovirt-engine-ui-extensions (https://gerrit.ovirt.org/101012) Change-Id: I360ef83f6e9ca8d7d2c7196b63644628bebd5d77 Signed-off-by: Scott J Dickerson <[email protected]>

Pull Request: oVirt/ovirt-web-ui#1048 Change-Id: I2e89d0d668a32b7fee1ee5003a6d6d46a1cdfbff Signed-off-by: Bohdan Iakymets <[email protected]>

Pull Request: oVirt/ovirt-web-ui#1048 Change-Id: I38a4846b12d9df08af9eb6e20e70820993b77711 Signed-off-by: Scott J Dickerson <[email protected]>

PR: oVirt/ovirt-web-ui#1048 Removed pre-seed for ui-extentions Patch: https://gerrit.ovirt.org/#/c/103041/ Change-Id: I662b8edd455e6448ad6a878ff9d63597b272a810 Signed-off-by: Bohdan Iakymets <[email protected]>

bond95 requested review from sjd78, sgratch, lwrigh and thaorell June 24, 2019 14:46

bond95 force-pushed the charts-replace branch from 501346d to d749dc6 Compare June 24, 2019 14:53

sjd78 requested changes Jun 25, 2019

View reviewed changes

sjd78 requested changes Jun 28, 2019

View reviewed changes

bond95 force-pushed the charts-replace branch 2 times, most recently from 100953c to 054d68c Compare July 2, 2019 12:15

sjd78 previously requested changes Jul 3, 2019

View reviewed changes

bond95 force-pushed the charts-replace branch from ef6aca4 to c1f4bec Compare July 11, 2019 09:13

thaorell previously requested changes Jul 15, 2019

View reviewed changes

sjd78 added the Flag: Needs QE needs testing by QE team before merging PR and closing issue label Jul 22, 2019

bond95 added Status: ON_QA status ON_QA (currently being tested) and removed Flag: Needs QE needs testing by QE team before merging PR and closing issue labels Jul 23, 2019

sjd78 added this to the 1.5.z (ovirt 4.3.z) milestone Aug 6, 2019

bond95 force-pushed the charts-replace branch from 7ec0e2f to 6a7e070 Compare August 27, 2019 12:00

bond95 added 7 commits August 28, 2019 13:05

Replaced PF3 charts with PF4

682b437

Fixes: oVirt#1031

Moved History charts to AreaChart component, change color of donuts a…

9afda46

…nd added ids

Updated victory version, to fix tooltip bug

887bd8c

Updated jest transform configuration to fix CI

ffba495

Removed console log

9f9ba2a

Made BarChart more responsive for bigger labels, updated version of @…

efeb210

…patternfly/react-charts, changed size of DonutChart

Updated patternfly@react-charts, made title and subTitle for donut ch…

d90ed4d

…arts bigger

bond95 force-pushed the charts-replace branch from 6a7e070 to d90ed4d Compare August 28, 2019 11:36

sjd78 modified the milestones: 1.6.0 (ovirt 4.3.z), 1.6.1 (ovirt 4.4) Aug 28, 2019

Updated dependency version of ovirt-engine-nodejs-modules

e8e8b3a

bond95 force-pushed the charts-replace branch from cf53302 to e8e8b3a Compare September 3, 2019 09:01

bond95 merged commit e052308 into oVirt:master Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replaced PF3 charts with PF4 #1048

Replaced PF3 charts with PF4 #1048

bond95 commented Jun 24, 2019 •

edited

Loading

lwrigh commented Jun 24, 2019

sjd78 left a comment

sjd78 commented Jun 25, 2019

bond95 commented Jun 27, 2019

bond95 commented Jun 27, 2019

sjd78 left a comment

sjd78 commented Jun 28, 2019 •

edited

Loading

bond95 commented Jul 2, 2019

sjd78 commented Jul 2, 2019

sjd78 commented Jul 3, 2019

sjd78 commented Jul 3, 2019

sjd78 commented Jul 3, 2019

sjd78 left a comment

bond95 commented Jul 9, 2019

bond95 commented Jul 9, 2019

bond95 commented Jul 9, 2019

bond95 commented Jul 10, 2019

lwrigh commented Jul 11, 2019

thaorell Jul 15, 2019

bond95 commented Jul 17, 2019

sjd78 commented Jul 18, 2019

sjd78 commented Jul 22, 2019

sjd78 commented Jul 22, 2019

bond95 commented Aug 28, 2019

michalskrivanek commented Aug 28, 2019

sjd78 commented Aug 28, 2019

bond95 commented Sep 5, 2019

isaranova commented Sep 10, 2019

Replaced PF3 charts with PF4 #1048

Replaced PF3 charts with PF4 #1048

Conversation

bond95 commented Jun 24, 2019 • edited Loading

lwrigh commented Jun 24, 2019

sjd78 left a comment

Choose a reason for hiding this comment

sjd78 commented Jun 25, 2019

bond95 commented Jun 27, 2019

bond95 commented Jun 27, 2019

sjd78 left a comment

Choose a reason for hiding this comment

sjd78 commented Jun 28, 2019 • edited Loading

bond95 commented Jul 2, 2019

sjd78 commented Jul 2, 2019

sjd78 commented Jul 3, 2019

sjd78 commented Jul 3, 2019

sjd78 commented Jul 3, 2019

sjd78 left a comment

Choose a reason for hiding this comment

bond95 commented Jul 9, 2019

bond95 commented Jul 9, 2019

bond95 commented Jul 9, 2019

bond95 commented Jul 10, 2019

lwrigh commented Jul 11, 2019

thaorell Jul 15, 2019

Choose a reason for hiding this comment

bond95 commented Jul 17, 2019

sjd78 commented Jul 18, 2019

sjd78 commented Jul 22, 2019

sjd78 commented Jul 22, 2019

bond95 commented Aug 28, 2019

michalskrivanek commented Aug 28, 2019

sjd78 commented Aug 28, 2019

bond95 commented Sep 5, 2019

isaranova commented Sep 10, 2019

bond95 commented Jun 24, 2019 •

edited

Loading

sjd78 commented Jun 28, 2019 •

edited

Loading