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

Paginated selectable list directive #6550

Closed
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
9c7c4d9
init commit
stormpython Mar 15, 2016
930a0eb
[startup] Write process pid file earlier as per #4680
bevacqua Mar 15, 2016
1aab5e0
troubleshooting
stormpython Mar 15, 2016
8455d56
almost ready
stormpython Mar 16, 2016
3aa06ab
removing commented code
stormpython Mar 16, 2016
c72c440
add support for a configPrefix setting
spalger Mar 16, 2016
c852844
remove configPrefix from test_bundle
w33ble Mar 16, 2016
5a488f4
restore the key validation
w33ble Mar 16, 2016
8790a79
add test for dot-notated config keys
w33ble Mar 16, 2016
041c2f6
adding back sort functionality to saved object and paginated selectab…
stormpython Mar 17, 2016
6939960
fixing issue with list not sorting
stormpython Mar 17, 2016
27c6f17
refactoring html and css
stormpython Mar 17, 2016
ec68aa0
Merge branch 'feature/design' into selectable_list_directive
stormpython Mar 17, 2016
f7cc5f1
empty schema test is not async
w33ble Mar 17, 2016
765185f
remove unused toPath
w33ble Mar 17, 2016
96e5937
actually check object keys in schema
w33ble Mar 17, 2016
41e3081
add some more tests around invalid config paths
w33ble Mar 17, 2016
1ff22b1
refactoring to allow for array of objects, removing useless css styles
stormpython Mar 17, 2016
242fe85
Merge branch 'feature/design' into selectable_list_directive
stormpython Mar 17, 2016
420c1bc
adding tests
stormpython Mar 18, 2016
2152047
correctly use Joi.object().keys(), finally
w33ble Mar 18, 2016
00f5527
Merge branch 'feature/design' into selectable_list_directive
stormpython Mar 18, 2016
b968b0b
adding more tests, refactoring directive
stormpython Mar 19, 2016
a8c1305
short url: ensure absolute path isn't persisted
jbudz Mar 18, 2016
2a71673
[short url] use url.format when creating /goto link
jbudz Mar 21, 2016
690a140
[short url] Add tests for query strings, no hashes
jbudz Mar 21, 2016
49d297f
[short url] Cleanup
jbudz Mar 21, 2016
a612a0e
[short url] Add server tests
jbudz Mar 21, 2016
1dc9a18
Merge branch 'feature/design' of github.com:elastic/kibana into selec…
stormpython Mar 22, 2016
d66df55
Merge pull request #6581 from jbudz/issues/6127
epixa Mar 22, 2016
149cc2e
fixing issue with background hover on paginated-selectable-list and s…
stormpython Mar 22, 2016
cf78a36
Merge pull request #6239 from elastic/feature/design
Mar 22, 2016
08bae15
Merge branch 'master' into selectable_list_directive
stormpython Mar 22, 2016
b126e35
move unset into its own module, include tests
w33ble Mar 22, 2016
115128b
use external unset module
w33ble Mar 22, 2016
2ebd94d
use unset instead of delete, fixes schema bug
w33ble Mar 22, 2016
08e9202
[npm] update babel
spalger Mar 22, 2016
12fe251
Merge pull request #6604 from spalger/update/babel/master
spalger Mar 22, 2016
7bbfa22
Merge pull request #6539 from bevacqua/feature/pid-file-earlier
spalger Mar 23, 2016
4408069
Merge pull request #6554 from w33ble/implement/pluginConfigPrefix
w33ble Mar 23, 2016
f458af2
Merge branch 'master' into selectable_list_directive
stormpython Mar 23, 2016
62316aa
Remove -reskin from 5.0 snapshot version
epixa Mar 23, 2016
66a8c7f
Merge branch 'master' into selectable_list_directive
stormpython Mar 23, 2016
2992a66
adding label filter for input on saved object filter placeholder
stormpython Mar 23, 2016
d90baae
removing div and adding display block to anchor tag to allow the enti…
stormpython Mar 23, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/plugins/kibana/public/visualize/styles/main.less
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@
padding: 0;
display: flex;

div.wizard-small {
flex: 2;
}

div.wizard-large {
flex: 3;
}

.wizard-column {
flex: 1;
display: flex;
Expand Down Expand Up @@ -45,7 +53,15 @@
.list-group {
margin-bottom: 0;

.list-group-item {
.list-group-item .list-group-item-menu:hover {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this list-group-item-menu class still in use?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see list-group-menu-item in use, but I don't see a style of it it (note the menu->item item->menu swap in the name). In any case it looks like everywhere it is used list-group-item is already a class on the element. Seems like it could go right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that can go.

background-color: transparent;
}

.list-group-item .list-group-item-menu.active {
background-color: transparent;
}

.list-group-item .list-group-item-menu {
border-radius: 0;
border: none;
}
Expand Down
23 changes: 9 additions & 14 deletions src/plugins/kibana/public/visualize/wizard/step_2.html
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
<div class="wizard">
<div class="wizard-column">
<h3>From a New Search</h3>
<!-- Index patterns -->
<div class="wizard-row">
<div class="panel panel-default">
<div class="panel-heading">Index Patterns</div>
</div>
<ul class="striped list-group">
<li class="list-group-item" ng-repeat="pattern in indexPattern.list | orderBy: 'toString()'">
<a class="index-link" kbn-href="{{ makeUrl(pattern) }}">{{pattern}}</a>
</li>
</ul>
</div>
<div class="wizard-small wizard-column">
<h3>From a New Search, Select Index</h3>
<paginated-selectable-list
per-page="20"
list="indexPattern.list"
user-make-url="makeUrl"
class="wizard-row">
</paginataed-selectable-list>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo paginataed -> paginated

</div>
<div class="wizard-column">
<div class="wizard-large wizard-column">
<h3>Or, From a Saved Search</h3>
<!-- Saved searches -->
<saved-object-finder
Expand Down
1 change: 1 addition & 0 deletions src/plugins/kibana/public/visualize/wizard/wizard.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'lodash';
import 'plugins/kibana/visualize/saved_visualizations/saved_visualizations';
import 'ui/directives/saved_object_finder';
import 'ui/directives/paginated_selectable_list';
import 'plugins/kibana/discover/saved_searches/saved_searches';
import routes from 'ui/routes';
import RegistryVisTypesProvider from 'ui/registry/vis_types';
Expand Down
2 changes: 0 additions & 2 deletions src/ui/public/directives/paginate.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,5 +194,3 @@ uiModules.get('kibana')
template: paginateControlsTemplate
};
});


56 changes: 56 additions & 0 deletions src/ui/public/directives/paginated_selectable_list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import _ from 'lodash';
import uiModules from 'ui/modules';
import paginatedSelectableListTemplate from 'ui/partials/paginated_selectable_list.html';

const module = uiModules.get('kibana');

module.directive('paginatedSelectableList', function (kbnUrl) {

return {
restrict: 'E',
scope: {
perPage: '=',
list: '=',
userMakeUrl: '=',
userOnSelect: '='
},
template: paginatedSelectableListTemplate,
controller: function ($scope, $element, $filter) {
$scope.perPage = $scope.perPage || 10;

$scope.hits = $scope.list.sort() || [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would only do anything useful if $scope.list was an array of strings. If the user of the directive wants sorting they need to pass in a property or a sort function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, re-reading this, the entire directive appear that this assume you always pass in a list of strings. I think it would be more likely that we'd have a list of objects and we'd want to pass that in. I mentioned that here: #6156 (comment)


$scope.hitCount = $scope.hits.length;

/**
* Boolean that keeps track of whether hits are sorted ascending (true)
* or descending (false) by title
* @type {Boolean}
*/
$scope.isAscending = true;

/**
* Sorts saved object finder hits either ascending or descending
* @param {Array} hits Array of saved finder object hits
* @return {Array} Array sorted either ascending or descending
*/
$scope.sortHits = function (hits) {
$scope.isAscending = !$scope.isAscending;
$scope.hits = $scope.isAscending ? hits.sort() : hits.reverse();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above note on sorting

};

$scope.makeUrl = function (hit) {
return $scope.userMakeUrl(hit);
};

$scope.onSelect = function (hit, $event) {
return $scope.userOnSelect(hit, $event);
};

$scope.$watch('query', function (val) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get rid of this watcher, this can be done in the view, see notes starting here: https://github.com/elastic/kibana/pull/6550/files#r56722858

$scope.hits = $filter('filter')($scope.list, val);
$scope.hitCount = $scope.hits.length;
});
}
};
});
49 changes: 49 additions & 0 deletions src/ui/public/partials/paginated_selectable_list.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<form role="form" class="form-inline">
<div class="container-fluid">
<div class="row">
<div class="input-group form-group finder-form col-md-9">
<span class="input-group-addon">
<i class="fa fa-search"></i>
</span>
<input
input-focus
ng-model="query"
placeholder="Filter..."
class="form-control"
name="query"
type="text"
autocomplete="off" />
</div>
<div class="finder-hit-count col-md-3">
<span>{{hitCount}} of {{hitCount}}</span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always show X of X. It should show X of list.length

</div>
</div>
</div>
</form>
<paginate list="hits" per-page="{{perPage}}">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this list="hits | filter: query"

<ul class="li-striped list-group list-group-menu">
<li class="list-group-item list-group-menu-item">
<span class="paginate-heading">
Name
<i
class="fa"
ng-click="sortHits(hits)"
ng-class="isAscending ? 'fa-caret-up' : 'fa-caret-down'">
</i>
</span>
</li>
<li
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire <li> should be clickable here, not just the text. Take a look at how the saved-object-finder does it.

class="list-group-item list-group-menu-item"
ng-repeat="hit in page | filter: query">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this filter, it can go on the list= attribute of paginate

<a ng-if="userMakeUrl" kbn-href="{{makeUrl(hit)}}">
<div><span>{{hit}}</span></div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this span wrapped in a div but the one below isn't?

</a>
<div ng-if="userOnSelect" ng-click="onSelect(hit, $event)">
<span>{{hit}}</span>
</div>
</li>
<li class="list-group-item list-group-no-results" ng-if="hits.length === 0">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit-pick, use ng-show here instead of ng-if.

<p ng-bind="'No matches found.'"></p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ng-bind is not required as this is a static string.

</li>
</ul>
</paginate>
2 changes: 0 additions & 2 deletions src/ui/public/partials/saved_object_finder.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
</form>
<paginate list="finder.hits" per-page="20">
<ul class="li-striped list-group list-group-menu" ng-class="{'select-mode': finder.selector.enabled}">

<li class="list-group-item list-group-menu-item">
<span class="paginate-heading">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this removed?

Name
Expand All @@ -37,7 +36,6 @@
</i>
</span>
</li>

<li
class="list-group-item list-group-menu-item"
ng-class="{'active': finder.selector.index === $index && finder.selector.enabled}"
Expand Down
54 changes: 29 additions & 25 deletions src/ui/public/styles/base.less
Original file line number Diff line number Diff line change
Expand Up @@ -288,14 +288,36 @@ table {
}

//== SavedObjectFinder
saved-object-finder {
saved-object-finder,
paginated-selectable-list {
.row {
background-color: @kibanaGray6;
padding: 10px;
display: flex;
flex-direction: row;
}

.finder-hit-count,
.finder-manage-object {
min-width: 80px;
padding: 5px;
text-align: center;
}

.finder-hit-count {
flex: 1;

span {
color: @kibanaGray3;
}
}

.finder-manage-object {
flex: 3;
text-align: left;
text-transform: capitalize;
}

.form-group {
margin-bottom: 0;
float: left;
Expand All @@ -318,30 +340,6 @@ saved-object-finder {
width: 15px;
}
}

.finder-hit-count, .finder-manage-object {
min-width: 80px;
padding: 5px;
}

.finder-hit-count {
flex: 1;
text-align: center;

span {
color: @kibanaGray3;
}
}

.finder-manage-object {
flex: 3;
text-align: left;
text-transform: capitalize;
}
}

.list-group-item-menu:hover {
background-color: transparent;
}

ul.li-striped {
Expand Down Expand Up @@ -432,6 +430,12 @@ saved-object-finder {
}
}
}

paginate {
paginate-controls {
margin: 20px;
}
}
}

// when rendered within a config dropdown, don't use a bottom margin
Expand Down
1 change: 0 additions & 1 deletion src/ui/public/styles/dark-variables.less
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,3 @@
@sidebar-bg: @btn-default-bg;
@sidebar-hover-bg: darken(@btn-default-bg, 5%);
@sidebar-hover-color: @text-color;