Navbarextensions improvements #9871
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
epixa
added
Team:Core
|
jenkins, test this |
|
Looks like the test got stuck: |
|
jenkins, test this |
|
one test failed, i'll check what's wrong: |
|
@scampi Can you rebase on master? We had an issue with the tests that I believe is fixed now. |
scampi
force-pushed
the
navbarextensions
branch
from
1e5e885 to
c9d5c0c
Compare
- added $location service to determine on which the user is at currently
|
@epixa rebased |
|
jenkins, test this |
|
ping ? |
|
Sorry @scampi I'll take a look at this today |
spalger
suggested changes
Mar 2, 2017
There was a problem hiding this comment.
In what way is this a controller? It doesn't serve the same function as a directive's controller right? It looks like maybe describing these as scope properties, or "locals" then injecting them into scope would be a bit more correct:
topNavController.addItem({
key: 'graph',
template: '<pre>{{controller.text}}</pre>',
locals: {
controller
}
})const $childScope = $scope.$new();
if (this.locals[currentKey]) {
Object.assign($childScope, this.locals[currentKey]);
}
const $el = $element.find('#template_wrapper').html(templateToRender).contents();
$compile($el)($childScope);| @@ -18,6 +18,7 @@ export default function ($compile) { | |||
| interval: intervalTemplate, | |||
| filter: filterTemplate, | |||
| }; | |||
| this.controllers = []; | |||
There was a problem hiding this comment.
I think this is supposed to be a map or object, not an array
|
I wanted to do something similar to what is done in the terms bucket and its html template file. But I agree that using the |
|
@spalger ready for another review, thanks! I have tested with the following extension: import registry from 'ui/registry/navbar_extensions';
registry.register(function () {
return {
appName: 'dashboard',
key: 'button',
order: 1,
template: '<div ng-controller="controller"><h2>Hello {{name}}</h2></div>',
locals: {
controller($scope, $timeout) {
$scope.name = "john";
$timeout(function () {
$scope.name = "conor";
}, 500);
}
}
};
}); |
spalger
approved these changes
Mar 23, 2017
|
This change LGTM. I'm curious why we need it though. You can get the same functionality right now using a custom directive with its own controller. import registry from 'ui/registry/navbar_extensions';
import modules from 'ui/modules';
const module = modules.get('example');
module.directive('nameToggleExample', () => {
return {
restrict: 'E',
scope: {
timeout: '=',
},
template: '<div><h2>Hello {{name}}</h2></div>',
controller($scope, $timeout) {
$scope.name = "john";
$timeout(function () {
$scope.name = "conor";
}, $scope.timeout || 500);
}
};
});
registry.register(function () {
return {
appName: 'dashboard',
key: 'button',
order: 1,
template: '<name-toggle-example timeout="500"></name-toggle-example>',
};
});This way That said, @spalger if it works for you, it works for me. |
|
I agree that locals can be injected with a custom directive, but I think this is simple enough to be useful and unless we reimplement angular's templating language, these navbar extensions will always be angular specific. If they were react templates then locals would be totally irrelevant (and we could drop support for them) because you could just pass values as props to your template. |
elastic-jasper
added a commit
that referenced
this pull request
Mar 24, 2017
Backports PR #9871 **Commit 1:** - added controller option - added $location service to determine on which the user is at currently * Original sha: ddc2eb5 * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-01-13T17:29:50Z **Commit 2:** added test for the hideButton method that is dependent on the location path * Original sha: f27a535 * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-01-13T23:43:14Z **Commit 3:** removed argument * Original sha: c9d5c0c * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-01-14T13:44:35Z **Commit 4:** use locals object to pass variables to the template's scope * Original sha: 07b7549 * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-03-04T21:26:33Z **Commit 5:** use map object * Original sha: f9ee311 * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-03-04T21:33:48Z
spalger
pushed a commit
that referenced
this pull request
Mar 24, 2017
Backports PR #9871 **Commit 1:** - added controller option - added $location service to determine on which the user is at currently * Original sha: ddc2eb5 * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-01-13T17:29:50Z **Commit 2:** added test for the hideButton method that is dependent on the location path * Original sha: f27a535 * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-01-13T23:43:14Z **Commit 3:** removed argument * Original sha: c9d5c0c * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-01-14T13:44:35Z **Commit 4:** use locals object to pass variables to the template's scope * Original sha: 07b7549 * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-03-04T21:26:33Z **Commit 5:** use map object * Original sha: f9ee311 * Authored by Stéphane Campinas <stephane.campinas@gmail.com> on 2017-03-04T21:33:48Z
That's what I meant, the functionality works that way today, it's just props into a custom element thing (angular here, but potentially react in the future, or anything else...). |
simianhacker
pushed a commit
to simianhacker/kibana
that referenced
this pull request
Mar 27, 2017
* - added controller option - added $location service to determine on which the user is at currently * added test for the hideButton method that is dependent on the location path * removed argument * use locals object to pass variables to the template's scope * use map object
simianhacker
added a commit
that referenced
this pull request
Mar 28, 2017
…#10890) * Removing derivative behavior... adding better tooltips to add delete * Re-organizing metrics; Adding headers; Adding default behavoir * Adding help text * Additional fixes - Fix for deactivated still being able to be added - Added Experimental Feature to description * [elasticsearch/healthCheck] ensure that multi.allow_explicit_index=true (#10855) * [elasticsearch/healthCheck] ensure that multi.allow_explicit_index=true * [elasticsearch/healthCheck] fix tests * [eslint/console] enable no-undef rule (#10881) * [eslint/console] enable no-undef rule * [eslint/console] fix no-undef violations * Navbarextensions improvements (#9871) * - added controller option - added $location service to determine on which the user is at currently * added test for the hideButton method that is dependent on the location path * removed argument * use locals object to pass variables to the template's scope * use map object * fixing percentage mode extents (#10843) * fixes axis title for new axis (#10866) * fixes metrics options matching (#10865) * Add kuiLocalTab-isDisabled state. (#10830) * Add kuiLocalTab-isDisabled state. * Update Management header to use kuiLocalTab-isDisabled class. * test: send content-type with proxy POST tests (#10903) Elasticsearch now requires that content-type be sent on all requests with payloads, so our tests should be sending it with requests as well. * [esArchiver] combine elasticDump and ScenarioManager (#10359) * As a part of bringing functional testing to plugins, esArchiver gives these plugins a way to capture and reload es indexes without needing to write a bunch of custom code. It works similarly to the elasticDump and ScenarioManager tools that it replaces. Differences: - Streaming implementation allows for much larger archives - CLI for creating and using archives - Configurable archive location - Stores the data in gzipped files (better for source control, searching, large archives) - Automatically identifies and upgrades Kibana config documents Methods: - `#load(name)`: import an archive - `#loadIfNeeded(name)`: import an archive, but skip the documents what belong to any existing index - `#unload(name)`: delete the indexes stored in an archive CLI operations: - `./bin/es_archiver save <name> [index patterns...]`: save the mapping and documents in one or more indexes that match the wild-card patterns into an the `<name>` archive - `./bin/es_archiver load <name>`: load the mapping and documents from the `<name>` archive * [functional_tests/common/nagivate] check for statusPage * [es_archiver] move bins into new scripts dir * [functional_tests/apps/context] use esArchiver * [esArchiver] general improvements after showing to a few folks - remove auto-upgrading config doc logic (until we have better access to kibana version info) - export unload command - remove preemptive checks in favor of reacting to errors - use type "doc" vs "hit" for doc records (consistency) - wrote a bunch of pending tests to think though and plan * [esArchiver] make log a stream that writes to itself * [esArchiver] fill in stats and archive format tests * [esArchiver] splitup action logic * [esArchiver/cli] fix cli --help output and comment * [esArchiver] remove type-based param coercion * [esArchiver/log] use strings for log levels * [esArchvier] remove unused var * [esArchiver/indexDocRecordsStream] add tests * [esArchive] fill in remaining tests * [esArchiver] fix dem tests * [eslint] remove unused vars * [esArchiver/loadIfNeeded] fix call to load() * [esArchiver] remove loadDumpData helpers
simianhacker
added a commit
that referenced
this pull request
Mar 28, 2017
…#10890) * Removing derivative behavior... adding better tooltips to add delete * Re-organizing metrics; Adding headers; Adding default behavoir * Adding help text * Additional fixes - Fix for deactivated still being able to be added - Added Experimental Feature to description * [elasticsearch/healthCheck] ensure that multi.allow_explicit_index=true (#10855) * [elasticsearch/healthCheck] ensure that multi.allow_explicit_index=true * [elasticsearch/healthCheck] fix tests * [eslint/console] enable no-undef rule (#10881) * [eslint/console] enable no-undef rule * [eslint/console] fix no-undef violations * Navbarextensions improvements (#9871) * - added controller option - added $location service to determine on which the user is at currently * added test for the hideButton method that is dependent on the location path * removed argument * use locals object to pass variables to the template's scope * use map object * fixing percentage mode extents (#10843) * fixes axis title for new axis (#10866) * fixes metrics options matching (#10865) * Add kuiLocalTab-isDisabled state. (#10830) * Add kuiLocalTab-isDisabled state. * Update Management header to use kuiLocalTab-isDisabled class. * test: send content-type with proxy POST tests (#10903) Elasticsearch now requires that content-type be sent on all requests with payloads, so our tests should be sending it with requests as well. * [esArchiver] combine elasticDump and ScenarioManager (#10359) * As a part of bringing functional testing to plugins, esArchiver gives these plugins a way to capture and reload es indexes without needing to write a bunch of custom code. It works similarly to the elasticDump and ScenarioManager tools that it replaces. Differences: - Streaming implementation allows for much larger archives - CLI for creating and using archives - Configurable archive location - Stores the data in gzipped files (better for source control, searching, large archives) - Automatically identifies and upgrades Kibana config documents Methods: - `#load(name)`: import an archive - `#loadIfNeeded(name)`: import an archive, but skip the documents what belong to any existing index - `#unload(name)`: delete the indexes stored in an archive CLI operations: - `./bin/es_archiver save <name> [index patterns...]`: save the mapping and documents in one or more indexes that match the wild-card patterns into an the `<name>` archive - `./bin/es_archiver load <name>`: load the mapping and documents from the `<name>` archive * [functional_tests/common/nagivate] check for statusPage * [es_archiver] move bins into new scripts dir * [functional_tests/apps/context] use esArchiver * [esArchiver] general improvements after showing to a few folks - remove auto-upgrading config doc logic (until we have better access to kibana version info) - export unload command - remove preemptive checks in favor of reacting to errors - use type "doc" vs "hit" for doc records (consistency) - wrote a bunch of pending tests to think though and plan * [esArchiver] make log a stream that writes to itself * [esArchiver] fill in stats and archive format tests * [esArchiver] splitup action logic * [esArchiver/cli] fix cli --help output and comment * [esArchiver] remove type-based param coercion * [esArchiver/log] use strings for log levels * [esArchvier] remove unused var * [esArchiver/indexDocRecordsStream] add tests * [esArchive] fill in remaining tests * [esArchiver] fix dem tests * [eslint] remove unused vars * [esArchiver/loadIfNeeded] fix call to load() * [esArchiver] remove loadDumpData helpers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
close #9860