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.

Sign up for GitHub

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

Jump to bottom

Topology Filters #1159

Merged
merged 2 commits into from
Mar 29, 2016
Merged

Topology Filters #1159

merged 2 commits into from
Mar 29, 2016

Conversation

davkal
Copy link
Contributor

@davkal davkal commented Mar 11, 2016

Fixes #1116

  • topology filters overhaul
  • allow SHOW X, SHOW Y (which is NOT X), SHOW BOTH

@davkal davkal changed the title Node States [WIP] Node States Mar 11, 2016
@paulbellamy paulbellamy mentioned this pull request Mar 15, 2016
Closed
@davkal davkal changed the title [WIP] Node States [WIP] Topology Filters Mar 17, 2016
@davkal davkal changed the title [WIP] Topology Filters Topology Filters Mar 17, 2016
@davkal davkal added this to the 0.14.0 milestone Mar 17, 2016
foot
foot reviewed Mar 17, 2016
View reviewed changes
client/app/scripts/stores/app-store.js
defaultOptions = defaultOptions.set(option, item.get('value'));
}
});
if (topology.has('options') && topology.get('options')) {

This comment was marked as abuse.

Sign in to view

@foot
Copy link
Contributor

foot commented Mar 17, 2016

I like the aesthetic!

  • Preloading positions by selecting app-containers then system-containers, and then both causes bad node overlap sometimes, can be force-layed-out though. I'm not sure if there's an easy fix here? I guess you could include filtered nodes in the layout and then hide, though this could lead to sub-optimal layouts...
  • Maybe give the unselected items a bg fill as they are already at 0.7opacity, looks quite nice. Otherwise a bit messy in the rare case of node/control overlap down there.
  • I quite like having the two bars the same length, but it sounds like that option was tried and outvoted. :)
  • Even when "both" is selected it says some nodes are still filtered, this is not a regression and i think i'm just noticing as i'm looking for little things, but maybe a tooltip? I don't even know what they are.

tomwilkie
tomwilkie reviewed Mar 21, 2016
View reviewed changes
app/api_topologies.go
Options: []APITopologyOption{
{"system", "System pods", render.FilterApplication},
{"application", "Application pods", render.FilterSystem},
{"both", "Both", render.FilterNoop},

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@foot foot assigned davkal and unassigned foot Mar 22, 2016
@davkal
Copy link
Contributor Author

davkal commented Mar 24, 2016

Current implementation:

screen shot 2016-03-24 at 14 11 28

Both in the middle:
screen shot 2016-03-24 at 14 12 31

@davkal
Copy link
Contributor Author

davkal commented Mar 24, 2016

@foot changed BG of unselected ones slightly and isolated topology caches, so nodes should overlap less PTAL

@foot
Copy link
Contributor

foot commented Mar 29, 2016

Topos look good!

Not sure about the BG! Looks a bit heavy now, I'd remove that actually :P.

But LGTM either way. 👍

@davkal davkal force-pushed the node-states branch 2 times, most recently from 76d1d05 to 98c265a Compare March 29, 2016 10:59
@davkal
Copy link
Contributor Author

davkal commented Mar 29, 2016

Fixes #1116

@davkal davkal merged commit 3744f47 into master Mar 29, 2016
@davkal davkal deleted the node-states branch March 29, 2016 11:54
@davkal davkal mentioned this pull request Apr 11, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@foot foot

Labels
None yet
Projects
None yet
Milestone
0.14.0
Development

Successfully merging this pull request may close these issues.
5 participants
@davkal @foot @rade @tomwilkie @paulbellamy