Skip to content
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

KIALI-1077 Another attempt at correctly handling root and outsider nodes #327

Merged
merged 1 commit into from Jul 2, 2018

Conversation

@jshaughn
Copy link
Contributor

commented Jun 30, 2018

I'm going to generate a PR that does two things:

  • remove the no outgoing edge restriction on outsider nodes.
  • disallow outsider roots.

That means that only insider traffic generators (and unknown) will get the isRoot=true and the subsequent diamond shape. This will distinguish insider traffic generator services from the rest of the insider services. It will also make all outsider services consistently show as a pentagon and make it easy to know which services support the navigation feature.

@jshaughn

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2018

screenshot

screenshot2

@jshaughn jshaughn requested review from cfcosta and jmazzitelli Jun 30, 2018
@jshaughn

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2018

@jmazzitelli , @cfcosta , @mwringe Can you guys give this a try and see if it delivers a better behavior? See the jira for a comment about semantics, it's really more about defining how we want things to behave than it is about the code. This latest pass seems like a better approach to me. I'll be away but I'll try to keep an eye on this.

@jmazzitelli

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2018

This "works" - but the UI is broken. Double-clicking does take me to the other namespace, but the selection is broken. When I get to the new namespace, some other node (apparently random) is selected. When switching to another namespace, the selection should be "de-selected" - no nodes should be selected. I think this PR can be merged. I'll write up a JIRA for this other bug since that bug is against the UI, not the server-side.

@jmazzitelli jmazzitelli merged commit cc6efeb into kiali:master Jul 2, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jshaughn jshaughn deleted the jshaughn:kiali-1077 branch Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.