-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add decorator for service & node name to allow using node metadata #70
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, but I definitely prefer creating an element (we would add icons and similar stuff easily then)
samples/consul-ui/decorators.js.erb
Outdated
@@ -36,6 +36,13 @@ function usefullLinksGenerator(instance, serviceName, node_meta_info) { | |||
return top; | |||
} | |||
|
|||
/** | |||
* instanceNameDecorator is called to resolve instance name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it resolves and display the nodeName, instance is too vague
samples/consul-ui/decorators.js.erb
Outdated
/** | ||
* instanceNameDecorator is called to resolve instance name. | ||
*/ | ||
function instanceNameDecorator(instance, nodemeta) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather call it: createNodeDisplayElement(nodeName, nodeMeta)
and return by default : document.createTextNode(nodeName)
=> So it can be used to display specific icons for instance
samples/consul-ui/js/utils.js
Outdated
instanceLink.setAttribute('target', '_blank'); | ||
} | ||
instanceLink.appendChild(document.createTextNode(nodename)); | ||
var name = instanceNameDecorator(node['Name'], node['Meta']) | ||
instanceLink.appendChild(document.createTextNode(name)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just do:
instanceLink.appendChild(createNodeDisplayElement(node['Name'], node['Meta']))
samples/consul-ui/js/utils.js
Outdated
instanceLink.appendChild(document.createTextNode(instance.name + appendPort)); | ||
var nodemeta = (node_info != null) ? node_info.meta : null; | ||
var name = instanceNameDecorator(instance.name, nodemeta) | ||
instanceLink.appendChild(document.createTextNode(name + appendPort)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just do:
instanceLink.appendChild(createNodeDisplayElement(instance.name, nodemeta))
ad0fed3
to
2bba6fa
Compare
In case meta needs to be used in place of node.name
2bba6fa
to
fd0024b
Compare
In case meta needs to be used in place of node.name