UI: Service Instances #5326
UI: Service Instances #5326
Conversation
1. Move healthcheck-status component to healthcheck-output 2. Create a new healthcheck-status component for showing the number of checks plus its icon 3. Create a new healthcheck-info component to group multiple statuses plus a diffferent view if there are no checks
The used to be munged in amongst the table css file, but now we have a component just for these it makes sense that they should have a CSS 'module' also
Looking at the diff the old code produced: ``` Healthcheck Passing ``` Note the important return there. We actually want: ``` Healthcheck Passing ``` If not it affects the layout on narrow screens.
1. This feature removes a certain amount of functionality. So we just remove the acceptence tests for this old functionality here. 2. We also do a basic amount of edits to make the default ember tests pass (these are generally 'does this thing even render' or 'can I actually instantiate this thing?' New tests will be added on top of this
| passingCount: subscribe('passing', count), | ||
| warningCount: subscribe('warning', count), | ||
| criticalCount: subscribe('critical', count), | ||
| }); |
There was a problem hiding this comment.
We've used our pure computed property helper here (details here #5079). I did wonder whether to start using this here or not as I'm unsure of what impact this has when/if we eventually move to decorators/@computed. My only concern here is having to create our own @subscribe decorator and having to switch out all instances of this to use our own decorator. We figured in the scheme of things it won't be much effort to do that manually.
An interesting point here is the benefit of the pure function, it's concerned with only the arguments that you pass to it and returning a value. It doesn't matter what the name of the property is for example:
passingCount: computed('passing', function() {
get(this, 'passing')
// do something with passing
},
warningCount: computed('warning', function() {
get(this, 'warning')
// do the same thing but with warning
},
criticalCount: computed('critical', function() {
get(this, 'critical')
// do the same thing again but with critical
}
)..using a pure computed function and thinking about subscribeing to when a property changes, means this code easily becomes less verbose. Last little note here I'm not 100% convinced on the subscribe word here, but I definitely didn't want to reuse computed for clarity.
We'd be happy for this to use traditional computed if anyone had concerns here.
There was a problem hiding this comment.
I would typically write this has a computed property macro:
const lengthOrValue = prop => computed(prop, function() {
const value = this.get(prop);
if (Array.isArray(value)) {
return value.length;
}
return value;
}
// ...
passingCount: lengthOrValue('passing'),Which eliminates the concern of computed vs. subscribed.
I'm not sure how I feel about the name subscribes, since I would expect to see computed, but I'm also not a fan of calling something computed that isn't Ember.computed, so subscribe seems like a fine name.
There was a problem hiding this comment.
Unrelated to naming, how do you feel about partial application for the purify helper? Something like...
const count = subscribe(function(value) {
if (Array.isArray(value)) {
return value.length;
}
return value;
});
// ...
passingCount: count('passing')
There was a problem hiding this comment.
Can't these (at least in this case) use the built-in computed macro alias ? passingCount: alias('passing.length')
There was a problem hiding this comment.
Alias won't account for the case where passing isn't a length but is instead the count value already.
There was a problem hiding this comment.
aha, thanks - thought there might be a regular input
There was a problem hiding this comment.
@DingoEatingFuzz , I would like to replace all computed with subscribe throughout the app. I prefer the pure approach, pass arguments in - return something out, touch as little as possible apart from what is passed in via arguments:
computed(
'passing',
function() { // missed opportunity to use arguments
// this is my particular gripe, `subscribe` gives it to me as an argument
// so I don't even need to import `get`
return get(this, 'passing')
}
)If you get time take another read of the PR for the purify thing, there's a lot of info there on other reasons why I prefer it.
Anyway time for me to finish up, lets chat Monday, have a good weekend!
There was a problem hiding this comment.
I have read the purify PR and I honestly don't get the motivation. I see the difference, and I agree with both yours and Tom's reasoning that dependent keys are nothing more than "hints" for Ember, but if this is a problem that will be addressed in Ember core, why take it on as your own problem?
Now what you have is an incomplete leaky abstraction over the top of Ember.computed that has no documentation.
Incomplete because as you mention, the args.map tries its hardest to convert dependent keys into the correct object properties, but it doesn't support the entire DSL (not to mention the DSL has ambiguity since it isn't necessarily developer intent).
Leaky because as a developer you still have to be conscious of Ember.computed, how it works and the dependent key DSL.
It just doesn't seem pragmatic to attempt to gloss over a cornerstone feature of the framework.
There was a problem hiding this comment.
Hey @DingoEatingFuzz
Sorry, I just meant to give it another pass if you had time. The main reason for purify is to make the function you pass to computed pure (which is why it's called purify). From your comments above I didn't get the impression that you'd taken that away so thought it might be useful to take another pass at it. For example:
Which eliminates the concern of
computedvs.subscribed.
If the main reason/concern for purify is to make the function you pass to computed pure, then let's use a good definition of a pure function to begin:
A pure function is a function where the return value is only determined by its input values, without observable side effects
In your code above:
const value = this.get(prop);This/this isn't an input value. To address your point directly, the concern of computed vs subscribe isn't eliminated here.
Maybe a familiar example might help:
filteredJobs: computed(
'visibleJobs.[]',
'selectionType',
'selectionStatus',
'selectionDatacenter',
'selectionPrefix',
function() {
const {
selectionType: types,
selectionStatus: statuses,
selectionDatacenter: datacenters,
selectionPrefix: prefixes,
} = this.getProperties(
'selectionType',
'selectionStatus',
'selectionDatacenter',
'selectionPrefix'
);
return this.get('visibleJobs').filter(job => {
...
});
}
),could be rewritten as:
filterJobs: subscribe(
'visibleJobs.[]',
'selectionType',
'selectionStatus',
'selectionDatacenter',
'selectionPrefix',
function(jobs = [], types = [], statuses = [], datacenters = [], prefixes = []) {
return jobs.filter(
job => {
...
}
)
}My reasons for doing this are that it's much easier to read and follow what's happening, and you get the additional benefit of being able to use default arguments. Good for type hinting and good for avoiding more complicated defensive coding. I can also hoist the function up and out if I want without having to worry about depending on this or get. I can then reuse it and/or test it in isolation.
Considering we use computed a lot in our codebases, which version of the above would you prefer to write multiple times?
I've read over your other points here a few times and wondered how to respond. I think it's important and fair that I touch on your points here, but I don't want to get too far into the weeds. More than happy to chat about this later if you need more clarification.
Overall I think we can all agree with Tom that this is a problematic. I don't see purify as a 'solution to a problem' more of a slight improvement.
I've decided to write these ~10-20 lines of code to help me reduce boilerplate, which in turn helps our users. As and when the ember core team release a different approach, I'll consider using that instead:
We then identify existing tools that simplify the workflow. If a sufficient tool does not exist, we step in to build it. This leads to a fundamentally technology-agnostic view — we will use the best technology available to solve the problem. As technologies evolve and better tooling emerges, the ideal workflow is just updated to leverage those technologies. Technologies change, end goals stay the same.
Your other points:
- Now what you have is an incomplete leaky abstraction over the top of
Ember.computedthat has no documentation.- Incomplete because as you mention, the
args.maptries its hardest to convert dependent keys into the correct object properties- Leaky because as a developer you still have to be conscious of
Ember.computed, how it works and the dependent key DSL.- It just doesn't seem pragmatic to attempt to gloss over a cornerstone feature of the framework.
In my eyes these are Straw Man arguments.
- Incomplete
// Right now this just takes the first part of the path so:
// `items.[]` or `[email protected]` etc
// gives you `items` which is 'probably' what you expect
// it won't work with something like `item.objects.[]`
// it could potentially be made to do so, but we don't need that right now at leastit could potentially be made to do so, but we don't need that right now at least
We don't need it so we didn't add it. This is not something that is meant to be shared, we are the only authors and consumers. It is 'complete' for what we want to use it for. Have we finished working on this? Maybe, maybe not. The only thing it doesn't support is Ember's unique syntax. If we want to use that we could just use alias first.
As a comparison, as far as I'm aware Ember angle bracket syntax isn't supported during testing, and doesn't support all types of components for example {{link-to}}. IMO this is more 'incomplete' - it doesn't stop anyone from using it though.
So, it's not incomplete, I could add extra features that I don't need, yes, but not 'incomplete'.
- Leaky 😱
This one really confused me.
Leaky because as a developer you still have to be conscious of
Ember.computed, how it works and the dependent key DSL.
...because as a developer I have to be conscious of Ember.computed? I don't understand completely how this phrase defines the term 'leaky'? Leaky as in abstraction leaky? As in leaking the details of the underlying implementation?
// passingCount subscribes to changes to `passing` and counts the new value
passingCount: subscribe('passing', count)passingCount subscribes to changes to passing and counts the value
I don't see how the underlying implementation is 'leaked' anywhere here. Am I missing something? If I change the name of count to lengthOrValue instead, then yes that would be leaking the implementation of counting into the abstraction (in this case, the name of the function).
I am aware that it happens to use computed just because I wrote it, but I guess I could implement what I've done using something else, like traditional EventListeners or maybe ember observables. I wouldn't have to change how I would use this if I did so.
There is no leaky abstraction here. There is no mention of computed, get or actually anything to do with Ember. It could be using computed, or observables, or maybe even EventSources - and you'd never know.
Maybe I've misunderstood what you meant.
- Tries its hardest 🙀
the
args.maptries its hardest to convert dependent keys into the correct object properties, but it doesn't support the entire DSL
I'm not sure what you meant by 'tries its hardest'? Sounds terrible! One of the aims of a Straw Man argument is to cause alarm. The code here doesn't 'try its hardest', it doesn't try at all, hopefully the comment in the code explains that.
- Has no documentation
Most of the Consul UI is undocumented, the same for Vault and Nomad. For example, I've also noticed you do something very similar here without any documentation:
https://github.com/hashicorp/nomad/blob/f29c3e81a2ab69474c105a85cd8ef4d87830b8f8/ui/app/controllers/clients/index.js#L40-L43
https://github.com/hashicorp/nomad/blob/f29c3e81a2ab69474c105a85cd8ef4d87830b8f8/ui/app/utils/qp-serialize.js
To be fair, here you are factually correct that there is no real documentation for this function. I'd be happy to add some if you think its useful. But I think that in the scheme of things this just isn't really important, which is why I'd still see this as "attacking a Straw Man"
Lastly, there is one and only one reason why I am hesitant to use purify and that is because at some point we'll be able to write code like:
export default class TodoList extends Component {
@computed('todos.length')
get uncompletedTodos() {
return this.todos.filter(todo => !todo.completed);
}
}My concern is the upgrade path from subscribe to be able to do this. I've considered this and decided that I am still ok using subscribe. Why?:
- If I don't tie any of my code to
subscribesay by wrapping it up like you have done above, instead I compose functionality. I can drop back easily if need be:
export default class TodoList extends Component {
@computed('passing')
get passingCount () { // you can't pass arguments to a getter
return count(this.passing);
}
}- Potentially I could write a
@subscribedecorator, like other people have written their own decorators. - I've no idea when decorators are going to land and I can use them. In my experience it can be at least a year of waiting (I'm not complaining). I get the feeling that this might be quicker, but who knows 🤷♂️
Hopefully that helps to explain where I'm coming from. Feel free to ping me offline if you'd like to discuss further.
Thanks
John
There was a problem hiding this comment.
Skipping over your explanation of what purify is, since I already told you I know what it is, what it does, and how it works, I'll restate my case on the leaky and incomplete parts.
As I already mentioned, it's leaky because purify + computed (aka subscribe) still relies on the user of subscribe to understand ember's DK syntax and semantics. Thus the details of the underlying behavior (computed) leaks.
I also already stated my case about the abstraction being incomplete, but I'll try to be clearer this time. When I say "tries its best" I am mostly referring to this comment directly from the purify source.
it won't work with something like
item.objects.[]
Sure, you don't need this so you didn't make it work, but the fact remains that to no fault of your own, dependent keys can't be perfectly mapped to the properties the author of a computed property may want to use.
Which brings us to the actual point of my comment that you neglected to mention in your response: I don't understand the motivation and it isn't pragmatic.
Computed properties are member functions, so why bother making them pure in the first place? Computed property macros (which are in fact a documented feature of ember) generate member functions. This is the intended way to DRY out common computed properties (as further evidenced by the framework provided macros such as alias, collect, etc.).
And since computed properties are already a thing, and computed property macros are already a thing, and the remaining blemish of having to declare dependent keys is already a problem being addressed by the framework, there is no case to be made for introducing your own replacement being pragmatic.
| return 0; | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
This component is now used to render healthcheck 'outputs' i.e. the log information and further detail for the healthchecks, it basically groups {{healthcheck-output}}. These 2 components have been created as we are now rendering this list of healthcheck outputs in various places in the UI. I've left the old implementation in the Nodes Detail
| return value.length; | ||
| } | ||
| return value; | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Again subscribe is handy here, but not completely necessary. Potentially, we could hoist the function itself up and out of this file and reuse it here and above in healthcheck-info
| export default Component.extend({ | ||
| tagName: 'dl', | ||
| classNames: ['tag-list'], | ||
| }); |
There was a problem hiding this comment.
Tag listing is now being used in various places, so it made sense to make it into a simple component to centralize the markup.
There was a problem hiding this comment.
big plus one to this, I didn't do this early enough in vault and we now need to make a pass to contain markup for various things and bundle them into components.
There was a problem hiding this comment.
Yeah there's more to come here. There's a lot in the feature that requires me to reuse things that originally were only in one place but are now in multiple places. I quibbled a lot about what to change in this PR as I didn't want to move too much around. Following on from this there is likely to be a lot of 'lets make that a component now its used in more than one place'.
| healthy: computed('filtered', function() { | ||
| return get(this, 'filtered').filter(function(item) { | ||
| return sumOfUnhealthy(item.Checks) === 0; | ||
| }); |
There was a problem hiding this comment.
We no longer filter by healthcheck status in the 'Service Instances' page
| {{healthcheck-status width=warningWidth name='warning' value=warningCount}} | ||
| {{healthcheck-status width=criticalWidth name='critical' value=criticalCount}} | ||
| </dl> | ||
| {{/if}} |
There was a problem hiding this comment.
New healthcheck-info component that composes 3 new healthcheck-status now that this is being used in more places
Visually it's this:
| <span>{{item}}</span> | ||
| {{/each}} | ||
| </dd> | ||
| {{/if}} |
There was a problem hiding this comment.
New simple {{tag-list}} component to centralize markup
| @@ -19,7 +19,7 @@ | |||
| <td data-test-service-name="{{item.Service}}"> | |||
| <a href={{href-to 'dc.services.show' item.Service}}> | |||
| <span data-test-external-source="{{service/external-source item}}" style={{{ concat 'background-image: ' (css-var (concat '--' (service/external-source item) '-color-svg') 'none')}}}></span> | |||
| {{item.Service}}{{#if (not-eq item.ID item.Service) }}<em data-test-service-id="{{item.ID}}">({{item.ID}})</em>{{/if}} | |||
| {{item.Service}}{{#if (not-eq item.ID item.Service) }} <em data-test-service-id="{{item.ID}}">({{item.ID}})</em>{{/if}} | |||
There was a problem hiding this comment.
Ah 😄 I noticed this space had been lost in a change somewhere, I changed it to a so it less easy to mistakenly remove.
| } | ||
| }; | ||
| export const subscribe = purify(computed); | ||
| export default purify; |
There was a problem hiding this comment.
The main changes here are to give us an easily preconfigured subscribe using purify as we don't want to have to set it up everytime we want to use it.
| @@ -123,31 +123,6 @@ Feature: components / catalog-filter | |||
| | Model | Page | Url | | |||
There was a problem hiding this comment.
All changes to tests below are removing user paths that no longer exist or tweaking existing paths that are slightly changed. More tests to come here to test new paths/functionality.
| passingCount: subscribe('passing', count), | ||
| warningCount: subscribe('warning', count), | ||
| criticalCount: subscribe('critical', count), | ||
| }); |
There was a problem hiding this comment.
I would typically write this has a computed property macro:
const lengthOrValue = prop => computed(prop, function() {
const value = this.get(prop);
if (Array.isArray(value)) {
return value.length;
}
return value;
}
// ...
passingCount: lengthOrValue('passing'),Which eliminates the concern of computed vs. subscribed.
I'm not sure how I feel about the name subscribes, since I would expect to see computed, but I'm also not a fan of calling something computed that isn't Ember.computed, so subscribe seems like a fine name.
| passingCount: subscribe('passing', count), | ||
| warningCount: subscribe('warning', count), | ||
| criticalCount: subscribe('critical', count), | ||
| }); |
There was a problem hiding this comment.
Unrelated to naming, how do you feel about partial application for the purify helper? Something like...
const count = subscribe(function(value) {
if (Array.isArray(value)) {
return value.length;
}
return value;
});
// ...
passingCount: count('passing')| actions: { | ||
| sortChecksByImportance: function(a, b) { | ||
| const statusA = get(a, 'Status'); | ||
| const statusB = get(b, 'Status'); |
There was a problem hiding this comment.
This is where Enums are nice. This sort is essentially ordering everything Critical, then Warning, then Passing?
You could do...
const precedence = {
passing: 0,
warning: 1,
critical: 2,
};
// ...
sortChecksByImportance: function(a, b) {
const statusA = precedence[get(a, 'Status')];
const statusB = precedence[get(b, 'Status')];
return statusB - statusA;
}There was a problem hiding this comment.
Ah nice! Very helpful, thankyou.
When I come to refactor this stuff I can use this.
| // This method is called immediately after `Route::setupController`, and done here rather than there | ||
| // as this is a variable used purely for view level things, if the view was different we might not | ||
| // need this variable | ||
| set(this, 'selectedTab', 'service-checks'); |
There was a problem hiding this comment.
Does it make sense to encode selectedTab as a query param? Or a child route?
There was a problem hiding this comment.
I thought the same originally, it did use to be like this but it was decided not to 'remember' it. (Remember this is all implemented in the same way as the rest of the app, I just copied the plumbing over to here)
| @@ -10,6 +10,15 @@ | |||
| display: flex; | |||
| align-items: flex-start; | |||
| } | |||
| %app-view header dl { | |||
| float: left; | |||
There was a problem hiding this comment.
Generally I'll reach for float first as it will automatically 'JustWork' when you squeeze up your screen, so floated elements will drop underneath each other into a column view. flex won't do that without you adding additional @media rules. Actually I can't remember whether I've looked at this properly from a responsive angle, will give it a double check.
There was a problem hiding this comment.
It should, and you can get finicky with things like flex-grow, flex-shrink, and flex-basis. IMO, it's better than having to deal with the ol' clearfix.
There was a problem hiding this comment.
Hi @DingoEatingFuzz ,
The property to use to implement this with flex would be flex-wrap. Here's what things look like when I change the CSS by switching out the float for a flex:
Before I go further into this. I'd be interested to know what the basis of:
IMO, it's better than having to deal with the ol'
clearfix.
..is?
I have my opinions on using a<div class="clearfix">, and just to note, I don't like using them at all, but I'm interested to know what your reasons are? Could you explain exactly why you don't like using "the ol' clearfix".
Thanks,
John
There was a problem hiding this comment.
The name says it all: clearfix. It's a workaround caused by float never being meant for layouts. Sure, it works like a charm, but flexbox and grid were introduced as proper solutions to the hole float/clearfix filled.
The property to use to implement this with
flexwould beflex-wrap.
There isn't one property to use. It's going to take a combination of things. flex-wrap will solve the browser squeeze issue you mentioned, but it alone isn't going to get you the layout you want, as your screenshot shows.
Before I go further into this.
It isn't a critical issue, so there is not need to go further into it unless you have time on your hands. I only bring it up because I see flexbox as the successor to this float/clearfix pattern.
There was a problem hiding this comment.
The name says it all: clearfix
Your exact reasons for not using something is just because of its name?
My exact reasons for not using an extra clearfix div is because I try to avoid adding markup to templates just for styling concerns, it makes it harder to build a responsive webapp.
It's important to note here, I don't use a clearfix anywhere here. This is another Straw man
Completely agree it's not critical and this isn't important in the scheme of things, and I definitely don't have time on my hands! But it is related to something that keeps coming up.
I do use clear, a CSS Level 1 property. Using float and clear here prevent me from adding extra markup for styling concerns or over-complicating things. Those are my reasons. Under slightly different circumstances I might have considered a flex approach.
| }); | ||
| }, | ||
| setupController: function(controller, model) { | ||
| this._super(...arguments); |
There was a problem hiding this comment.
I don't know if you still want to call super here - pretty sure just does controller.set('model', model) so now you'll have model.item, model.proxy, item, and proxy in your template context.
There was a problem hiding this comment.
Hey @meirish
Sorry, I forgot about this and merged! Agree that it's questionable whether super should be called here or not.
I'm going to look into this a little further. I'm not sure if it only sets variables on the controller for you. I vaguely remember reading a blog article saying you should always call super in setupController otherwise things can go wrong - but I might be misremembering the method it was talking about, pretty sure it was this though as I remember a while ago going through add adding these supers in everywhere in places that I'd missed.
Up until now I've found it really difficult to know whether something in ember is deemed a 'hook' (and therefore a super call isn't needed) and when something is 'accepted overriding of a method' and therefore a super call is definitely needed. I've mostly just been using super everywhere unless its very clear that its a hook (for example in the model hook). I also checked to see what effect calling super has when a method on the super class doesn't exist, and as far as I remember its essentially a noop.
Anyway lemme look into it further.
Thanks
There was a problem hiding this comment.
Ok found the blog post:
But, reading that more carefully and looking at the ember source:
The 'things can go wrong' is that a model variable isn't set in the template context/controller.
In my case I never use model to access things so I could just remove this? I can do this in another PR.
Thanks,
There was a problem hiding this comment.
Added a PR here #5383
| // This method is called immediately after `Route::setupController`, and done here rather than there | ||
| // as this is a variable used purely for view level things, if the view was different we might not | ||
| // need this variable | ||
| set(this, 'selectedTab', 'service-checks'); |
There was a problem hiding this comment.
I thought the same originally, it did use to be like this but it was decided not to 'remember' it. (Remember this is all implemented in the same way as the rest of the app, I just copied the plumbing over to here)
| @@ -10,6 +10,15 @@ | |||
| display: flex; | |||
| align-items: flex-start; | |||
| } | |||
| %app-view header dl { | |||
| float: left; | |||
There was a problem hiding this comment.
Generally I'll reach for float first as it will automatically 'JustWork' when you squeeze up your screen, so floated elements will drop underneath each other into a column view. flex won't do that without you adding additional @media rules. Actually I can't remember whether I've looked at this properly from a responsive angle, will give it a double check.
| export default Component.extend({ | ||
| tagName: 'dl', | ||
| classNames: ['tag-list'], | ||
| }); |
There was a problem hiding this comment.
Yeah there's more to come here. There's a lot in the feature that requires me to reuse things that originally were only in one place but are now in multiple places. I quibbled a lot about what to change in this PR as I didn't want to move too much around. Following on from this there is likely to be a lot of 'lets make that a component now its used in more than one place'.
| passingCount: subscribe('passing', count), | ||
| warningCount: subscribe('warning', count), | ||
| criticalCount: subscribe('critical', count), | ||
| }); |
There was a problem hiding this comment.
@DingoEatingFuzz , I would like to replace all computed with subscribe throughout the app. I prefer the pure approach, pass arguments in - return something out, touch as little as possible apart from what is passed in via arguments:
computed(
'passing',
function() { // missed opportunity to use arguments
// this is my particular gripe, `subscribe` gives it to me as an argument
// so I don't even need to import `get`
return get(this, 'passing')
}
)If you get time take another read of the PR for the purify thing, there's a lot of info there on other reasons why I prefer it.
Anyway time for me to finish up, lets chat Monday, have a good weekend!
1. Remove `subscribe` and couting from healthcheck-info its not needed here at all 2. Switch back to `computed` in healthcheck-status 3. Make sure the healthcheck-info js component follows the rest of my CSS component structure instead of keeping the old healthcheck-status which is just a single icon rather than all 3 4. Fix CSS bug where the zero healthcheck icon wasn't being displayed.
This gives more prominence to 'Service Instances' as opposed to 'Services'. It also begins to surface Connect related 'nouns' such as 'Proxies' and 'Upstreams' and begins to interconnect them giving more visibility to operators. Various smaller changes: 1. Move healthcheck-status component to healthcheck-output 2. Create a new healthcheck-status component for showing the number of checks plus its icon 3. Create a new healthcheck-info component to group multiple statuses plus a different view if there are no checks 4. Componentize tag-list
This gives more prominence to 'Service Instances' as opposed to 'Services'. It also begins to surface Connect related 'nouns' such as 'Proxies' and 'Upstreams' and begins to interconnect them giving more visibility to operators. Various smaller changes: 1. Move healthcheck-status component to healthcheck-output 2. Create a new healthcheck-status component for showing the number of checks plus its icon 3. Create a new healthcheck-info component to group multiple statuses plus a different view if there are no checks 4. Componentize tag-list
This gives more prominence to 'Service Instances' as opposed to 'Services'. It also begins to surface Connect related 'nouns' such as 'Proxies' and 'Upstreams' and begins to interconnect them giving more visibility to operators. Various smaller changes: 1. Move healthcheck-status component to healthcheck-output 2. Create a new healthcheck-status component for showing the number of checks plus its icon 3. Create a new healthcheck-info component to group multiple statuses plus a different view if there are no checks 4. Componentize tag-list
This PR gives more prominence to 'Service Instances' as opposed to 'Services'. It also begins to surface Connect related 'nouns' such as 'Proxies' and 'Upstreams' and begins to interconnect them giving more visibility to operators.
The best way to explain this is probably with some screengrabs:
Before
After
Here we surface more detail for the 'Service Instance' that could be useful for operators, such as Service Instance healthchecks split by Node specific and Service Instance specific checks, also Service Instance specific Tags and Upstreams.
Implementation wise there are a few things that will change over the next week or so, and a few changes here that are not strictly feature changes. Generally this feature is 'run-of-the-mill' data renaming and the addition of further data, so I've tried to keep 'refactoring' changes that I've done here to a minimum in order to keep the changeset as small as possible.
Leading on from that information, there will be further PR's near term to add smaller features and improve on this feature before its merged into
master. There is also likely to be some refactoring PR's which I'll try to hint at inline - these may happen before or after this ends up inmaster.