Angular infra dashboards #1901

h-kataria · 2017-08-14T13:29:05Z

BEFORE:

AFTER:

https://www.pivotaltracker.com/story/show/147962059

h-kataria · 2017-09-20T20:56:20Z

@himdel please review/suggest how to convert this PR to Angular 2.

himdel · 2017-09-25T12:32:58Z

@h-kataria Oh, this looks really close now :) 👍

One note.. you probably can't use id in bindings, or rather, you can, but it's confusing whether it's meant as a browser element id or a component-dependent value .. can you rename those to something a bit more descriptive? (Like widget-id maybe? Or is it ems-id?)

Oh, and is patternfly.card the right module? Feels weird to create components in a module we don't own, unless this is really a future patternfly component.. (doesn't matter in practice, just looks surprising :)).

As for angular2 .. We would need to make that whole screen (well, the right cell anyway) an Angular 2 app (that should come first, otherwise you can't use the components) .. then those components should be useable via ngUpgrade as they are now... And once that is working, the actual component conversion should be relatively trivial...

angular.module( 'patternfly.card' ).component('pfAggregateStatusCard', {
  bindings: {
    status: '=',
    showTopBorder: '@?',
    altLayout: '@?',
    layout: '@?'
  },
  templateUrl: '/static/pf_charts/aggregate-status-card.html.haml',
  controller: function () {
    'use strict';
    var vm = this;
    vm.$onInit = function () {
      vm.shouldShowTopBorder = (vm.showTopBorder === 'true');
      vm.isAltLayout = (vm.altLayout === 'true' || vm.layout === 'tall');
      vm.isMiniLayout = (vm.layout === 'mini');
    };
  }
});

would become..

@Component({
  selector: 'pf-aggregate-status-card',
  templateUrl: '/static/pf_charts/aggregate-status-card.html.haml',
  inputs: [ 'status', 'showTopBorder', 'altLayout', 'layout' ],
})
class pfAggregateStatusCard {
  ngOnInit() {
    this.shouldShowTopBorder = (vm.showTopBorder === 'true');
    this.isAltLayout = (vm.altLayout === 'true' || vm.layout === 'tall');
    this.isMiniLayout = (vm.layout === 'mini');
  }
}

and that's pretty much it.. :)

+- unexpected stuff :)

If you wanted to go full typescript right away, you'd also need...

class pfAggregateStatusCard implements OnInit {
  private status: string;
  private showTopBorder: string?;
  private altLayout: string?;
  private layout: string?;

To indicate the types and required/optional for those fields, and to mention it implements the ngOnInit function.

Oh.. and templates.. Right now, we can't have an angular2 template in haml - there's a haml extension for angular2, which works with haml but not hamlit.... Alternatively we could use the JS haml-loader and use template: require('path') instead of that templateUrl... but that means the haml conversion would happen during webpack, in JS code, so we could no longer use i18n there..

That's something I need to look into still... If you wanted to do the angular2 version right away, the only option is html, not haml.

(Unless the only logic in that template is {{foo}} .. in that case, haml is fine.. it just can't quite handle attributes names like (foo) or [bar], etc.)

IMO it still feels like we're not quite there yet with angular2 and I'd rather merge this as an angular 1 dashboard that works, but if it can wait... (Or if you want to solve those issues yourself :).)

h-kataria · 2017-09-26T15:45:50Z

@himdel can you please point out which id were you referencing to in your comment.

About your comment on usage of patterfly.card yes that's the correct module and usage as far as i could tell from several examples here of different patternfly component usages in AngularJS http://www.patternfly.org/pattern-library/cards/aggregate-status-card/#/code/angular, maybe there is a way to include patternfly globally, i am unsure.

h-kataria · 2017-10-02T13:28:24Z

@martinpovolny @himdel i would like to get this PR merged and do any other suggested changes as a follow up PR, please review/merge.

himdel · 2017-10-02T15:03:29Z

@h-kataria sorry for the delay..

@himdel can you please point out which id were you referencing to in your comment.

Every time you write <component id=...> and mean something different by id than browser element id.

So.. <component id="foo_component_1"> is probably fine, but <component id="#{@record.id}"> sounds like a case where you probably didn't want to search for that element via $('#1000000002345') and so should call that record-id instead (or something).

But looks like you found them all .. only https://github.com/ManageIQ/manageiq-ui-classic/pull/1901/files#diff-1089af45db5ae37204a2abb2310cf1b0R8 remains :).

About your comment on usage of patterfly.card yes that's the correct module and usage as far as i could tell from several examples here of different patternfly component usages in AngularJS http://www.patternfly.org/pattern-library/cards/aggregate-status-card/#/code/angular, maybe there is a way to include patternfly globally, i am unsure.

I mean, it's OK to use them :). I was just concerned we're creating new components and pretending they're from patternfly.. but not a blocker either way :).

martinpovolny · 2017-10-03T09:21:07Z

So.. is probably fine, but sounds like a case where you probably didn't want to search for that element via $('#1000000002345') and so should call that record-id instead (or something).

But! The QE would be actually happy if you created some IDs that would have a relation to the entities being displayed. Such as <component id="component_#{@record.id}">. Such markup would help them write tests ;-)

Ping @mfalesni

h-kataria · 2017-10-03T13:31:15Z

@martinpovolny addressed your comment regarding adding ids for each component

martinpovolny · 2017-10-03T18:39:45Z

@h-kataria : just glanced over the CC issues, many are trivial to fix.

- Changed dashboard service calls to send up individual calls to build charts - some code cleanup to remove dummy heatmap data setup https://www.pivotaltracker.com/story/show/150048620

And other components that are needed to show Global Utilization charts: c3Chart, donutChart, donutPctChart, sparklineChart components along with their counterpart controllers, templates etc.

And Line chart component and related controllers, template, views to display Recent Hosts & Recent VMs line charts.

- Removed unused old code and moved files appropriately under ems_infra_dashboard directory - Addressed some rubocop warning in ems_infra_dashboard_service https://www.pivotaltracker.com/story/show/150048620

https://www.pivotaltracker.com/story/show/147962059

Addressed syntax related code climate warnings that were brought over by downloading patternfly components from patternfly repo. https://www.pivotaltracker.com/story/show/147962059

Added unique id for each component that can be referenced in QE tests https://www.pivotaltracker.com/story/show/147962059

https://www.pivotaltracker.com/story/show/147962059

- Added .catch to all API/http request calls. https://www.pivotaltracker.com/story/show/147962059

himdel · 2017-10-11T10:58:22Z

@h-kataria I've found the source of the bug...

previously, pf-line-chart would never go without a ng-if="loadingDone".

Now, only heatmap does that, so the others need a similar bit of code as well.

(And yes, this could be considered a PF bug, but right now, we have to fix it :).)

Added loadingDone checks to make sure to load charts once data is available https://www.pivotaltracker.com/story/show/147962059

h-kataria · 2017-10-11T14:20:56Z

@himdel thx for your help on getting Error: url or json or rows or columns is required. error fixed. Please re-test/re-review. I think i have all the issues addressed now. @epwinchell is working on styling changes, we should have those soon.

epwinchell · 2017-10-12T14:38:35Z

@martinpovolny Could you have a look at what needs to be done to make the charts resize inside the cards like the main dashboard and cap & u charts? (The donut charts may be an exception) Thanks.

martinpovolny · 2017-10-13T11:32:33Z

@martinpovolny Could you have a look at what needs to be done to make the charts resize inside the cards like the main dashboard and cap & u charts? (The donut charts may be an exception) Thanks.

I searched the history and believe that what I did is gone with the last bits of jqplot. It must have been somehow redone for C3, but that was not me (or I don't remember). Ping @PanSpagetka, Robin, can you answer this, please?

https://www.pivotaltracker.com/story/show/147962059

miq-bot · 2017-10-14T21:58:41Z

himdel · 2017-10-16T14:26:58Z

so..another round of ui testing..

the single provider logo is consistently smaller now

EDIT: but it matches the PF style 👍

the clusters/hosts/vms... bar now uses black icons not blue, and those are visible even when the number is 0 (on master, they were visible only when nonzero)

EDIT: black is right, the the icon being always visible makes more sense anyway 👍

it also says "0 Cluster" instead of "0 Clusters"

the clusters/hosts/vms... bar sometimes shows different labels and/or different things

EDIT: but deployment role is the same thing as cluster, except different name

cluster utilization is larger when no data

EDIT: but does not break anything 👍

global utilization never has any data, even though it should

EDIT: this is caused by the difference in date range .. 30.days.ago 👍 (https://github.com/ManageIQ/manageiq-ui-classic/pull/1901/files#diff-482f2c40348265e920c212dd5ba8e8a4R161)

epwinchell · 2017-10-16T15:03:16Z

the clusters/hosts/vms... bar now uses black icons not blue..

@himdel This is correct Patternfly. Containers, specifically styled theirs blue.

https://rawgit.com/patternfly/patternfly/master-dist/dist/tests/cards.html
http://www.patternfly.org/pattern-library/dashboard/dashboard-layout/

himdel · 2017-10-16T15:35:38Z

So, any remaning problems are not really that important, merging..

himdel · 2018-05-15T10:28:56Z

app/assets/javascripts/controllers/ems_infra_dashboard/aggregate_status_card_controller.js

+    $q.all([promiseProviderData]).then(function() {
+      vm.status = {
+//         "title": vm.provider.name,
+        "iconImage": "/assets/svg/vendor-" + getIcon(vm.provider.type) + ".svg",


This does not work outside the development enrionment.

Image paths must be created using the image_path helper, ruby-side

(noticed the same problem in #3806)

h-kataria added dashboards enhancement wip labels Aug 14, 2017

h-kataria assigned himdel Aug 14, 2017

h-kataria force-pushed the angular_infra_dashboards branch from 75a8e99 to 404a986 Compare August 15, 2017 20:18

h-kataria force-pushed the angular_infra_dashboards branch 2 times, most recently from 30a9768 to 3fef8cc Compare August 27, 2017 03:29

h-kataria mentioned this pull request Aug 28, 2017

Add alerts to dashboard #1234

Merged

h-kataria force-pushed the angular_infra_dashboards branch 7 times, most recently from b15fbae to 25bfb1c Compare September 20, 2017 20:34

h-kataria removed the wip label Sep 25, 2017

h-kataria changed the title ~~[WIP] - Angular infra dashboards~~ Angular infra dashboards Sep 25, 2017

dclarizio added the fine/no label Sep 28, 2017

h-kataria force-pushed the angular_infra_dashboards branch 3 times, most recently from 1459d4e to ccca920 Compare September 28, 2017 18:56

h-kataria force-pushed the angular_infra_dashboards branch from 1e3768e to 58e6f50 Compare October 3, 2017 13:30

h-kataria added 11 commits October 9, 2017 11:39

Separated out dashboard service calls.

b44384e

- Changed dashboard service calls to send up individual calls to build charts - some code cleanup to remove dummy heatmap data setup https://www.pivotaltracker.com/story/show/150048620

Added Utilization Trend chart component

c2f7184

And other components that are needed to show Global Utilization charts: c3Chart, donutChart, donutPctChart, sparklineChart components along with their counterpart controllers, templates etc.

Added Line chart component

557df6f

And Line chart component and related controllers, template, views to display Recent Hosts & Recent VMs line charts.

Removed unused old code and moved files to appropriate directory

3938612

- Removed unused old code and moved files appropriately under ems_infra_dashboard directory - Addressed some rubocop warning in ems_infra_dashboard_service https://www.pivotaltracker.com/story/show/150048620

Changes to fix existing jasmine spec test failures

1c274db

https://www.pivotaltracker.com/story/show/147962059

Show "No Data Available" component when recent Hosts/VMs not available

c1d237e

https://www.pivotaltracker.com/story/show/147962059

Deleted some unused code.

e7ace95

https://www.pivotaltracker.com/story/show/147962059

Addressed code climate warnings.

331fc2f

Addressed syntax related code climate warnings that were brought over by downloading patternfly components from patternfly repo. https://www.pivotaltracker.com/story/show/147962059

Addressed PR comment.

ecd5d0b

Added unique id for each component that can be referenced in QE tests https://www.pivotaltracker.com/story/show/147962059

Addressed some more cc warnings

6393290

https://www.pivotaltracker.com/story/show/147962059

Fixed miq_bootstrap call

9fcd199

- Added .catch to all API/http request calls. https://www.pivotaltracker.com/story/show/147962059

h-kataria force-pushed the angular_infra_dashboards branch from 7465f68 to 9fcd199 Compare October 9, 2017 15:45

Added loadingDone checks

9cdaca2

Added loadingDone checks to make sure to load charts once data is available https://www.pivotaltracker.com/story/show/147962059

Styling updates

0c6e34c

epwinchell approved these changes Oct 12, 2017

View reviewed changes

Set "Last xx days" label in JS controller instead of hardcoding in view

d663770

https://www.pivotaltracker.com/story/show/147962059

himdel merged commit fd0d087 into ManageIQ:master Oct 16, 2017

himdel added this to the Sprint 71 Ending Oct 16, 2017 milestone Oct 16, 2017

h-kataria mentioned this pull request Oct 17, 2017

Container project dashboard view #1527

Merged

h-kataria deleted the angular_infra_dashboards branch October 24, 2017 03:30

himdel reviewed May 15, 2018

View reviewed changes

himdel mentioned this pull request May 15, 2018

Add a dashboard view for a physical infra provider #3806

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Angular infra dashboards #1901

Angular infra dashboards #1901

h-kataria commented Aug 14, 2017 •

edited

h-kataria commented Sep 20, 2017

himdel commented Sep 25, 2017 •

edited

h-kataria commented Sep 26, 2017

h-kataria commented Oct 2, 2017

himdel commented Oct 2, 2017 •

edited

martinpovolny commented Oct 3, 2017 •

edited

h-kataria commented Oct 3, 2017

martinpovolny commented Oct 3, 2017

himdel commented Oct 11, 2017 •

edited

h-kataria commented Oct 11, 2017

epwinchell commented Oct 12, 2017 •

edited

martinpovolny commented Oct 13, 2017

miq-bot commented Oct 14, 2017

himdel commented Oct 16, 2017 •

edited

epwinchell commented Oct 16, 2017 •

edited

himdel commented Oct 16, 2017

himdel May 15, 2018 •

edited

Angular infra dashboards #1901

Angular infra dashboards #1901

Conversation

h-kataria commented Aug 14, 2017 • edited

h-kataria commented Sep 20, 2017

himdel commented Sep 25, 2017 • edited

h-kataria commented Sep 26, 2017

h-kataria commented Oct 2, 2017

himdel commented Oct 2, 2017 • edited

martinpovolny commented Oct 3, 2017 • edited

h-kataria commented Oct 3, 2017

martinpovolny commented Oct 3, 2017

himdel commented Oct 11, 2017 • edited

h-kataria commented Oct 11, 2017

epwinchell commented Oct 12, 2017 • edited

martinpovolny commented Oct 13, 2017

miq-bot commented Oct 14, 2017

himdel commented Oct 16, 2017 • edited

epwinchell commented Oct 16, 2017 • edited

himdel commented Oct 16, 2017

himdel May 15, 2018 • edited

Choose a reason for hiding this comment

h-kataria commented Aug 14, 2017 •

edited

himdel commented Sep 25, 2017 •

edited

himdel commented Oct 2, 2017 •

edited

martinpovolny commented Oct 3, 2017 •

edited

himdel commented Oct 11, 2017 •

edited

epwinchell commented Oct 12, 2017 •

edited

himdel commented Oct 16, 2017 •

edited

epwinchell commented Oct 16, 2017 •

edited

himdel May 15, 2018 •

edited