You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2018/10/26 11:16:28 UTC

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

GitHub user ahgittin opened a pull request:

    https://github.com/apache/brooklyn-ui/pull/94

    Composer recent and filters

    store how recently an entity or palette icon was used in the composer, and introduce a filter and sort to use this info.  also move sorts and dropdown to new places as shown below.  fix sort (was not working), allow multi-sort (just remembers the last things you sorted from, does not show them), and add a footer to describe the results of sorts and filters
    
    builds on #93 so review that first (or review the two togethers
    
    <img width="528" alt="screen shot 2018-10-26 at 12 15 19" src="https://user-images.githubusercontent.com/496540/47563212-edfca480-d918-11e8-91c7-67818070dfb6.png">


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ahgittin/brooklyn-ui composer-recent-and-filters

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/brooklyn-ui/pull/94.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #94
    
----
commit 5ea0aae0c29fa37f8efd61363d5a04c343b7bf01
Author: Alex Heneveld <al...@...>
Date:   2018-10-24T09:25:29Z

    rename btn-ouline to btn-outline

commit 63031129ce1a972bda8c049f9518c6c17e232f85
Author: Alex Heneveld <al...@...>
Date:   2018-10-24T12:37:39Z

    dynamically compute pagination limit for palette based on height

commit 3db686cf3da05948d781c25ce2bb12ecac53232b
Author: Alex Heneveld <al...@...>
Date:   2018-10-24T13:45:34Z

    minor styling changes to make palette nicer
    
    esp for drag and to work with multi-width

commit 7670595878a6eb46701c45c67bce1327e443960b
Author: Alex Heneveld <al...@...>
Date:   2018-10-24T14:45:34Z

    allow palette to have view mode customised to show 6, 4, 3, or 1 item per row

commit 6ccc2b1af2c7c9953a113fb0e6346c60b8ae3fee
Author: Alex Heneveld <al...@...>
Date:   2018-10-24T15:08:57Z

    share palette view state among all palettes

commit 977e6a3dcecaa9f3df338c5b4a8b7cc5e6efb2e0
Author: Alex Heneveld <al...@...>
Date:   2018-10-24T15:43:23Z

    tidy dropdown and other composer styling

commit ca6d09c32e5ffedc2291aa7bfbb2b8b4af2cf8ef
Author: Alex Heneveld <al...@...>
Date:   2018-10-24T16:37:24Z

    make member spec selector act like main catalog palette, filtered for type

commit fb275c64f693fd9fcf2c1c83e26a65e96a3ee6aa
Author: Alex Heneveld <al...@...>
Date:   2018-10-25T09:28:09Z

    support 'mode' parameter for sections

commit 046c65601e1ed314db55ce96fc6de8d6bc9e44e6
Author: Alex Heneveld <al...@...>
Date:   2018-10-25T09:54:57Z

    css so add member spec use of palette can share format

commit b4a13961dd8bffc169363cf936ef694e256237af
Author: Alex Heneveld <al...@...>
Date:   2018-10-25T13:25:23Z

    palette allows filters to be specified, with placeholder for recent

commit 17e120c2f341703064f0121e5a77fe8cc6b85b29
Author: Alex Heneveld <al...@...>
Date:   2018-10-25T16:31:07Z

    improve sort by, including sort-by recency
    
    next need to enable recent filter (shows one page of recent items),
    set defaults,
    and add a tidied footer to disable categories when needed
    
    could also show last used in popup, and in catalog, and allow clearing in catalog

commit 44262c886c3125c261cd58217d66fea4e498d63b
Author: Alex Heneveld <al...@...>
Date:   2018-10-26T09:05:16Z

    "Recent" filter shows a page-full of most recent items
    
    last used shown in popup, and a tag can be used to preselect items for inclusion in the "recent" view

commit ca88021b9907effa9efc3119bae0d98e7322c376
Author: Alex Heneveld <al...@...>
Date:   2018-10-26T10:29:47Z

    add a footer to make it easier to navigate palette when filters don't match

----


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229354392
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -110,6 +110,38 @@ export function catalogSelectorSearchFilter() {
         }
     }
     
    +export function catalogSelectorFiltersFilter() {
    +    // compute counts and apply active filters;     
    +    // this is called by the view after filtering based on search,
    +    // so filters can adjust based on number of search results
    +    return function (items, $scope) {
    --- End diff --
    
    we set variables on the scope when the filter runs.  i take your point a nested object would be cleaner but let's do performance analysis before optimizing.  this doesn't seem to be that inefficient.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228977294
  
    --- Diff: ui-modules/blueprint-composer/package.json ---
    @@ -60,7 +60,8 @@
         "lodash": "^4.16.4",
         "ngclipboard": "^1.1.1",
         "normalizr": "^2.2.1",
    -    "underscore": "^1.8.3"
    +    "underscore": "^1.8.3",
    +    "moment": "^2.15.1"
    --- End diff --
    
    nice, will try this


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229351508
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -230,16 +248,85 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
             });
             $scope.items = items;
         });
    -    // this can be overridden for third-party filters.
    -    // it receives result of filtering based on search so filters can adjust based on number of search resullts
    -    $scope.filterPaletteItems = (items) => items;
    +    $scope.lastUsedText = (item) => {
    +        let l = (Number)(item.lastUsed);
    +        if (!l || isNaN(l) || l<=0) return "";
    +        if (l < 100000) return 'Preselected for inclusion in "Recent" filter.';
    +        return 'Last used: ' + moment(l).fromNow();
    +    }; 
    +    $scope.showPaletteControls = false;
    +    $scope.onFiltersShown = () => {
    +      $timeout( () => {
    +        // check do we need to show the multiline
    +        let filters = angular.element($element[0].querySelector(".filters"));
    +        $scope.$apply( () => $scope.filterSettings.filtersMultilineAvailable = filters[0].scrollHeight > filters[0].offsetHeight );
    +        
    +        repaginate($scope, $element);
    +      } );
    +    };
    +    $scope.togglePaletteControls = () => {
    +        $scope.showPaletteControls = !$scope.showPaletteControls;
    +        $timeout( () => repaginate($scope, $element) );
    --- End diff --
    
    Should use `$scope.$applyAsync` instead of `$timeout`


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228903437
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector-palette-footer.html ---
    @@ -0,0 +1,60 @@
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one
    +  or more contributor license agreements.  See the NOTICE file
    +  distributed with this work for additional information
    +  regarding copyright ownership.  The ASF licenses this file
    +  to you under the Apache License, Version 2.0 (the
    +  "License"); you may not use this file except in compliance
    +  with the License.  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing,
    +  software distributed under the License is distributed on an
    +  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +  KIND, either express or implied.  See the License for the
    +  specific language governing permissions and limitations
    +  under the License.
    +-->
    +<div class="paletteItemsFooter">
    +    <div ng-if="searchedItems.length === 0" class="no-match">
    +        <i class="fa fa-lemon-o"></i>
    +    </div>
    +    <small class="help-block text-sm palette-footer-message">
    +        <span ng-if="search">
    +          <span ng-if="searchedItems.length === 0">
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    +                    matching search but excluded by filters. 
    +                    <button class="btn btn-outline btn-info" ng-click="disableFilters()">Clear filters</button>
    +            </span>
    +            <span ng-if="skippingFilters || itemsBeforeActiveFilters.length === 0">
    +                    No matches for <code>{{ search }}</code>.
    +            </span>
    +          </span>
    +          <span ng-if="searchedItems.length > 0">
    +            <span ng-if="skippingFilters">
    +                    <b>No matches with selected filters.</b><br/>
    +                    Showing matches ignoring filters.
    +            </span>
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} more item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    +                    matching search but excluded by filters.
    +                    <button class="btn btn-outline btn-info" ng-click="disableFilters()">Clear filters</button>
    +            </span>
    +          </span>
    +        </span>
    +        <span ng-if="!search">
    +            <span ng-if="skippingFilters && searchedItems.length > 0">
    +                    <b>Nothing available in selected filters.</b><br/>
    --- End diff --
    
    Should use `<strong>` instead of `<b>` tag.
    ```suggestion
                        <strong>Nothing available in selected filters.</strong><br/>
    ```


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229309854
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    --- End diff --
    
    seems like the issue is that it can through; will handle that with `try ... catch`, good point.  won't convert to stringified map unless you know a good reason to do that.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228904564
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -30,15 +47,43 @@ export function catalogSelectorDirective() {
             scope: {
                 family: '<',
                 onSelect: '&',
    -            itemsPerPage: '<',
    +            rowsPerPage: '<',  // if unset then fill
                 reservedKeys: '<?',
    +            state: '<?',
                 mode: '@?',  // for use by downstream projects to pass in special modes
             },
             template: template,
    -        controller: ['$scope', '$element', '$q', '$uibModal', '$log', '$templateCache', 'paletteApi', 'paletteDragAndDropService', 'iconGenerator', 'composerOverrides', controller]
    +        controller: ['$scope', '$element', '$timeout', '$q', '$uibModal', '$log', '$templateCache', 'paletteApi', 'paletteDragAndDropService', 'iconGenerator', 'composerOverrides', 'recentlyUsedService', controller],
    +        link: link,
         };
     }
     
    +function link($scope, $element, attrs, controller) {
    +    let main = angular.element($element[0].querySelector(".catalog-palette-main"));
    +
    +    // repaginate when load completes (and items are shown), or it is resized
    +    $scope.$watchGroup(
    +        [ () => $scope.isLoading, () => main[0].offsetHeight, () => $scope.state.viewMode.itemsPerRow ],
    +        (values) => controller.$timeout( () => repaginate($scope, $element) ) );
    +    // also repaginate on window resize    
    +    angular.element(window).bind('resize', () => repaginate($scope, $element));
    +}
    +
    +function repaginate($scope, $element) {
    +    let rowsPerPage = $scope.rowsPerPage;
    +    if (!rowsPerPage) {
    +        let main = angular.element($element[0].querySelector(".catalog-palette-main"));
    +        if (!main || main[0].offsetHeight==0) {
    +            // console.log("no main or hidden or items per page fixed");
    --- End diff --
    
    Either removing this or use the `$log` service


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228905790
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -230,16 +248,85 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
             });
             $scope.items = items;
         });
    -    // this can be overridden for third-party filters.
    -    // it receives result of filtering based on search so filters can adjust based on number of search resullts
    -    $scope.filterPaletteItems = (items) => items;
    +    $scope.lastUsedText = (item) => {
    +        let l = (Number)(item.lastUsed);
    +        if (!l || isNaN(l) || l<=0) return "";
    +        if (l < 100000) return 'Preselected for inclusion in "Recent" filter.';
    +        return 'Last used: ' + moment(l).fromNow();
    +    }; 
    +    $scope.showPaletteControls = false;
    +    $scope.onFiltersShown = () => {
    +      $timeout( () => {
    --- End diff --
    
    You can use `$scope.$applyAsync` instead, which is cleaner than a timeout
    ```suggestion
          $scope.$applyAsync( () => {
    ```


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228904969
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -65,58 +110,21 @@ export function catalogSelectorSearchFilter() {
         }
     }
     
    -export function catalogSelectorSortFilter($filter) {
    -    return function (items, family) {
    -        return items.sort(function (left, right) {
    -            let nameLeft;
    -            let nameRight;
    -            if (family) {
    -                switch (family) {
    -                    case EntityFamily.ENTITY:
    -                    case EntityFamily.SPEC:
    -                    case EntityFamily.POLICY:
    -                    case EntityFamily.ENRICHER:
    -                    case EntityFamily.LOCATION:
    -                        nameLeft = $filter('entityName')(left);
    -                        nameRight = $filter('entityName')(right);
    -                        break;
    -                }
    -            }
    -
    -            if (!nameLeft || !nameRight) {
    -                return 0;
    -            }
    -            let nameCompare = nameLeft.localeCompare(nameRight);
    -            if (nameCompare !== 0) {
    -                return nameCompare;
    -            }
    -            let versionCompare = right.version.localeCompare(left.version);
    -            if (versionCompare !== 0) {
    -                return versionCompare
    -            }
    -            // TODO should symbolic name be the sorted field?
    -            let symNameCompare = left.symbolicName.localeCompare(right.symbolicName);
    -            if (symNameCompare !== 0) {
    -                return symNameCompare
    -            }            
    -            let containingBundleCompare = right.containingBundle.localeCompare(left.containingBundle);
    -            if (containingBundleCompare !== 0) {
    -                return containingBundleCompare
    -            }            
    -            return 0;
    -        });
    -    }
    -}
    +function controller($scope, $element, $timeout, $q, $uibModal, $log, $templateCache, paletteApi, paletteDragAndDropService, iconGenerator, composerOverrides, recentlyUsedService) {
    +    this.$timeout = $timeout;
     
    -function controller($scope, $element, $q, $uibModal, $log, $templateCache, paletteApi, paletteDragAndDropService, iconGenerator, composerOverrides) {
    +    $scope.viewModes = PALETTE_VIEW_MODES;
    +    $scope.viewOrders = PALETTE_VIEW_ORDERS;
    +    
    +    if (!$scope.state) $scope.state = {};
    +    if (!$scope.state.viewMode) $scope.state.viewMode = PALETTE_VIEW_MODES.normal;
    +    if (!$scope.state.currentOrder) $scope.state.currentOrder = [ PALETTE_VIEW_ORDERS.name.field, '-version' ];
    --- End diff --
    
    Could you use curly brackets for the `if` statements in order to be consistent with the codebase please? It also makes it easier to read 


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228908739
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    --- End diff --
    
    Storing each item in the `sessionStorage` can lead to crippled instances as there is a limited number of items you can store them. Instead, a good solution is to store the entire `Map` of items as a `JSON.stringify()` into one `sessionStorage.setItem()`


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228905605
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -230,16 +248,85 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
             });
             $scope.items = items;
         });
    -    // this can be overridden for third-party filters.
    -    // it receives result of filtering based on search so filters can adjust based on number of search resullts
    -    $scope.filterPaletteItems = (items) => items;
    +    $scope.lastUsedText = (item) => {
    +        let l = (Number)(item.lastUsed);
    +        if (!l || isNaN(l) || l<=0) return "";
    +        if (l < 100000) return 'Preselected for inclusion in "Recent" filter.';
    --- End diff --
    
    Could you use curly brackets for the `if` statements in order to be consistent with the codebase please? It also makes it easier to read 


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229322252
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    +        if (item.lastUsed) item.lastUsed = when;
    +    };
    +    service.getLastUsed = (item) => {
    +        let id = service.getId(item); 
    +        let s = sessionStorage.getItem(PREFIX+id);
    +        if (s) return ((Number)(s));
    +        let tag = item.tags && item.tags.find(t => t['ui-composer-recent-preselect']);
    +        if (tag) return tag['ui-composer-recent-preselect']; 
    +        return -1; 
    +    };
    +    
    +    service.embellish = (item) => {
    +        item.lastUsed = service.getLastUsed(item);
    --- End diff --
    
    `miscData` is not available here.  i think it's used on things in the blueprint but not things in the catalog?


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228907223
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.template.html ---
    @@ -30,44 +30,83 @@
             </div>
         </div>
     
    -    <div ng-show="!isLoading">
    +    <div ng-show="!isLoading" class="catalog-palette-main palette-full-height-wrapper">
    +      <div class="catalog-palette-header">
             <div class="form-group" ng-class="{'has-error': isReserved()}">
                 <div class="input-group input-group-sm">
    -                <span class="input-group-btn" uib-dropdown keyboard-nav>
    -                    <button type="button" class="btn btn-default dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false" uib-dropdown-toggle>
    -                        <i class="fa fa-sort"></i>
    +                <span class="input-group-btn" keyboard-nav>
    +                    <button id="palette-controls-button" type="button" class="btn btn-default dropdown-toggle" ng-class="{ 'btn-primary': showPaletteControls }" id="palette-controls" aria-haspopup="true" aria-expanded="false" 
    +                            ng-click="togglePaletteControls()">
    +                        <i class="fa fa-cog"></i>
                         </button>
    -                    {{filters | json}}
    -                    <ul class="dropdown-menu" role="menu" uib-dropdown-menu>
    -                        <li role="menuitem" class="dropdown-header">Order by</li>
    -                        <li role="menuitem" ng-repeat="order in state.orders track by $index" ng-class="{'active': state.currentOrder === order}" class="layer">
    -                            <a ng-click="state.currentOrder = order"><i class="fa fa-fw fa-circle"></i> {{order | capitalize}}</a>
    -                        </li>
    -                    </ul>
                     </span>
                     <input ng-model="search" type="text" placeholder="{{getPlaceHolder()}}" class="form-control" auto-focus />
                     <span class="input-group-addon">
                         <strong>{{searchedItems.length === 0 && search && allowFreeForm() ? 1 : searchedItems.length}}</strong>
                         {{(searchedItems.length === 0 && search && allowFreeForm() ? 1 : searchedItems.length) == 1 ? 'item' : 'items'}}
                     </span>
                 </div>
    +            <div class="pane-nav-row" id="palette-controls" ng-if="showPaletteControls" aria-labelledby="palette-controls-button">
    +
    +             <div class="filters" ng-class="{ 'multiline': filterSettings.showAllFilters }" ng-init="onFiltersShown()">
    +             
    +              <div class="spacer" ng-repeat-start="filter in filters" ng-if="filter.spacerBefore"></div>
    +              <div class="nav-row-item" ng-repeat-end ng-click="filter.enabled = !filter.enabled">
    +                <span title="{{ filter.hoverTest }}" class="label" ng-class="{'label-primary': filter.enabled, 'label-default': !filter.enabled }">
    +                    <i class="fa fa-{{ filter.icon }}" ng-if="filter.icon"></i>
    +                    <span ng-if="filter.label">{{ filter.label }}</span>
    +                </span>
    +              </div>
    +             </div>
    +
    +              <div class="nav-row-item" ng-if="filterSettings.filtersMultilineAvailable" title="More filters available"
    +                    ng-click="toggleShowAllFilters()" >
    +                <span class="label" ng-class="{'label-primary': filterSettings.showAllFilters, 'label-default': !filterSettings.showAllFilters }">
    +                    ...
    +                </span>
    +              </div>
    +              
    +              <div class="spacer"></div>
    +                
    +              <span uib-dropdown>
    +                <a href id="palette-sort" uib-dropdown-toggle aria-haspopup="true" aria-expanded="false" >
    +                  <div class="nav-row-item tool" title="Sort">
    +                    <i class="fa fa-sort"></i></div>
    +                </a>
    +                <ul class="dropdown-menu right-align-icon" role="menu" uib-dropdown-menu aria-labelledby="palette-sort">
    +                        <li role="menuitem" ng-repeat="order in viewOrders track by $index" ng-class="{'active': state.currentOrder[0] === order.field}" class="layer">
    +                            <a ng-click="sortBy(order)"><i class="fa fa-fw fa-circle"></i> {{ order.label }}</a>
    +                        </li>
    +                </ul>
    +              </span>
    +              
    +              <span uib-dropdown>
    +                <a href id="palette-view-mode" uib-dropdown-toggle aria-haspopup="true" aria-expanded="false" >
    +                  <div class="nav-row-item tool" title="Display mode">
    +                    <i class="fa fa-th"></i></div>
    +                </a>
    +                <ul class="dropdown-menu right-align-icon" role="menu" uib-dropdown-menu aria-labelledby="palette-view-mode">
    +                        <li role="menuitem" ng-repeat="view in viewModes track by $index" ng-class="{'active': state.viewMode === view}" class="layer">
    +                            <a ng-click="state.viewMode = view"><i class="fa fa-fw fa-circle"></i> {{view.name}}</a>
    +                        </li>
    +                </ul>
    +              </span>
    +            </div>
                 <ng-include src="customSubHeadTemplateName"/>
             </div>
    -        <small class="help-block text-sm no-match" ng-if="search && searchedItems.length === 0">
    -            No {{family.displayName.toLowerCase()}} matching the current search
    -        </small>
    +      </div>
     
    -        <div class="row grid catalog-palette">
    +        <div class="row grid">
                 <!-- here and below, col-xs-3 or -4 or -2 all work giving different densities;
                      this could be configurable ("compressed"=xs-2 w no labels, "normal"=xs-3, "big"=xs-4) -->
    -            <div class="col-xs-3 catalog-palette-item"
    -                    ng-repeat="item in searchedItems = (filterPaletteItems(items | catalogSelectorSearch:search) | catalogSelectorSort:family) | orderBy:state.currentOrder | limitTo:pagination.itemsPerPage:(pagination.page-1)*pagination.itemsPerPage track by (item.containingBundle + ':' + item.symbolicName + ':' + item.version)"
    +            <div class="catalog-palette-item" ng-class="state.viewMode.classes"
    +                    ng-repeat="item in searchedItems = filterPaletteItemsWithActiveFilters(items | catalogSelectorSearch:search) | orderBy:state.currentOrder | limitTo:pagination.itemsPerPage:(pagination.page-1)*pagination.itemsPerPage track by (item.containingBundle + ':' + item.symbolicName + ':' + item.version)"
    --- End diff --
    
    It's a very bad angular practice to call for a function that returns a new array on every call: the track cannot run properly as it will be new items every time. Considering the number of items in the list, this should use an angular filter instead.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229355523
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -110,6 +110,38 @@ export function catalogSelectorSearchFilter() {
         }
     }
     
    +export function catalogSelectorFiltersFilter() {
    +    // compute counts and apply active filters;     
    +    // this is called by the view after filtering based on search,
    +    // so filters can adjust based on number of search results
    +    return function (items, $scope) {
    --- End diff --
    
    https://stackoverflow.com/questions/17596246/access-scope-variables-from-a-filter-in-angularjs gives good caveats on this


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229351549
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -230,16 +248,85 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
             });
             $scope.items = items;
         });
    -    // this can be overridden for third-party filters.
    -    // it receives result of filtering based on search so filters can adjust based on number of search resullts
    -    $scope.filterPaletteItems = (items) => items;
    +    $scope.lastUsedText = (item) => {
    +        let l = (Number)(item.lastUsed);
    +        if (!l || isNaN(l) || l<=0) return "";
    +        if (l < 100000) return 'Preselected for inclusion in "Recent" filter.';
    +        return 'Last used: ' + moment(l).fromNow();
    +    }; 
    +    $scope.showPaletteControls = false;
    +    $scope.onFiltersShown = () => {
    +      $timeout( () => {
    +        // check do we need to show the multiline
    +        let filters = angular.element($element[0].querySelector(".filters"));
    +        $scope.$apply( () => $scope.filterSettings.filtersMultilineAvailable = filters[0].scrollHeight > filters[0].offsetHeight );
    +        
    +        repaginate($scope, $element);
    +      } );
    +    };
    +    $scope.togglePaletteControls = () => {
    +        $scope.showPaletteControls = !$scope.showPaletteControls;
    +        $timeout( () => repaginate($scope, $element) );
    +    }
    +    $scope.toggleShowAllFilters = () => {
    +        $scope.filterSettings.showAllFilters = !$scope.filterSettings.showAllFilters;
    +        $timeout( () => repaginate($scope, $element) );
    --- End diff --
    
    Should use `$scope.$applyAsync` instead of `$timeout`


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229322318
  
    --- Diff: ui-modules/blueprint-composer/package.json ---
    @@ -60,7 +60,8 @@
         "lodash": "^4.16.4",
         "ngclipboard": "^1.1.1",
         "normalizr": "^2.2.1",
    -    "underscore": "^1.8.3"
    +    "underscore": "^1.8.3",
    +    "moment": "^2.15.1"
    --- End diff --
    
    works a treat


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229267319
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    +        if (item.lastUsed) item.lastUsed = when;
    +    };
    +    service.getLastUsed = (item) => {
    +        let id = service.getId(item); 
    +        let s = sessionStorage.getItem(PREFIX+id);
    +        if (s) return ((Number)(s));
    +        let tag = item.tags && item.tags.find(t => t['ui-composer-recent-preselect']);
    +        if (tag) return tag['ui-composer-recent-preselect']; 
    +        return -1; 
    +    };
    +    
    +    service.embellish = (item) => {
    +        item.lastUsed = service.getLastUsed(item);
    --- End diff --
    
    Mainly for consistency and clear separation of concerns: top fields are the essential information of the model. Secondary one are all put into the `miscData` map


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228906038
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -230,16 +248,85 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
             });
             $scope.items = items;
         });
    -    // this can be overridden for third-party filters.
    -    // it receives result of filtering based on search so filters can adjust based on number of search resullts
    -    $scope.filterPaletteItems = (items) => items;
    +    $scope.lastUsedText = (item) => {
    +        let l = (Number)(item.lastUsed);
    +        if (!l || isNaN(l) || l<=0) return "";
    +        if (l < 100000) return 'Preselected for inclusion in "Recent" filter.';
    +        return 'Last used: ' + moment(l).fromNow();
    +    }; 
    +    $scope.showPaletteControls = false;
    +    $scope.onFiltersShown = () => {
    +      $timeout( () => {
    +        // check do we need to show the multiline
    +        let filters = angular.element($element[0].querySelector(".filters"));
    +        $scope.$apply( () => $scope.filterSettings.filtersMultilineAvailable = filters[0].scrollHeight > filters[0].offsetHeight );
    +        
    +        repaginate($scope, $element);
    +      } );
    +    };
    +    $scope.togglePaletteControls = () => {
    +        $scope.showPaletteControls = !$scope.showPaletteControls;
    +        $timeout( () => repaginate($scope, $element) );
    +    }
    +    $scope.toggleShowAllFilters = () => {
    +        $scope.filterSettings.showAllFilters = !$scope.filterSettings.showAllFilters;
    +        $timeout( () => repaginate($scope, $element) );
    +    };
    +    $scope.filterSettings = {};
    +
    +    $scope.filters = [
    +        { label: 'Recent', icon: 'clock-o', title: "Recently used and standard favorites", limitToOnePage: true,
    +            filterInit: items => {
    +                $scope.recentItems = items.filter( i => i.lastUsed && i.lastUsed > 0 );
    +                $scope.recentItems.sort( (a,b) => b.lastUsed - a.lastUsed );
    +                return $scope.recentItems; 
    +            }, enabled: false },
    +    ];
    +    $scope.disableFilters = (showFilters) => {
    +        $scope.filters.forEach( f => f.enabled = false );
    +        if (showFilters !== false) $scope.showPaletteControls = true;
    --- End diff --
    
    Could you use curly brackets for the `if` statements in order to be consistent with the codebase please? It also makes it easier to read 


---

[GitHub] brooklyn-ui issue #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-ui/pull/94
  
    addresses all PR comments


---

[GitHub] brooklyn-ui issue #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:

    https://github.com/apache/brooklyn-ui/pull/94
  
    Ah makes sense, you want the height therefore the template must be compiled. `$scope.$applyAsync` is executing at the beginning of a digest cycle therefore this info is not available.
    
    LGTM then, merging this. Thanks @ahgittin 



---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228903327
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector-palette-footer.html ---
    @@ -0,0 +1,60 @@
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one
    +  or more contributor license agreements.  See the NOTICE file
    +  distributed with this work for additional information
    +  regarding copyright ownership.  The ASF licenses this file
    +  to you under the Apache License, Version 2.0 (the
    +  "License"); you may not use this file except in compliance
    +  with the License.  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing,
    +  software distributed under the License is distributed on an
    +  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +  KIND, either express or implied.  See the License for the
    +  specific language governing permissions and limitations
    +  under the License.
    +-->
    +<div class="paletteItemsFooter">
    +    <div ng-if="searchedItems.length === 0" class="no-match">
    +        <i class="fa fa-lemon-o"></i>
    +    </div>
    +    <small class="help-block text-sm palette-footer-message">
    +        <span ng-if="search">
    +          <span ng-if="searchedItems.length === 0">
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    +                    matching search but excluded by filters. 
    +                    <button class="btn btn-outline btn-info" ng-click="disableFilters()">Clear filters</button>
    +            </span>
    +            <span ng-if="skippingFilters || itemsBeforeActiveFilters.length === 0">
    +                    No matches for <code>{{ search }}</code>.
    +            </span>
    +          </span>
    +          <span ng-if="searchedItems.length > 0">
    +            <span ng-if="skippingFilters">
    +                    <b>No matches with selected filters.</b><br/>
    +                    Showing matches ignoring filters.
    +            </span>
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} more item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    --- End diff --
    
    Same as 2 comments above


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229351261
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -127,15 +135,15 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
         $scope.getPlaceHolder = function () {
             return 'Search';
         };
    -
    +    
         $scope.isLoading = true;
     
         $scope.$watch('search', () => {
             $scope.freeFormTile = {
                 symbolicName: $scope.search,
                 name: $scope.search,
                 displayName: $scope.search,
    -            supertypes: [$scope.family.superType]
    +            supertypes: ($scope.family ? [ $scope.family.superType ] : []),
    --- End diff --
    
    This directive wasn't designed to support `null` family. Something is wrong is that's the case, I don't see any reason why we would use it without a family


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228976153
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    --- End diff --
    
    hmm, i thought stringifying a potentially very large map on every change would be less performant than using session storage.  do you have more info on the limits?  (the limits i've seen are on _size_ not number of entries.)


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229306022
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector-palette-footer.html ---
    @@ -0,0 +1,60 @@
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one
    +  or more contributor license agreements.  See the NOTICE file
    +  distributed with this work for additional information
    +  regarding copyright ownership.  The ASF licenses this file
    +  to you under the Apache License, Version 2.0 (the
    +  "License"); you may not use this file except in compliance
    +  with the License.  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing,
    +  software distributed under the License is distributed on an
    +  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +  KIND, either express or implied.  See the License for the
    +  specific language governing permissions and limitations
    +  under the License.
    +-->
    +<div class="paletteItemsFooter">
    +    <div ng-if="searchedItems.length === 0" class="no-match">
    +        <i class="fa fa-lemon-o"></i>
    +    </div>
    +    <small class="help-block text-sm palette-footer-message">
    +        <span ng-if="search">
    +          <span ng-if="searchedItems.length === 0">
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    --- End diff --
    
    ack


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229348593
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -30,15 +47,43 @@ export function catalogSelectorDirective() {
             scope: {
                 family: '<',
                 onSelect: '&',
    -            itemsPerPage: '<',
    +            rowsPerPage: '<',  // if unset then fill
                 reservedKeys: '<?',
    +            state: '<?',
                 mode: '@?',  // for use by downstream projects to pass in special modes
             },
             template: template,
    -        controller: ['$scope', '$element', '$q', '$uibModal', '$log', '$templateCache', 'paletteApi', 'paletteDragAndDropService', 'iconGenerator', 'composerOverrides', controller]
    +        controller: ['$scope', '$element', '$timeout', '$q', '$uibModal', '$log', '$templateCache', 'paletteApi', 'paletteDragAndDropService', 'iconGenerator', 'composerOverrides', 'recentlyUsedService', controller],
    +        link: link,
         };
     }
     
    +function link($scope, $element, attrs, controller) {
    +    let main = angular.element($element[0].querySelector(".catalog-palette-main"));
    +
    +    // repaginate when load completes (and items are shown), or it is resized
    +    $scope.$watchGroup(
    +        [ () => $scope.isLoading, () => main[0].offsetHeight, () => $scope.state.viewMode.itemsPerRow ],
    +        (values) => controller.$timeout( () => repaginate($scope, $element) ) );
    --- End diff --
    
    Should use `$scope.$applyAsync` instead of `$timeout`


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229350690
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -30,15 +47,43 @@ export function catalogSelectorDirective() {
             scope: {
                 family: '<',
                 onSelect: '&',
    -            itemsPerPage: '<',
    +            rowsPerPage: '<',  // if unset then fill
    --- End diff --
    
    This parameter is optional, it should have a `?` at the end, like the ones below.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228905867
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -230,16 +248,85 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
             });
             $scope.items = items;
         });
    -    // this can be overridden for third-party filters.
    -    // it receives result of filtering based on search so filters can adjust based on number of search resullts
    -    $scope.filterPaletteItems = (items) => items;
    +    $scope.lastUsedText = (item) => {
    +        let l = (Number)(item.lastUsed);
    +        if (!l || isNaN(l) || l<=0) return "";
    +        if (l < 100000) return 'Preselected for inclusion in "Recent" filter.';
    +        return 'Last used: ' + moment(l).fromNow();
    +    }; 
    +    $scope.showPaletteControls = false;
    +    $scope.onFiltersShown = () => {
    +      $timeout( () => {
    +        // check do we need to show the multiline
    +        let filters = angular.element($element[0].querySelector(".filters"));
    +        $scope.$apply( () => $scope.filterSettings.filtersMultilineAvailable = filters[0].scrollHeight > filters[0].offsetHeight );
    +        
    +        repaginate($scope, $element);
    +      } );
    +    };
    +    $scope.togglePaletteControls = () => {
    +        $scope.showPaletteControls = !$scope.showPaletteControls;
    +        $timeout( () => repaginate($scope, $element) );
    --- End diff --
    
    Same as above


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228903493
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector-palette-footer.html ---
    @@ -0,0 +1,60 @@
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one
    +  or more contributor license agreements.  See the NOTICE file
    +  distributed with this work for additional information
    +  regarding copyright ownership.  The ASF licenses this file
    +  to you under the Apache License, Version 2.0 (the
    +  "License"); you may not use this file except in compliance
    +  with the License.  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing,
    +  software distributed under the License is distributed on an
    +  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +  KIND, either express or implied.  See the License for the
    +  specific language governing permissions and limitations
    +  under the License.
    +-->
    +<div class="paletteItemsFooter">
    +    <div ng-if="searchedItems.length === 0" class="no-match">
    +        <i class="fa fa-lemon-o"></i>
    +    </div>
    +    <small class="help-block text-sm palette-footer-message">
    +        <span ng-if="search">
    +          <span ng-if="searchedItems.length === 0">
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    +                    matching search but excluded by filters. 
    +                    <button class="btn btn-outline btn-info" ng-click="disableFilters()">Clear filters</button>
    +            </span>
    +            <span ng-if="skippingFilters || itemsBeforeActiveFilters.length === 0">
    +                    No matches for <code>{{ search }}</code>.
    +            </span>
    +          </span>
    +          <span ng-if="searchedItems.length > 0">
    +            <span ng-if="skippingFilters">
    +                    <b>No matches with selected filters.</b><br/>
    +                    Showing matches ignoring filters.
    +            </span>
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} more item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    +                    matching search but excluded by filters.
    +                    <button class="btn btn-outline btn-info" ng-click="disableFilters()">Clear filters</button>
    +            </span>
    +          </span>
    +        </span>
    +        <span ng-if="!search">
    +            <span ng-if="skippingFilters && searchedItems.length > 0">
    +                    <b>Nothing available in selected filters.</b><br/>
    +                    Ignoring filters.
    +            </span>
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} more item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    --- End diff --
    
    Same as 2 comments above


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229358124
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -127,15 +135,15 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
         $scope.getPlaceHolder = function () {
             return 'Search';
         };
    -
    +    
         $scope.isLoading = true;
     
         $scope.$watch('search', () => {
             $scope.freeFormTile = {
                 symbolicName: $scope.search,
                 name: $scope.search,
                 displayName: $scope.search,
    -            supertypes: [$scope.family.superType]
    +            supertypes: ($scope.family ? [ $scope.family.superType ] : []),
    --- End diff --
    
    pretty sure i added it because of an NPE here.  not sure how/why but this check won't hurt.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229348023
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -110,6 +110,38 @@ export function catalogSelectorSearchFilter() {
         }
     }
     
    +export function catalogSelectorFiltersFilter() {
    +    // compute counts and apply active filters;     
    +    // this is called by the view after filtering based on search,
    +    // so filters can adjust based on number of search results
    +    return function (items, $scope) {
    --- End diff --
    
    Passing scope around is a bad idea for performances, you should pass each individual parameter, or an object containing all them instead.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229345898
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    +        if (item.lastUsed) item.lastUsed = when;
    +    };
    +    service.getLastUsed = (item) => {
    +        let id = service.getId(item); 
    +        let s = sessionStorage.getItem(PREFIX+id);
    +        if (s) return ((Number)(s));
    +        let tag = item.tags && item.tags.find(t => t['ui-composer-recent-preselect']);
    +        if (tag) return tag['ui-composer-recent-preselect']; 
    +        return -1; 
    +    };
    +    
    +    service.embellish = (item) => {
    +        item.lastUsed = service.getLastUsed(item);
    --- End diff --
    
    Aaah `item` refers here as items from the API, not the model. 
    Right so it makes sense then.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/brooklyn-ui/pull/94


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228904299
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -30,15 +47,43 @@ export function catalogSelectorDirective() {
             scope: {
                 family: '<',
                 onSelect: '&',
    -            itemsPerPage: '<',
    +            rowsPerPage: '<',  // if unset then fill
                 reservedKeys: '<?',
    +            state: '<?',
                 mode: '@?',  // for use by downstream projects to pass in special modes
             },
             template: template,
    -        controller: ['$scope', '$element', '$q', '$uibModal', '$log', '$templateCache', 'paletteApi', 'paletteDragAndDropService', 'iconGenerator', 'composerOverrides', controller]
    +        controller: ['$scope', '$element', '$timeout', '$q', '$uibModal', '$log', '$templateCache', 'paletteApi', 'paletteDragAndDropService', 'iconGenerator', 'composerOverrides', 'recentlyUsedService', controller],
    +        link: link,
         };
     }
     
    +function link($scope, $element, attrs, controller) {
    +    let main = angular.element($element[0].querySelector(".catalog-palette-main"));
    +
    +    // repaginate when load completes (and items are shown), or it is resized
    +    $scope.$watchGroup(
    +        [ () => $scope.isLoading, () => main[0].offsetHeight, () => $scope.state.viewMode.itemsPerRow ],
    +        (values) => controller.$timeout( () => repaginate($scope, $element) ) );
    --- End diff --
    
    Instead of using `$timeout`, you should use `$scope.$applyAsync`
    
    ```suggestion
            (values) => $scope.$applyAsync( () => repaginate($scope, $element) ) );
    ```


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228976580
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    +        if (item.lastUsed) item.lastUsed = when;
    +    };
    +    service.getLastUsed = (item) => {
    +        let id = service.getId(item); 
    +        let s = sessionStorage.getItem(PREFIX+id);
    +        if (s) return ((Number)(s));
    +        let tag = item.tags && item.tags.find(t => t['ui-composer-recent-preselect']);
    +        if (tag) return tag['ui-composer-recent-preselect']; 
    +        return -1; 
    +    };
    +    
    +    service.embellish = (item) => {
    +        item.lastUsed = service.getLastUsed(item);
    --- End diff --
    
    will do, though curious, why?


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228903286
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector-palette-footer.html ---
    @@ -0,0 +1,60 @@
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one
    +  or more contributor license agreements.  See the NOTICE file
    +  distributed with this work for additional information
    +  regarding copyright ownership.  The ASF licenses this file
    +  to you under the Apache License, Version 2.0 (the
    +  "License"); you may not use this file except in compliance
    +  with the License.  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing,
    +  software distributed under the License is distributed on an
    +  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +  KIND, either express or implied.  See the License for the
    +  specific language governing permissions and limitations
    +  under the License.
    +-->
    +<div class="paletteItemsFooter">
    +    <div ng-if="searchedItems.length === 0" class="no-match">
    +        <i class="fa fa-lemon-o"></i>
    +    </div>
    +    <small class="help-block text-sm palette-footer-message">
    +        <span ng-if="search">
    +          <span ng-if="searchedItems.length === 0">
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    +                    matching search but excluded by filters. 
    +                    <button class="btn btn-outline btn-info" ng-click="disableFilters()">Clear filters</button>
    +            </span>
    +            <span ng-if="skippingFilters || itemsBeforeActiveFilters.length === 0">
    +                    No matches for <code>{{ search }}</code>.
    +            </span>
    +          </span>
    +          <span ng-if="searchedItems.length > 0">
    +            <span ng-if="skippingFilters">
    +                    <b>No matches with selected filters.</b><br/>
    --- End diff --
    
    Should use `<strong>` instead of `<b>` tag.
    ```suggestion
                        <strong>No matches with selected filters.</strong><br/>
    ```


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228903041
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector-palette-footer.html ---
    @@ -0,0 +1,60 @@
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one
    +  or more contributor license agreements.  See the NOTICE file
    +  distributed with this work for additional information
    +  regarding copyright ownership.  The ASF licenses this file
    +  to you under the Apache License, Version 2.0 (the
    +  "License"); you may not use this file except in compliance
    +  with the License.  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing,
    +  software distributed under the License is distributed on an
    +  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +  KIND, either express or implied.  See the License for the
    +  specific language governing permissions and limitations
    +  under the License.
    +-->
    +<div class="paletteItemsFooter">
    +    <div ng-if="searchedItems.length === 0" class="no-match">
    +        <i class="fa fa-lemon-o"></i>
    +    </div>
    +    <small class="help-block text-sm palette-footer-message">
    +        <span ng-if="search">
    +          <span ng-if="searchedItems.length === 0">
    +            <span ng-if="!skippingFilters && itemsBeforeActiveFilters.length > itemsAfterActiveFilters.length">
    +                    <b>{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length }} item{{ itemsBeforeActiveFilters.length - itemsAfterActiveFilters.length > 1 ? 's' : ''}}</b> 
    --- End diff --
    
    Should use `<strong>` instead of `<b>` tag. Also, should use `ng-pluralize` (https://docs.angularjs.org/api/ng/directive/ngPluralize) to handle plurals


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228974845
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.template.html ---
    @@ -30,44 +30,83 @@
             </div>
         </div>
     
    -    <div ng-show="!isLoading">
    +    <div ng-show="!isLoading" class="catalog-palette-main palette-full-height-wrapper">
    +      <div class="catalog-palette-header">
             <div class="form-group" ng-class="{'has-error': isReserved()}">
                 <div class="input-group input-group-sm">
    -                <span class="input-group-btn" uib-dropdown keyboard-nav>
    -                    <button type="button" class="btn btn-default dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false" uib-dropdown-toggle>
    -                        <i class="fa fa-sort"></i>
    +                <span class="input-group-btn" keyboard-nav>
    +                    <button id="palette-controls-button" type="button" class="btn btn-default dropdown-toggle" ng-class="{ 'btn-primary': showPaletteControls }" id="palette-controls" aria-haspopup="true" aria-expanded="false" 
    +                            ng-click="togglePaletteControls()">
    +                        <i class="fa fa-cog"></i>
                         </button>
    -                    {{filters | json}}
    -                    <ul class="dropdown-menu" role="menu" uib-dropdown-menu>
    -                        <li role="menuitem" class="dropdown-header">Order by</li>
    -                        <li role="menuitem" ng-repeat="order in state.orders track by $index" ng-class="{'active': state.currentOrder === order}" class="layer">
    -                            <a ng-click="state.currentOrder = order"><i class="fa fa-fw fa-circle"></i> {{order | capitalize}}</a>
    -                        </li>
    -                    </ul>
                     </span>
                     <input ng-model="search" type="text" placeholder="{{getPlaceHolder()}}" class="form-control" auto-focus />
                     <span class="input-group-addon">
                         <strong>{{searchedItems.length === 0 && search && allowFreeForm() ? 1 : searchedItems.length}}</strong>
                         {{(searchedItems.length === 0 && search && allowFreeForm() ? 1 : searchedItems.length) == 1 ? 'item' : 'items'}}
                     </span>
                 </div>
    +            <div class="pane-nav-row" id="palette-controls" ng-if="showPaletteControls" aria-labelledby="palette-controls-button">
    +
    +             <div class="filters" ng-class="{ 'multiline': filterSettings.showAllFilters }" ng-init="onFiltersShown()">
    +             
    +              <div class="spacer" ng-repeat-start="filter in filters" ng-if="filter.spacerBefore"></div>
    +              <div class="nav-row-item" ng-repeat-end ng-click="filter.enabled = !filter.enabled">
    +                <span title="{{ filter.hoverTest }}" class="label" ng-class="{'label-primary': filter.enabled, 'label-default': !filter.enabled }">
    +                    <i class="fa fa-{{ filter.icon }}" ng-if="filter.icon"></i>
    +                    <span ng-if="filter.label">{{ filter.label }}</span>
    +                </span>
    +              </div>
    +             </div>
    +
    +              <div class="nav-row-item" ng-if="filterSettings.filtersMultilineAvailable" title="More filters available"
    +                    ng-click="toggleShowAllFilters()" >
    +                <span class="label" ng-class="{'label-primary': filterSettings.showAllFilters, 'label-default': !filterSettings.showAllFilters }">
    +                    ...
    +                </span>
    +              </div>
    +              
    +              <div class="spacer"></div>
    +                
    +              <span uib-dropdown>
    +                <a href id="palette-sort" uib-dropdown-toggle aria-haspopup="true" aria-expanded="false" >
    +                  <div class="nav-row-item tool" title="Sort">
    +                    <i class="fa fa-sort"></i></div>
    +                </a>
    +                <ul class="dropdown-menu right-align-icon" role="menu" uib-dropdown-menu aria-labelledby="palette-sort">
    +                        <li role="menuitem" ng-repeat="order in viewOrders track by $index" ng-class="{'active': state.currentOrder[0] === order.field}" class="layer">
    +                            <a ng-click="sortBy(order)"><i class="fa fa-fw fa-circle"></i> {{ order.label }}</a>
    +                        </li>
    +                </ul>
    +              </span>
    +              
    +              <span uib-dropdown>
    +                <a href id="palette-view-mode" uib-dropdown-toggle aria-haspopup="true" aria-expanded="false" >
    +                  <div class="nav-row-item tool" title="Display mode">
    +                    <i class="fa fa-th"></i></div>
    +                </a>
    +                <ul class="dropdown-menu right-align-icon" role="menu" uib-dropdown-menu aria-labelledby="palette-view-mode">
    +                        <li role="menuitem" ng-repeat="view in viewModes track by $index" ng-class="{'active': state.viewMode === view}" class="layer">
    +                            <a ng-click="state.viewMode = view"><i class="fa fa-fw fa-circle"></i> {{view.name}}</a>
    +                        </li>
    +                </ul>
    +              </span>
    +            </div>
                 <ng-include src="customSubHeadTemplateName"/>
             </div>
    -        <small class="help-block text-sm no-match" ng-if="search && searchedItems.length === 0">
    -            No {{family.displayName.toLowerCase()}} matching the current search
    -        </small>
    +      </div>
     
    -        <div class="row grid catalog-palette">
    +        <div class="row grid">
                 <!-- here and below, col-xs-3 or -4 or -2 all work giving different densities;
                      this could be configurable ("compressed"=xs-2 w no labels, "normal"=xs-3, "big"=xs-4) -->
    -            <div class="col-xs-3 catalog-palette-item"
    -                    ng-repeat="item in searchedItems = (filterPaletteItems(items | catalogSelectorSearch:search) | catalogSelectorSort:family) | orderBy:state.currentOrder | limitTo:pagination.itemsPerPage:(pagination.page-1)*pagination.itemsPerPage track by (item.containingBundle + ':' + item.symbolicName + ':' + item.version)"
    +            <div class="catalog-palette-item" ng-class="state.viewMode.classes"
    +                    ng-repeat="item in searchedItems = filterPaletteItemsWithActiveFilters(items | catalogSelectorSearch:search) | orderBy:state.currentOrder | limitTo:pagination.itemsPerPage:(pagination.page-1)*pagination.itemsPerPage track by (item.containingBundle + ':' + item.symbolicName + ':' + item.version)"
    --- End diff --
    
    noted - will work on this


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228903921
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -30,15 +47,43 @@ export function catalogSelectorDirective() {
             scope: {
                 family: '<',
                 onSelect: '&',
    -            itemsPerPage: '<',
    +            rowsPerPage: '<',  // if unset then fill
    --- End diff --
    
    If this parameter is option, it should have a `?` at the end, like the ones below


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229348741
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -230,16 +248,85 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
             });
             $scope.items = items;
         });
    -    // this can be overridden for third-party filters.
    -    // it receives result of filtering based on search so filters can adjust based on number of search resullts
    -    $scope.filterPaletteItems = (items) => items;
    +    $scope.lastUsedText = (item) => {
    +        let l = (Number)(item.lastUsed);
    +        if (!l || isNaN(l) || l<=0) return "";
    +        if (l < 100000) return 'Preselected for inclusion in "Recent" filter.';
    +        return 'Last used: ' + moment(l).fromNow();
    +    }; 
    +    $scope.showPaletteControls = false;
    +    $scope.onFiltersShown = () => {
    +      $timeout( () => {
    --- End diff --
    
    Should use `$scope.$applyAsync` instead of `$timeout`


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r229266836
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    --- End diff --
    
    We did hit the maximum size on the earlier day of the composer while saving the generated icons. Now, I think it's the actual size of the `localStorage` (5MB if I recall correctly) but don't remember if there is a limit per item or in total.
    
    Nonetheless, we need to handle this because `sessionStorage.setItem()` will throw if there is no more space left.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228905287
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -127,15 +135,15 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
         $scope.getPlaceHolder = function () {
             return 'Search';
         };
    -
    +    
         $scope.isLoading = true;
     
         $scope.$watch('search', () => {
             $scope.freeFormTile = {
                 symbolicName: $scope.search,
                 name: $scope.search,
                 displayName: $scope.search,
    -            supertypes: [$scope.family.superType]
    +            supertypes: ($scope.family ? [ $scope.family.superType ] : []),
    --- End diff --
    
    `scope.family` is marked as required in the directive specification, you shouldn't need that


---

[GitHub] brooklyn-ui issue #94: Composer recent and filters

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-ui/pull/94
  
    PS i tried with `$scope.$applyAsync` but it ran before the html was reprocessed -- it had `offsetHeight==0` whereas with `$timeout` it is correctly set as `24`.


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228905899
  
    --- Diff: ui-modules/blueprint-composer/app/components/catalog-selector/catalog-selector.directive.js ---
    @@ -230,16 +248,85 @@ function controller($scope, $element, $q, $uibModal, $log, $templateCache, palet
             });
             $scope.items = items;
         });
    -    // this can be overridden for third-party filters.
    -    // it receives result of filtering based on search so filters can adjust based on number of search resullts
    -    $scope.filterPaletteItems = (items) => items;
    +    $scope.lastUsedText = (item) => {
    +        let l = (Number)(item.lastUsed);
    +        if (!l || isNaN(l) || l<=0) return "";
    +        if (l < 100000) return 'Preselected for inclusion in "Recent" filter.';
    +        return 'Last used: ' + moment(l).fromNow();
    +    }; 
    +    $scope.showPaletteControls = false;
    +    $scope.onFiltersShown = () => {
    +      $timeout( () => {
    +        // check do we need to show the multiline
    +        let filters = angular.element($element[0].querySelector(".filters"));
    +        $scope.$apply( () => $scope.filterSettings.filtersMultilineAvailable = filters[0].scrollHeight > filters[0].offsetHeight );
    +        
    +        repaginate($scope, $element);
    +      } );
    +    };
    +    $scope.togglePaletteControls = () => {
    +        $scope.showPaletteControls = !$scope.showPaletteControls;
    +        $timeout( () => repaginate($scope, $element) );
    +    }
    +    $scope.toggleShowAllFilters = () => {
    +        $scope.filterSettings.showAllFilters = !$scope.filterSettings.showAllFilters;
    +        $timeout( () => repaginate($scope, $element) );
    --- End diff --
    
    Same as above


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228908909
  
    --- Diff: ui-modules/blueprint-composer/app/components/providers/recently-used-service.provider.js ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +export function recentlyUsedServiceProvider() {
    +    return {
    +        $get: ['$log', function ($log) { return new RecentlyUsedService($log); }]
    +    }
    +}
    +
    +export const PREFIX = 'org.apache.brooklyn.composer.lastUsed:';
    +
    +function RecentlyUsedService($log) {
    +    let service = {};
    +    
    +    service.getId = (brooklynObject) => {
    +        if (typeof brooklynObject === 'string') return brooklynObject;
    +        return (brooklynObject.containingBundle || '?') + "::" + 
    +            (brooklynObject.symbolicName || '?') + ":" + (brooklynObject.version || '?');
    +    };
    +    
    +    service.markUsed = (item, when) => {
    +        let id = service.getId(item);
    +        if (when) {
    +            let old = service.getLastUsed(id);
    +            if (old > when) return;
    +        } else {
    +            when = Date.now();
    +        }
    +        sessionStorage.setItem(PREFIX+id, when);
    +        if (item.lastUsed) item.lastUsed = when;
    +    };
    +    service.getLastUsed = (item) => {
    +        let id = service.getId(item); 
    +        let s = sessionStorage.getItem(PREFIX+id);
    +        if (s) return ((Number)(s));
    +        let tag = item.tags && item.tags.find(t => t['ui-composer-recent-preselect']);
    +        if (tag) return tag['ui-composer-recent-preselect']; 
    +        return -1; 
    +    };
    +    
    +    service.embellish = (item) => {
    +        item.lastUsed = service.getLastUsed(item);
    --- End diff --
    
    This should be added into the modal `miscData`
    ```suggestion
            item.miscData.set('lastUsed, service.getLastUsed(item));
    ```


---

[GitHub] brooklyn-ui pull request #94: Composer recent and filters

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-ui/pull/94#discussion_r228909725
  
    --- Diff: ui-modules/blueprint-composer/package.json ---
    @@ -60,7 +60,8 @@
         "lodash": "^4.16.4",
         "ngclipboard": "^1.1.1",
         "normalizr": "^2.2.1",
    -    "underscore": "^1.8.3"
    +    "underscore": "^1.8.3",
    +    "moment": "^2.15.1"
    --- End diff --
    
    This will increase the bundle size by about 350k. There are smaller alternatives available if you just want to display the time from now like https://date-fns.org/


---