Skip to content

Create topology graph component #395

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.

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

Merged

Conversation

dgutride
Copy link
Member

This ports the kubernetes-ui component created by Red Hat over to PatternFly with new functionality that has been used by other products.

Inclues unit tests, less for styling and an example of how to style specific nodes, search and filter.

* <ul style='list-style-type: none'>
* <li>.name - name of the item the node represents
* <li>.status - optional status of the node (can be used to differentiate the circle color)
* <li>.kind - the kind of node
Copy link
Member

Choose a reason for hiding this comment

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

Can you elaborate on what 'kinds' of charts are supported, and what the 'kind' values may be?
Thanks,

  • Dave

angular.module('patternfly.charts').component('pfTopology', {
bindings: {
items: '=',
relations: '=',
Copy link
Member

Choose a reason for hiding this comment

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

Hi, do these have to be two way bindings?

@dtaylor113
Copy link
Member

LGTM 👍

Copy link
Member

@jeff-phillips-18 jeff-phillips-18 left a comment

Choose a reason for hiding this comment

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

👍 LGTM, some minor issues/questions.

.pf-topology-svg g text.glyph {
font-size: 20px;
fill: #1186C1;
}
Copy link
Member

Choose a reason for hiding this comment

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

Lots of additions to A-PF, these should be in Patternfly. Is there a backlog story to move this to Patternfly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not yet - we'll make sure one gets in

* @restrict E
*
* @description
* Component for rendering a topology chart. Individual nodes and relationships can be represented with this view. CSS is especially important for rendering the noes and lines. The example inline contains specific examples that can be used to change the icon size and the line type of the relationships.
Copy link
Member

Choose a reason for hiding this comment

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

Typo: noes -> nodes

"fontfamily": "PatternFlyIcons-webfont"
}
},
});
Copy link
Member

Choose a reason for hiding this comment

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

Funky indentation.

font-size: 18px;
}
</file>

Copy link
Member

Choose a reason for hiding this comment

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

Is it the expectation that applications will want to define each of these?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, due to the fact that they are text icons - this is the way they'll need to be styled.

ctrl.$onInit = function () {
$element.css("display", "block");
options = {"force": ctrl.force, "radius": ctrl.radius};
//graph = topologyGraph($element[0], notify, options);
Copy link
Member

Choose a reason for hiding this comment

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

is this left in for a reason?

return "unknown";
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

May want to use case insensitive checks here.

@jeff-phillips-18 jeff-phillips-18 merged commit 22eb250 into patternfly:branch-4.0-dev Jan 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants