You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/07/05 04:01:52 UTC

[GitHub] [guacamole-client] mike-jumper opened a new pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

mike-jumper opened a new pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623


   This change adds a new grid layout of multiple connections that allows those connections to be visible within the same tab. The user can also select multiple visible connections to interact with each using the keyboard simultaneously.
   
   The main entrypoint into these changes is the connection dropdown in the Guacamole menu, which now contains checkboxes:
   
   ![Screenshot 2021-07-04 8 50 11 PM](https://user-images.githubusercontent.com/4632905/124415606-1106c580-dd0a-11eb-8f75-13b872c70327.png)
   
   If multiple boxes are checked, those connections will automatically be opened and tiled evenly:
   
   ![Screenshot 2021-07-04 8 50 40 PM](https://user-images.githubusercontent.com/4632905/124415684-3693cf00-dd0a-11eb-950d-82e2f643983a.png)
   
   Keyboard focus is assigned by clicking on a connection. By holding Shift or Ctrl while clicking, multiple connections can be focused, and each will then receive keyboard input simultaneously:
   
   ![Screenshot 2021-07-04 8 51 14 PM](https://user-images.githubusercontent.com/4632905/124415796-6e027b80-dd0a-11eb-937a-870389be474f.png)
   
   Focus also dictates the contents of the Guacamole menu, which will generally only show options specific to the currently-focused connection. If multiple connections are focused, only options that are applicable to all connections will be visible.
   
   Clicking the "X" in the corner of any connection closes the connection, with others rearranging to fill the new space:
   
   ![Screenshot 2021-07-04 8 51 37 PM](https://user-images.githubusercontent.com/4632905/124415878-9c805680-dd0a-11eb-8b26-499c92676ecf.png)
   
   Part of these changes involved moving the client panel to a higher, common level, allowing connections to be monitored and switched while viewing other parts other Guacamole webapp:
   
   ![Screenshot 2021-07-04 8 51 54 PM](https://user-images.githubusercontent.com/4632905/124415940-c5a0e700-dd0a-11eb-8026-6e71b0cc95be.png)
   
   As I also needed to add some new variations of existing graphics, I went ahead and migrated our old PNGs to SVG so that we can benefit from scaling and so that they can be an actual part of the source. All browsers currently supported by Guacamole support SVG, even IE10.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r666068139



##########
File path: guacamole/src/main/frontend/src/app/client/directives/guacClientNotification.js
##########
@@ -0,0 +1,431 @@
+/*
+ * 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.
+ */
+
+/**
+ * A directive for displaying a non-global notification describing the status
+ * of a specific Guacamole client, including prompts for any information
+ * necessary to continue the connection.
+ */
+angular.module('client').directive('guacClientNotification', [function guacClientNotification() {
+
+    var directive = {
+        restrict: 'E',
+        replace: true,
+        templateUrl: 'app/client/templates/guacClientNotification.html'
+    };
+
+    directive.scope = {
+
+        /**
+         * The client whose status should be displayed.
+         * 
+         * @type ManagedClient
+         */
+        client : '='
+        
+    };
+
+    directive.controller = ['$scope', '$injector', '$element',
+        function guacClientNotificationController($scope, $injector, $element) {
+   
+        // Required types
+        var ManagedClient      = $injector.get('ManagedClient');
+        var ManagedClientState = $injector.get('ManagedClientState');
+        var Protocol           = $injector.get('Protocol');
+
+        // Required services
+        var $location              = $injector.get('$location');
+        var authenticationService  = $injector.get('authenticationService');
+        var guacClientManager      = $injector.get('guacClientManager');
+        var requestService         = $injector.get('requestService');
+        var userPageService        = $injector.get('userPageService');
+
+        /**
+         * A Notification object describing the client status to display as a
+         * dialog or prompt, as would be accepted by guacNotification.showStatus(),
+         * or false if no status should be shown.
+         *
+         * @type {Notification|Object|Boolean}
+         */
+        $scope.status = false;
+
+        /**
+         * All client error codes handled and passed off for translation. Any error
+         * code not present in this list will be represented by the "DEFAULT"
+         * translation.
+         */
+        var CLIENT_ERRORS = {

Review comment:
       OK - migrated all directly touched code from `var` to `const`/`let` via rebase. I avoided making sweeping changes, updating only the code that is already within this diff.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r666072494



##########
File path: guacamole/src/main/frontend/src/app/client/controllers/clientController.js
##########
@@ -370,134 +384,58 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
     var substituteKeysPressed = {};
 
     /**
-     * Map of all currently pressed keys (by keysym) to the clipboard contents
-     * received from the remote desktop while those keys were pressed. All keys
-     * not currently pressed will not have entries within this map.
+     * Returns whether the shortcut for showing/hiding the Guacamole menu
+     * (Ctrl+Alt+Shift) has been pressed.
      *
-     * @type Object.<Number, ClipboardData>
-     */
-    var clipboardDataFromKey = {};
-
-    /*
-     * Check to see if all currently pressed keys are in the set of menu keys.
+     * @param {Guacamole.Keyboard} keyboard
+     *     The Guacamole.Keyboard object tracking the local keyboard state.
+     *
+     * @returns {boolean}
+     *     true of Ctrl+Alt+Shift has been pressed, false otherwise.

Review comment:
       Corrected via rebase.

##########
File path: guacamole/src/main/frontend/src/app/client/types/ManagedClientGroup.js
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.
+ */
+
+/**
+ * Provides the ManagedClientGroup class used by the guacClientManager service.
+ */
+angular.module('client').factory('ManagedClientGroup', ['$injector', function defineManagedClientGroup($injector) {
+
+    /**
+     * Object which serves as a grouping of ManagedClients. Each
+     * ManagedClientGroup may be attached, detached, and reattached dynamically
+     * from different client views, with its contents automatically displayed
+     * in a tiled arrangment if needed.
+     * 
+     * @constructor
+     * @param {ManagedClientGroup|Object} [template={}]
+     *     The object whose properties should be copied within the new
+     *     ManagedClientGroup.
+     */
+    var ManagedClientGroup = function ManagedClientGroup(template) {
+
+        // Use empty object by default
+        template = template || {};
+
+        /**
+         * The time that this group was last brought to the foreground of
+         * the current tab, as the number of milliseconds elapsed since
+         * midnight of January 1, 1970 UTC. If the group has not yet been
+         * viewed, this will be 0.
+         *
+         * @type Number
+         */
+        this.lastUsed = template.lastUsed || 0;
+
+        /**
+         * Whether this ManagedClientGroup is currently attached to the client
+         * interface (true) or is running in the background (false).
+         *
+         * @type {boolean}
+         * @default false
+         */
+        this.attached = template.attached || false;
+
+        /**
+         * The clients that should be displayed within the client interface
+         * when this group is attached.
+         *
+         * @type {ManagedClient[]}
+         * @default []
+         */
+        this.clients = template.clients || [];
+
+        /**
+         * The number of rows that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.rows = template.rows || ManagedClientGroup.getRows(this);
+
+        /**
+         * The number of columns that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.columns = template.columns || ManagedClientGroup.getColumns(this);
+
+    };
+
+    /**
+     * Updates the number of rows and columns stored within the given
+     * ManagedClientGroup such that the clients within the group are evenly
+     * distributed. This function should be called whenever the size of a
+     * group changes.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup that should be updated.
+     */
+    ManagedClientGroup.recalculateTiles = function recalculateTiles(group) {
+
+        var recalculated = new ManagedClientGroup({
+            clients : group.clients
+        });
+
+        group.rows = recalculated.rows;
+        group.columns = recalculated.columns;
+
+    };
+
+    /**
+     * Returns the unique ID representing the given ManagedClientGroup or set
+     * of client IDs. The ID of a ManagedClientGroup consists simply of the
+     * IDs of all its ManagedClients, separated by periods.
+     *
+     * @param {ManagedClientGroup|string[]} group
+     *     The ManagedClientGroup or array of client IDs to determine the
+     *     ManagedClientGroup ID of.
+     *
+     * @returns {string}
+     *     The unique ID representing the given ManagedClientGroup, or the
+     *     unique ID that would represent a ManagedClientGroup containing the
+     *     clients with the given IDs.
+     */
+    ManagedClientGroup.getIdentifier = function getIdentifier(group) {
+
+        if (!_.isArray(group))
+            group = _.map(group.clients, client => client.id);
+
+        return group.join('.');
+
+    };
+
+    /**
+     * Returns an array of client identifiers for all clients contained within
+     * the given ManagedClientGroup. Orcer of the identifiers is preserved

Review comment:
       Corrected via rebase.

##########
File path: guacamole/src/main/frontend/src/app/client/types/ManagedClientGroup.js
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.
+ */
+
+/**
+ * Provides the ManagedClientGroup class used by the guacClientManager service.
+ */
+angular.module('client').factory('ManagedClientGroup', ['$injector', function defineManagedClientGroup($injector) {
+
+    /**
+     * Object which serves as a grouping of ManagedClients. Each
+     * ManagedClientGroup may be attached, detached, and reattached dynamically
+     * from different client views, with its contents automatically displayed
+     * in a tiled arrangment if needed.
+     * 
+     * @constructor
+     * @param {ManagedClientGroup|Object} [template={}]
+     *     The object whose properties should be copied within the new
+     *     ManagedClientGroup.
+     */
+    var ManagedClientGroup = function ManagedClientGroup(template) {
+
+        // Use empty object by default
+        template = template || {};
+
+        /**
+         * The time that this group was last brought to the foreground of
+         * the current tab, as the number of milliseconds elapsed since
+         * midnight of January 1, 1970 UTC. If the group has not yet been
+         * viewed, this will be 0.
+         *
+         * @type Number
+         */
+        this.lastUsed = template.lastUsed || 0;
+
+        /**
+         * Whether this ManagedClientGroup is currently attached to the client
+         * interface (true) or is running in the background (false).
+         *
+         * @type {boolean}
+         * @default false
+         */
+        this.attached = template.attached || false;
+
+        /**
+         * The clients that should be displayed within the client interface
+         * when this group is attached.
+         *
+         * @type {ManagedClient[]}
+         * @default []
+         */
+        this.clients = template.clients || [];
+
+        /**
+         * The number of rows that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.rows = template.rows || ManagedClientGroup.getRows(this);
+
+        /**
+         * The number of columns that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.columns = template.columns || ManagedClientGroup.getColumns(this);
+
+    };
+
+    /**
+     * Updates the number of rows and columns stored within the given
+     * ManagedClientGroup such that the clients within the group are evenly
+     * distributed. This function should be called whenever the size of a
+     * group changes.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup that should be updated.
+     */
+    ManagedClientGroup.recalculateTiles = function recalculateTiles(group) {
+
+        var recalculated = new ManagedClientGroup({
+            clients : group.clients
+        });
+
+        group.rows = recalculated.rows;
+        group.columns = recalculated.columns;
+
+    };
+
+    /**
+     * Returns the unique ID representing the given ManagedClientGroup or set
+     * of client IDs. The ID of a ManagedClientGroup consists simply of the
+     * IDs of all its ManagedClients, separated by periods.
+     *
+     * @param {ManagedClientGroup|string[]} group
+     *     The ManagedClientGroup or array of client IDs to determine the
+     *     ManagedClientGroup ID of.
+     *
+     * @returns {string}
+     *     The unique ID representing the given ManagedClientGroup, or the
+     *     unique ID that would represent a ManagedClientGroup containing the
+     *     clients with the given IDs.
+     */
+    ManagedClientGroup.getIdentifier = function getIdentifier(group) {
+
+        if (!_.isArray(group))
+            group = _.map(group.clients, client => client.id);
+
+        return group.join('.');
+
+    };
+
+    /**
+     * Returns an array of client identifiers for all clients contained within
+     * the given ManagedClientGroup. Orcer of the identifiers is preserved
+     * with respect to the order of the clients within the group.
+     *
+     * @param {ManagedClientGroup|string} group
+     *     The ManagedClientGroup to retrieve the client identifiers from,
+     *     or its ID.
+     *
+     * @returns {string[]}
+     *     The client identifiers of all clients contained within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getClientIdentifiers = function getClientIdentifiers(group) {
+
+        if (_.isString(group))
+            return group.split(/\./);
+
+        return group.clients.map(client => client.id);
+
+    };
+
+    /**
+     * Returns the number of columns that should be used to evenly arrange
+     * all provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of columns that should be used for the grid of
+     *     clients.
+     */
+    ManagedClientGroup.getColumns = function getColumns(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(Math.sqrt(group.clients.length));
+
+    };
+
+    /**
+     * Returns the number of rows that should be used to evenly arrange all
+     * provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of rows that should be used for the grid of clients.
+     */
+    ManagedClientGroup.getRows = function getRows(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(group.clients.length / ManagedClientGroup.getColumns(group));
+
+    };
+
+    /**
+     * Returns the title which should be displayed as the page title if the
+     * given client group is attached to the interface.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the title of.
+     *
+     * @returns {string}
+     *     The title of the given ManagedClientGroup.
+     */
+    ManagedClientGroup.getTitle = function getTitle(group) {
+
+        // Use client-specific title if only one client
+        if (group.clients.length === 1)
+            return group.clients[0].title;
+
+        // With multiple clients, somehow combining multiple page titles would
+        // be confusing. Instead, use the combined names.
+        return ManagedClientGroup.getName(group);
+
+    };
+
+    /**
+     * Returns the combined names of all clients within the given
+     * ManagedClientGroup, as determined by the names of the associated
+     * connections or connection groups.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the name of.
+     *
+     * @returns {string}
+     *     The combined names of all clients within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getName = function getName(group) {
+
+        // Generate a name from ONLY the focused clients, unless there are no
+        // focused clients
+        var relevantClients = _.filter(group.clients, client => client.clientProperties.focused);
+        if (!relevantClients.length)
+            relevantClients = group.clients;
+
+        return _.filter(relevantClients, (client => !!client.name)).map(client => client.name).join(', ') || '...';
+
+    };
+
+    /**
+     * A callback that is invoked for a ManagedClient within a ManagedClientGroup.
+     *
+     * @callback ManagedClientGroup~clientCallback
+     * @param {ManagedClient} client
+     *     The relevant ManagedClient.
+     *
+     * @param {number} row
+     *     The row number of the client within the tiled grid, where 0 is the
+     *     first row.
+     *
+     * @param {number} column
+     *     The column number of the client within the tiled grid, where 0 is
+     *     the first column.
+     *
+     * @param {number} index
+     *     The index of the client within the relevant
+     *     {@link ManagedClientGroup#clients} array.
+     */
+
+    /**
+     * Loops through each of the clients associated with the given
+     * ManagedClientGroup, invoking the given callback for each client.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to loop through.
+     *
+     * @param {ManagedClientGroup~clientCallback} callback
+     *     The callback to invoke for each of the clients within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.forEach = function forEach(group, callback) {
+        var current = 0;
+        for (var row = 0; row < group.rows; row++) {
+            for (var column = 0; column < group.columns; column++) {
+
+                callback(group.clients[current], row, column, current);
+                current++;
+
+                if (current >= group.clients.length)
+                    return;
+
+            }
+        }
+    };
+
+    /**
+     * Returns whether the given ManagedClientGroup contains more than one
+     * client.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to test.
+     *
+     * @returns {boolean}
+     *     true if two or more clients are currently present in the given
+     *     group, false otherwise.
+     */
+    ManagedClientGroup.hasMultipleClients = function hasMultipleClients(group) {
+        return group && group.clients.length > 1;
+    };
+
+    /**
+     * Returns a two-dimensional array of all ManagedClients within the given
+     * group, arranged in the grid defined by {@link ManagedClientGroup#rows}
+     * and {@link ManagedClientGroup#columns}. If any grid cell lacks a
+     * corresponding client (because the number of clients does not divide
+     * evenly into a grid), that cell will be null.
+     * 
+     * For the sake of AngularJS scope watches, the results of calling this
+     * function are cached and will always favor modifying an existing array
+     * over creating a new array, even for nested arrays.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup defining the tiled grid arrangement of
+     *     ManagedClients.
+     *
+     * @returns {ManagedClient[][]}
+     *     A two-dimensional array of all ManagedClients within the given
+     *     group.
+     */
+    ManagedClientGroup.getClientGrid = function getClientGrid(group) {
+
+        var index = 0;
+
+        // Operate on cached copy of grid
+        var clientGrid = group._grid || (group._grid = []);
+
+        // Delete any rows in excess of the required size
+        clientGrid.splice(group.rows);
+
+        for (var row = 0; row < group.rows; row++) {
+
+            // Prefer to use existing column arrays, deleting any columnd in

Review comment:
       Corrected via rebase.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r666068413



##########
File path: guacamole/src/main/frontend/src/app/client/controllers/clientController.js
##########
@@ -26,6 +26,7 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
     // Required types
     var ConnectionGroup    = $injector.get('ConnectionGroup');
     var ManagedClient      = $injector.get('ManagedClient');
+    var ManagedClientGroup = $injector.get('ManagedClientGroup');

Review comment:
       OK - migrated all directly touched code from `var` to `const`/`let` via rebase. I avoided making sweeping changes, updating only the code that is already within this diff.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665850089



##########
File path: guacamole/src/main/frontend/src/app/client/types/ManagedClientGroup.js
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.
+ */
+
+/**
+ * Provides the ManagedClientGroup class used by the guacClientManager service.
+ */
+angular.module('client').factory('ManagedClientGroup', ['$injector', function defineManagedClientGroup($injector) {
+
+    /**
+     * Object which serves as a grouping of ManagedClients. Each
+     * ManagedClientGroup may be attached, detached, and reattached dynamically
+     * from different client views, with its contents automatically displayed
+     * in a tiled arrangment if needed.
+     * 
+     * @constructor
+     * @param {ManagedClientGroup|Object} [template={}]
+     *     The object whose properties should be copied within the new
+     *     ManagedClientGroup.
+     */
+    var ManagedClientGroup = function ManagedClientGroup(template) {
+
+        // Use empty object by default
+        template = template || {};
+
+        /**
+         * The time that this group was last brought to the foreground of
+         * the current tab, as the number of milliseconds elapsed since
+         * midnight of January 1, 1970 UTC. If the group has not yet been
+         * viewed, this will be 0.
+         *
+         * @type Number
+         */
+        this.lastUsed = template.lastUsed || 0;
+
+        /**
+         * Whether this ManagedClientGroup is currently attached to the client
+         * interface (true) or is running in the background (false).
+         *
+         * @type {boolean}
+         * @default false
+         */
+        this.attached = template.attached || false;
+
+        /**
+         * The clients that should be displayed within the client interface
+         * when this group is attached.
+         *
+         * @type {ManagedClient[]}
+         * @default []
+         */
+        this.clients = template.clients || [];
+
+        /**
+         * The number of rows that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.rows = template.rows || ManagedClientGroup.getRows(this);
+
+        /**
+         * The number of columns that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.columns = template.columns || ManagedClientGroup.getColumns(this);
+
+    };
+
+    /**
+     * Updates the number of rows and columns stored within the given
+     * ManagedClientGroup such that the clients within the group are evenly
+     * distributed. This function should be called whenever the size of a
+     * group changes.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup that should be updated.
+     */
+    ManagedClientGroup.recalculateTiles = function recalculateTiles(group) {
+
+        var recalculated = new ManagedClientGroup({
+            clients : group.clients
+        });
+
+        group.rows = recalculated.rows;
+        group.columns = recalculated.columns;
+
+    };
+
+    /**
+     * Returns the unique ID representing the given ManagedClientGroup or set
+     * of client IDs. The ID of a ManagedClientGroup consists simply of the
+     * IDs of all its ManagedClients, separated by periods.
+     *
+     * @param {ManagedClientGroup|string[]} group
+     *     The ManagedClientGroup or array of client IDs to determine the
+     *     ManagedClientGroup ID of.
+     *
+     * @returns {string}
+     *     The unique ID representing the given ManagedClientGroup, or the
+     *     unique ID that would represent a ManagedClientGroup containing the
+     *     clients with the given IDs.
+     */
+    ManagedClientGroup.getIdentifier = function getIdentifier(group) {
+
+        if (!_.isArray(group))
+            group = _.map(group.clients, client => client.id);
+
+        return group.join('.');
+
+    };
+
+    /**
+     * Returns an array of client identifiers for all clients contained within
+     * the given ManagedClientGroup. Orcer of the identifiers is preserved
+     * with respect to the order of the clients within the group.
+     *
+     * @param {ManagedClientGroup|string} group
+     *     The ManagedClientGroup to retrieve the client identifiers from,
+     *     or its ID.
+     *
+     * @returns {string[]}
+     *     The client identifiers of all clients contained within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getClientIdentifiers = function getClientIdentifiers(group) {
+
+        if (_.isString(group))
+            return group.split(/\./);
+
+        return group.clients.map(client => client.id);
+
+    };
+
+    /**
+     * Returns the number of columns that should be used to evenly arrange
+     * all provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of columns that should be used for the grid of
+     *     clients.
+     */
+    ManagedClientGroup.getColumns = function getColumns(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(Math.sqrt(group.clients.length));
+
+    };
+
+    /**
+     * Returns the number of rows that should be used to evenly arrange all
+     * provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of rows that should be used for the grid of clients.
+     */
+    ManagedClientGroup.getRows = function getRows(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(group.clients.length / ManagedClientGroup.getColumns(group));
+
+    };
+
+    /**
+     * Returns the title which should be displayed as the page title if the
+     * given client group is attached to the interface.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the title of.
+     *
+     * @returns {string}
+     *     The title of the given ManagedClientGroup.
+     */
+    ManagedClientGroup.getTitle = function getTitle(group) {
+
+        // Use client-specific title if only one client
+        if (group.clients.length === 1)
+            return group.clients[0].title;
+
+        // With multiple clients, somehow combining multiple page titles would
+        // be confusing. Instead, use the combined names.
+        return ManagedClientGroup.getName(group);
+
+    };
+
+    /**
+     * Returns the combined names of all clients within the given
+     * ManagedClientGroup, as determined by the names of the associated
+     * connections or connection groups.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the name of.
+     *
+     * @returns {string}
+     *     The combined names of all clients within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getName = function getName(group) {
+
+        // Generate a name from ONLY the focused clients, unless there are no
+        // focused clients
+        var relevantClients = _.filter(group.clients, client => client.clientProperties.focused);
+        if (!relevantClients.length)
+            relevantClients = group.clients;
+
+        return _.filter(relevantClients, (client => !!client.name)).map(client => client.name).join(', ') || '...';
+
+    };
+
+    /**
+     * A callback that is invoked for a ManagedClient within a ManagedClientGroup.

Review comment:
       What callback is this referring to?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r666068413



##########
File path: guacamole/src/main/frontend/src/app/client/controllers/clientController.js
##########
@@ -26,6 +26,7 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
     // Required types
     var ConnectionGroup    = $injector.get('ConnectionGroup');
     var ManagedClient      = $injector.get('ManagedClient');
+    var ManagedClientGroup = $injector.get('ManagedClientGroup');

Review comment:
       OK - migrated all directly touched code from `var` to `const`/`let`. I avoided making sweeping changes, updating only the code that is already within this diff.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665849549



##########
File path: guacamole/src/main/frontend/src/app/client/controllers/clientController.js
##########
@@ -26,6 +26,7 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
     // Required types
     var ConnectionGroup    = $injector.get('ConnectionGroup');
     var ManagedClient      = $injector.get('ManagedClient');
+    var ManagedClientGroup = $injector.get('ManagedClientGroup');

Review comment:
       Sure. I'll at least update the files that have been touched here.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665845738



##########
File path: guacamole/src/main/frontend/src/app/client/directives/guacClientNotification.js
##########
@@ -0,0 +1,431 @@
+/*
+ * 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.
+ */
+
+/**
+ * A directive for displaying a non-global notification describing the status
+ * of a specific Guacamole client, including prompts for any information
+ * necessary to continue the connection.
+ */
+angular.module('client').directive('guacClientNotification', [function guacClientNotification() {
+
+    var directive = {
+        restrict: 'E',
+        replace: true,
+        templateUrl: 'app/client/templates/guacClientNotification.html'
+    };
+
+    directive.scope = {
+
+        /**
+         * The client whose status should be displayed.
+         * 
+         * @type ManagedClient
+         */
+        client : '='
+        
+    };
+
+    directive.controller = ['$scope', '$injector', '$element',
+        function guacClientNotificationController($scope, $injector, $element) {
+   
+        // Required types
+        var ManagedClient      = $injector.get('ManagedClient');
+        var ManagedClientState = $injector.get('ManagedClientState');
+        var Protocol           = $injector.get('Protocol');
+
+        // Required services
+        var $location              = $injector.get('$location');
+        var authenticationService  = $injector.get('authenticationService');
+        var guacClientManager      = $injector.get('guacClientManager');
+        var requestService         = $injector.get('requestService');
+        var userPageService        = $injector.get('userPageService');
+
+        /**
+         * A Notification object describing the client status to display as a
+         * dialog or prompt, as would be accepted by guacNotification.showStatus(),
+         * or false if no status should be shown.
+         *
+         * @type {Notification|Object|Boolean}
+         */
+        $scope.status = false;
+
+        /**
+         * All client error codes handled and passed off for translation. Any error
+         * code not present in this list will be represented by the "DEFAULT"
+         * translation.
+         */
+        var CLIENT_ERRORS = {

Review comment:
       Have you considered the `Set` type? It's a lot less awkward than this.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665850967



##########
File path: guacamole/src/main/frontend/src/app/client/types/ManagedClientGroup.js
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.
+ */
+
+/**
+ * Provides the ManagedClientGroup class used by the guacClientManager service.
+ */
+angular.module('client').factory('ManagedClientGroup', ['$injector', function defineManagedClientGroup($injector) {
+
+    /**
+     * Object which serves as a grouping of ManagedClients. Each
+     * ManagedClientGroup may be attached, detached, and reattached dynamically
+     * from different client views, with its contents automatically displayed
+     * in a tiled arrangment if needed.
+     * 
+     * @constructor
+     * @param {ManagedClientGroup|Object} [template={}]
+     *     The object whose properties should be copied within the new
+     *     ManagedClientGroup.
+     */
+    var ManagedClientGroup = function ManagedClientGroup(template) {
+
+        // Use empty object by default
+        template = template || {};
+
+        /**
+         * The time that this group was last brought to the foreground of
+         * the current tab, as the number of milliseconds elapsed since
+         * midnight of January 1, 1970 UTC. If the group has not yet been
+         * viewed, this will be 0.
+         *
+         * @type Number
+         */
+        this.lastUsed = template.lastUsed || 0;
+
+        /**
+         * Whether this ManagedClientGroup is currently attached to the client
+         * interface (true) or is running in the background (false).
+         *
+         * @type {boolean}
+         * @default false
+         */
+        this.attached = template.attached || false;
+
+        /**
+         * The clients that should be displayed within the client interface
+         * when this group is attached.
+         *
+         * @type {ManagedClient[]}
+         * @default []
+         */
+        this.clients = template.clients || [];
+
+        /**
+         * The number of rows that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.rows = template.rows || ManagedClientGroup.getRows(this);
+
+        /**
+         * The number of columns that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.columns = template.columns || ManagedClientGroup.getColumns(this);
+
+    };
+
+    /**
+     * Updates the number of rows and columns stored within the given
+     * ManagedClientGroup such that the clients within the group are evenly
+     * distributed. This function should be called whenever the size of a
+     * group changes.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup that should be updated.
+     */
+    ManagedClientGroup.recalculateTiles = function recalculateTiles(group) {
+
+        var recalculated = new ManagedClientGroup({
+            clients : group.clients
+        });
+
+        group.rows = recalculated.rows;
+        group.columns = recalculated.columns;
+
+    };
+
+    /**
+     * Returns the unique ID representing the given ManagedClientGroup or set
+     * of client IDs. The ID of a ManagedClientGroup consists simply of the
+     * IDs of all its ManagedClients, separated by periods.
+     *
+     * @param {ManagedClientGroup|string[]} group
+     *     The ManagedClientGroup or array of client IDs to determine the
+     *     ManagedClientGroup ID of.
+     *
+     * @returns {string}
+     *     The unique ID representing the given ManagedClientGroup, or the
+     *     unique ID that would represent a ManagedClientGroup containing the
+     *     clients with the given IDs.
+     */
+    ManagedClientGroup.getIdentifier = function getIdentifier(group) {
+
+        if (!_.isArray(group))
+            group = _.map(group.clients, client => client.id);
+
+        return group.join('.');
+
+    };
+
+    /**
+     * Returns an array of client identifiers for all clients contained within
+     * the given ManagedClientGroup. Orcer of the identifiers is preserved
+     * with respect to the order of the clients within the group.
+     *
+     * @param {ManagedClientGroup|string} group
+     *     The ManagedClientGroup to retrieve the client identifiers from,
+     *     or its ID.
+     *
+     * @returns {string[]}
+     *     The client identifiers of all clients contained within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getClientIdentifiers = function getClientIdentifiers(group) {
+
+        if (_.isString(group))
+            return group.split(/\./);
+
+        return group.clients.map(client => client.id);
+
+    };
+
+    /**
+     * Returns the number of columns that should be used to evenly arrange
+     * all provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of columns that should be used for the grid of
+     *     clients.
+     */
+    ManagedClientGroup.getColumns = function getColumns(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(Math.sqrt(group.clients.length));
+
+    };
+
+    /**
+     * Returns the number of rows that should be used to evenly arrange all
+     * provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of rows that should be used for the grid of clients.
+     */
+    ManagedClientGroup.getRows = function getRows(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(group.clients.length / ManagedClientGroup.getColumns(group));
+
+    };
+
+    /**
+     * Returns the title which should be displayed as the page title if the
+     * given client group is attached to the interface.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the title of.
+     *
+     * @returns {string}
+     *     The title of the given ManagedClientGroup.
+     */
+    ManagedClientGroup.getTitle = function getTitle(group) {
+
+        // Use client-specific title if only one client
+        if (group.clients.length === 1)
+            return group.clients[0].title;
+
+        // With multiple clients, somehow combining multiple page titles would
+        // be confusing. Instead, use the combined names.
+        return ManagedClientGroup.getName(group);
+
+    };
+
+    /**
+     * Returns the combined names of all clients within the given
+     * ManagedClientGroup, as determined by the names of the associated
+     * connections or connection groups.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the name of.
+     *
+     * @returns {string}
+     *     The combined names of all clients within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getName = function getName(group) {
+
+        // Generate a name from ONLY the focused clients, unless there are no
+        // focused clients
+        var relevantClients = _.filter(group.clients, client => client.clientProperties.focused);
+        if (!relevantClients.length)
+            relevantClients = group.clients;
+
+        return _.filter(relevantClients, (client => !!client.name)).map(client => client.name).join(', ') || '...';
+
+    };
+
+    /**
+     * A callback that is invoked for a ManagedClient within a ManagedClientGroup.

Review comment:
       Yes. It's type documentation for the callback used by `forEach`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner merged pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner merged pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665866045



##########
File path: guacamole/src/main/frontend/src/app/client/directives/guacClientNotification.js
##########
@@ -0,0 +1,431 @@
+/*
+ * 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.
+ */
+
+/**
+ * A directive for displaying a non-global notification describing the status
+ * of a specific Guacamole client, including prompts for any information
+ * necessary to continue the connection.
+ */
+angular.module('client').directive('guacClientNotification', [function guacClientNotification() {
+
+    var directive = {
+        restrict: 'E',
+        replace: true,
+        templateUrl: 'app/client/templates/guacClientNotification.html'
+    };
+
+    directive.scope = {
+
+        /**
+         * The client whose status should be displayed.
+         * 
+         * @type ManagedClient
+         */
+        client : '='
+        
+    };
+
+    directive.controller = ['$scope', '$injector', '$element',
+        function guacClientNotificationController($scope, $injector, $element) {
+   
+        // Required types
+        var ManagedClient      = $injector.get('ManagedClient');
+        var ManagedClientState = $injector.get('ManagedClientState');
+        var Protocol           = $injector.get('Protocol');
+
+        // Required services
+        var $location              = $injector.get('$location');
+        var authenticationService  = $injector.get('authenticationService');
+        var guacClientManager      = $injector.get('guacClientManager');
+        var requestService         = $injector.get('requestService');
+        var userPageService        = $injector.get('userPageService');
+
+        /**
+         * A Notification object describing the client status to display as a
+         * dialog or prompt, as would be accepted by guacNotification.showStatus(),
+         * or false if no status should be shown.
+         *
+         * @type {Notification|Object|Boolean}
+         */
+        $scope.status = false;
+
+        /**
+         * All client error codes handled and passed off for translation. Any error
+         * code not present in this list will be represented by the "DEFAULT"
+         * translation.
+         */
+        var CLIENT_ERRORS = {

Review comment:
       No dice:
   
   ![Screenshot 2021-07-07 9 38 21 PM](https://user-images.githubusercontent.com/4632905/124863434-ea47c980-df6b-11eb-8cb0-07499231843a.png)
   
   If we wanted to use `Set`, we'd need a polyfill.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665847580



##########
File path: guacamole/src/main/frontend/src/app/client/controllers/clientController.js
##########
@@ -370,134 +384,58 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
     var substituteKeysPressed = {};
 
     /**
-     * Map of all currently pressed keys (by keysym) to the clipboard contents
-     * received from the remote desktop while those keys were pressed. All keys
-     * not currently pressed will not have entries within this map.
+     * Returns whether the shortcut for showing/hiding the Guacamole menu
+     * (Ctrl+Alt+Shift) has been pressed.
      *
-     * @type Object.<Number, ClipboardData>
-     */
-    var clipboardDataFromKey = {};
-
-    /*
-     * Check to see if all currently pressed keys are in the set of menu keys.
+     * @param {Guacamole.Keyboard} keyboard
+     *     The Guacamole.Keyboard object tracking the local keyboard state.
+     *
+     * @returns {boolean}
+     *     true of Ctrl+Alt+Shift has been pressed, false otherwise.

Review comment:
       Typo: "true of ..."




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner commented on pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#issuecomment-880193833


   Well, I think all my concerns are now addressed. :shipit: 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665857316



##########
File path: guacamole/src/main/frontend/src/app/client/directives/guacClientNotification.js
##########
@@ -0,0 +1,431 @@
+/*
+ * 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.
+ */
+
+/**
+ * A directive for displaying a non-global notification describing the status
+ * of a specific Guacamole client, including prompts for any information
+ * necessary to continue the connection.
+ */
+angular.module('client').directive('guacClientNotification', [function guacClientNotification() {
+
+    var directive = {
+        restrict: 'E',
+        replace: true,
+        templateUrl: 'app/client/templates/guacClientNotification.html'
+    };
+
+    directive.scope = {
+
+        /**
+         * The client whose status should be displayed.
+         * 
+         * @type ManagedClient
+         */
+        client : '='
+        
+    };
+
+    directive.controller = ['$scope', '$injector', '$element',
+        function guacClientNotificationController($scope, $injector, $element) {
+   
+        // Required types
+        var ManagedClient      = $injector.get('ManagedClient');
+        var ManagedClientState = $injector.get('ManagedClientState');
+        var Protocol           = $injector.get('Protocol');
+
+        // Required services
+        var $location              = $injector.get('$location');
+        var authenticationService  = $injector.get('authenticationService');
+        var guacClientManager      = $injector.get('guacClientManager');
+        var requestService         = $injector.get('requestService');
+        var userPageService        = $injector.get('userPageService');
+
+        /**
+         * A Notification object describing the client status to display as a
+         * dialog or prompt, as would be accepted by guacNotification.showStatus(),
+         * or false if no status should be shown.
+         *
+         * @type {Notification|Object|Boolean}
+         */
+        $scope.status = false;
+
+        /**
+         * All client error codes handled and passed off for translation. Any error
+         * code not present in this list will be represented by the "DEFAULT"
+         * translation.
+         */
+        var CLIENT_ERRORS = {

Review comment:
       I haven't, no. Is that something that will get magically compiled down to essentially this? I know things like the arrow notation, `const`, etc. are compiled away during the build, allowing us to maintain IE10+ compatibility, but I'm not sure about full-fledged objects/types like `Set`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665850326



##########
File path: guacamole/src/main/frontend/src/app/client/types/ManagedClientGroup.js
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.
+ */
+
+/**
+ * Provides the ManagedClientGroup class used by the guacClientManager service.
+ */
+angular.module('client').factory('ManagedClientGroup', ['$injector', function defineManagedClientGroup($injector) {
+
+    /**
+     * Object which serves as a grouping of ManagedClients. Each
+     * ManagedClientGroup may be attached, detached, and reattached dynamically
+     * from different client views, with its contents automatically displayed
+     * in a tiled arrangment if needed.
+     * 
+     * @constructor
+     * @param {ManagedClientGroup|Object} [template={}]
+     *     The object whose properties should be copied within the new
+     *     ManagedClientGroup.
+     */
+    var ManagedClientGroup = function ManagedClientGroup(template) {
+
+        // Use empty object by default
+        template = template || {};
+
+        /**
+         * The time that this group was last brought to the foreground of
+         * the current tab, as the number of milliseconds elapsed since
+         * midnight of January 1, 1970 UTC. If the group has not yet been
+         * viewed, this will be 0.
+         *
+         * @type Number
+         */
+        this.lastUsed = template.lastUsed || 0;
+
+        /**
+         * Whether this ManagedClientGroup is currently attached to the client
+         * interface (true) or is running in the background (false).
+         *
+         * @type {boolean}
+         * @default false
+         */
+        this.attached = template.attached || false;
+
+        /**
+         * The clients that should be displayed within the client interface
+         * when this group is attached.
+         *
+         * @type {ManagedClient[]}
+         * @default []
+         */
+        this.clients = template.clients || [];
+
+        /**
+         * The number of rows that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.rows = template.rows || ManagedClientGroup.getRows(this);
+
+        /**
+         * The number of columns that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.columns = template.columns || ManagedClientGroup.getColumns(this);
+
+    };
+
+    /**
+     * Updates the number of rows and columns stored within the given
+     * ManagedClientGroup such that the clients within the group are evenly
+     * distributed. This function should be called whenever the size of a
+     * group changes.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup that should be updated.
+     */
+    ManagedClientGroup.recalculateTiles = function recalculateTiles(group) {
+
+        var recalculated = new ManagedClientGroup({
+            clients : group.clients
+        });
+
+        group.rows = recalculated.rows;
+        group.columns = recalculated.columns;
+
+    };
+
+    /**
+     * Returns the unique ID representing the given ManagedClientGroup or set
+     * of client IDs. The ID of a ManagedClientGroup consists simply of the
+     * IDs of all its ManagedClients, separated by periods.
+     *
+     * @param {ManagedClientGroup|string[]} group
+     *     The ManagedClientGroup or array of client IDs to determine the
+     *     ManagedClientGroup ID of.
+     *
+     * @returns {string}
+     *     The unique ID representing the given ManagedClientGroup, or the
+     *     unique ID that would represent a ManagedClientGroup containing the
+     *     clients with the given IDs.
+     */
+    ManagedClientGroup.getIdentifier = function getIdentifier(group) {
+
+        if (!_.isArray(group))
+            group = _.map(group.clients, client => client.id);
+
+        return group.join('.');
+
+    };
+
+    /**
+     * Returns an array of client identifiers for all clients contained within
+     * the given ManagedClientGroup. Orcer of the identifiers is preserved
+     * with respect to the order of the clients within the group.
+     *
+     * @param {ManagedClientGroup|string} group
+     *     The ManagedClientGroup to retrieve the client identifiers from,
+     *     or its ID.
+     *
+     * @returns {string[]}
+     *     The client identifiers of all clients contained within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getClientIdentifiers = function getClientIdentifiers(group) {
+
+        if (_.isString(group))
+            return group.split(/\./);
+
+        return group.clients.map(client => client.id);
+
+    };
+
+    /**
+     * Returns the number of columns that should be used to evenly arrange
+     * all provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of columns that should be used for the grid of
+     *     clients.
+     */
+    ManagedClientGroup.getColumns = function getColumns(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(Math.sqrt(group.clients.length));
+
+    };
+
+    /**
+     * Returns the number of rows that should be used to evenly arrange all
+     * provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of rows that should be used for the grid of clients.
+     */
+    ManagedClientGroup.getRows = function getRows(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(group.clients.length / ManagedClientGroup.getColumns(group));
+
+    };
+
+    /**
+     * Returns the title which should be displayed as the page title if the
+     * given client group is attached to the interface.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the title of.
+     *
+     * @returns {string}
+     *     The title of the given ManagedClientGroup.
+     */
+    ManagedClientGroup.getTitle = function getTitle(group) {
+
+        // Use client-specific title if only one client
+        if (group.clients.length === 1)
+            return group.clients[0].title;
+
+        // With multiple clients, somehow combining multiple page titles would
+        // be confusing. Instead, use the combined names.
+        return ManagedClientGroup.getName(group);
+
+    };
+
+    /**
+     * Returns the combined names of all clients within the given
+     * ManagedClientGroup, as determined by the names of the associated
+     * connections or connection groups.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the name of.
+     *
+     * @returns {string}
+     *     The combined names of all clients within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getName = function getName(group) {
+
+        // Generate a name from ONLY the focused clients, unless there are no
+        // focused clients
+        var relevantClients = _.filter(group.clients, client => client.clientProperties.focused);
+        if (!relevantClients.length)
+            relevantClients = group.clients;
+
+        return _.filter(relevantClients, (client => !!client.name)).map(client => client.name).join(', ') || '...';
+
+    };
+
+    /**
+     * A callback that is invoked for a ManagedClient within a ManagedClientGroup.

Review comment:
       I guess it's referring to the callback invoked by `forEach` below?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665846968



##########
File path: guacamole/src/main/frontend/src/app/client/controllers/clientController.js
##########
@@ -26,6 +26,7 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
     // Required types
     var ConnectionGroup    = $injector.get('ConnectionGroup');
     var ManagedClient      = $injector.get('ManagedClient');
+    var ManagedClientGroup = $injector.get('ManagedClientGroup');

Review comment:
       What do you think of the new `let` and `const` keywords? I'm certainly not asking you to rewrite this beast, but I find them to be clearer than using `var`. 
   
   Plus we'll get compile-time checking for things that shouldn't be mutated.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665850871



##########
File path: guacamole/src/main/frontend/src/app/client/types/ManagedClientGroup.js
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.
+ */
+
+/**
+ * Provides the ManagedClientGroup class used by the guacClientManager service.
+ */
+angular.module('client').factory('ManagedClientGroup', ['$injector', function defineManagedClientGroup($injector) {
+
+    /**
+     * Object which serves as a grouping of ManagedClients. Each
+     * ManagedClientGroup may be attached, detached, and reattached dynamically
+     * from different client views, with its contents automatically displayed
+     * in a tiled arrangment if needed.
+     * 
+     * @constructor
+     * @param {ManagedClientGroup|Object} [template={}]
+     *     The object whose properties should be copied within the new
+     *     ManagedClientGroup.
+     */
+    var ManagedClientGroup = function ManagedClientGroup(template) {
+
+        // Use empty object by default
+        template = template || {};
+
+        /**
+         * The time that this group was last brought to the foreground of
+         * the current tab, as the number of milliseconds elapsed since
+         * midnight of January 1, 1970 UTC. If the group has not yet been
+         * viewed, this will be 0.
+         *
+         * @type Number
+         */
+        this.lastUsed = template.lastUsed || 0;
+
+        /**
+         * Whether this ManagedClientGroup is currently attached to the client
+         * interface (true) or is running in the background (false).
+         *
+         * @type {boolean}
+         * @default false
+         */
+        this.attached = template.attached || false;
+
+        /**
+         * The clients that should be displayed within the client interface
+         * when this group is attached.
+         *
+         * @type {ManagedClient[]}
+         * @default []
+         */
+        this.clients = template.clients || [];
+
+        /**
+         * The number of rows that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.rows = template.rows || ManagedClientGroup.getRows(this);
+
+        /**
+         * The number of columns that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.columns = template.columns || ManagedClientGroup.getColumns(this);
+
+    };
+
+    /**
+     * Updates the number of rows and columns stored within the given
+     * ManagedClientGroup such that the clients within the group are evenly
+     * distributed. This function should be called whenever the size of a
+     * group changes.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup that should be updated.
+     */
+    ManagedClientGroup.recalculateTiles = function recalculateTiles(group) {
+
+        var recalculated = new ManagedClientGroup({
+            clients : group.clients
+        });
+
+        group.rows = recalculated.rows;
+        group.columns = recalculated.columns;
+
+    };
+
+    /**
+     * Returns the unique ID representing the given ManagedClientGroup or set
+     * of client IDs. The ID of a ManagedClientGroup consists simply of the
+     * IDs of all its ManagedClients, separated by periods.
+     *
+     * @param {ManagedClientGroup|string[]} group
+     *     The ManagedClientGroup or array of client IDs to determine the
+     *     ManagedClientGroup ID of.
+     *
+     * @returns {string}
+     *     The unique ID representing the given ManagedClientGroup, or the
+     *     unique ID that would represent a ManagedClientGroup containing the
+     *     clients with the given IDs.
+     */
+    ManagedClientGroup.getIdentifier = function getIdentifier(group) {
+
+        if (!_.isArray(group))
+            group = _.map(group.clients, client => client.id);
+
+        return group.join('.');
+
+    };
+
+    /**
+     * Returns an array of client identifiers for all clients contained within
+     * the given ManagedClientGroup. Orcer of the identifiers is preserved
+     * with respect to the order of the clients within the group.
+     *
+     * @param {ManagedClientGroup|string} group
+     *     The ManagedClientGroup to retrieve the client identifiers from,
+     *     or its ID.
+     *
+     * @returns {string[]}
+     *     The client identifiers of all clients contained within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getClientIdentifiers = function getClientIdentifiers(group) {
+
+        if (_.isString(group))
+            return group.split(/\./);
+
+        return group.clients.map(client => client.id);
+
+    };
+
+    /**
+     * Returns the number of columns that should be used to evenly arrange
+     * all provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of columns that should be used for the grid of
+     *     clients.
+     */
+    ManagedClientGroup.getColumns = function getColumns(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(Math.sqrt(group.clients.length));
+
+    };
+
+    /**
+     * Returns the number of rows that should be used to evenly arrange all
+     * provided clients in a tiled grid.
+     *
+     * @returns {Number}
+     *     The number of rows that should be used for the grid of clients.
+     */
+    ManagedClientGroup.getRows = function getRows(group) {
+
+        if (!group.clients.length)
+            return 0;
+
+        return Math.ceil(group.clients.length / ManagedClientGroup.getColumns(group));
+
+    };
+
+    /**
+     * Returns the title which should be displayed as the page title if the
+     * given client group is attached to the interface.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the title of.
+     *
+     * @returns {string}
+     *     The title of the given ManagedClientGroup.
+     */
+    ManagedClientGroup.getTitle = function getTitle(group) {
+
+        // Use client-specific title if only one client
+        if (group.clients.length === 1)
+            return group.clients[0].title;
+
+        // With multiple clients, somehow combining multiple page titles would
+        // be confusing. Instead, use the combined names.
+        return ManagedClientGroup.getName(group);
+
+    };
+
+    /**
+     * Returns the combined names of all clients within the given
+     * ManagedClientGroup, as determined by the names of the associated
+     * connections or connection groups.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to determine the name of.
+     *
+     * @returns {string}
+     *     The combined names of all clients within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.getName = function getName(group) {
+
+        // Generate a name from ONLY the focused clients, unless there are no
+        // focused clients
+        var relevantClients = _.filter(group.clients, client => client.clientProperties.focused);
+        if (!relevantClients.length)
+            relevantClients = group.clients;
+
+        return _.filter(relevantClients, (client => !!client.name)).map(client => client.name).join(', ') || '...';
+
+    };
+
+    /**
+     * A callback that is invoked for a ManagedClient within a ManagedClientGroup.
+     *
+     * @callback ManagedClientGroup~clientCallback
+     * @param {ManagedClient} client
+     *     The relevant ManagedClient.
+     *
+     * @param {number} row
+     *     The row number of the client within the tiled grid, where 0 is the
+     *     first row.
+     *
+     * @param {number} column
+     *     The column number of the client within the tiled grid, where 0 is
+     *     the first column.
+     *
+     * @param {number} index
+     *     The index of the client within the relevant
+     *     {@link ManagedClientGroup#clients} array.
+     */
+
+    /**
+     * Loops through each of the clients associated with the given
+     * ManagedClientGroup, invoking the given callback for each client.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to loop through.
+     *
+     * @param {ManagedClientGroup~clientCallback} callback
+     *     The callback to invoke for each of the clients within the given
+     *     ManagedClientGroup.
+     */
+    ManagedClientGroup.forEach = function forEach(group, callback) {
+        var current = 0;
+        for (var row = 0; row < group.rows; row++) {
+            for (var column = 0; column < group.columns; column++) {
+
+                callback(group.clients[current], row, column, current);
+                current++;
+
+                if (current >= group.clients.length)
+                    return;
+
+            }
+        }
+    };
+
+    /**
+     * Returns whether the given ManagedClientGroup contains more than one
+     * client.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup to test.
+     *
+     * @returns {boolean}
+     *     true if two or more clients are currently present in the given
+     *     group, false otherwise.
+     */
+    ManagedClientGroup.hasMultipleClients = function hasMultipleClients(group) {
+        return group && group.clients.length > 1;
+    };
+
+    /**
+     * Returns a two-dimensional array of all ManagedClients within the given
+     * group, arranged in the grid defined by {@link ManagedClientGroup#rows}
+     * and {@link ManagedClientGroup#columns}. If any grid cell lacks a
+     * corresponding client (because the number of clients does not divide
+     * evenly into a grid), that cell will be null.
+     * 
+     * For the sake of AngularJS scope watches, the results of calling this
+     * function are cached and will always favor modifying an existing array
+     * over creating a new array, even for nested arrays.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup defining the tiled grid arrangement of
+     *     ManagedClients.
+     *
+     * @returns {ManagedClient[][]}
+     *     A two-dimensional array of all ManagedClients within the given
+     *     group.
+     */
+    ManagedClientGroup.getClientGrid = function getClientGrid(group) {
+
+        var index = 0;
+
+        // Operate on cached copy of grid
+        var clientGrid = group._grid || (group._grid = []);
+
+        // Delete any rows in excess of the required size
+        clientGrid.splice(group.rows);
+
+        for (var row = 0; row < group.rows; row++) {
+
+            // Prefer to use existing column arrays, deleting any columnd in

Review comment:
       typo: 'columnd'




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665862406



##########
File path: guacamole/src/main/frontend/src/app/client/directives/guacClientNotification.js
##########
@@ -0,0 +1,431 @@
+/*
+ * 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.
+ */
+
+/**
+ * A directive for displaying a non-global notification describing the status
+ * of a specific Guacamole client, including prompts for any information
+ * necessary to continue the connection.
+ */
+angular.module('client').directive('guacClientNotification', [function guacClientNotification() {
+
+    var directive = {
+        restrict: 'E',
+        replace: true,
+        templateUrl: 'app/client/templates/guacClientNotification.html'
+    };
+
+    directive.scope = {
+
+        /**
+         * The client whose status should be displayed.
+         * 
+         * @type ManagedClient
+         */
+        client : '='
+        
+    };
+
+    directive.controller = ['$scope', '$injector', '$element',
+        function guacClientNotificationController($scope, $injector, $element) {
+   
+        // Required types
+        var ManagedClient      = $injector.get('ManagedClient');
+        var ManagedClientState = $injector.get('ManagedClientState');
+        var Protocol           = $injector.get('Protocol');
+
+        // Required services
+        var $location              = $injector.get('$location');
+        var authenticationService  = $injector.get('authenticationService');
+        var guacClientManager      = $injector.get('guacClientManager');
+        var requestService         = $injector.get('requestService');
+        var userPageService        = $injector.get('userPageService');
+
+        /**
+         * A Notification object describing the client status to display as a
+         * dialog or prompt, as would be accepted by guacNotification.showStatus(),
+         * or false if no status should be shown.
+         *
+         * @type {Notification|Object|Boolean}
+         */
+        $scope.status = false;
+
+        /**
+         * All client error codes handled and passed off for translation. Any error
+         * code not present in this list will be represented by the "DEFAULT"
+         * translation.
+         */
+        var CLIENT_ERRORS = {

Review comment:
       I'll just do a quick test refactoring this one instance to `Set` and see what code is emitted.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#issuecomment-876533325


   Thanks @jmuehlner! I was trying to find time to review it and haven't had a lot, unfortunately.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jmuehlner commented on a change in pull request #623: GUACAMOLE-724: Add support for displaying multiple connections as tiles.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on a change in pull request #623:
URL: https://github.com/apache/guacamole-client/pull/623#discussion_r665849165



##########
File path: guacamole/src/main/frontend/src/app/client/types/ManagedClientGroup.js
##########
@@ -0,0 +1,359 @@
+/*
+ * 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.
+ */
+
+/**
+ * Provides the ManagedClientGroup class used by the guacClientManager service.
+ */
+angular.module('client').factory('ManagedClientGroup', ['$injector', function defineManagedClientGroup($injector) {
+
+    /**
+     * Object which serves as a grouping of ManagedClients. Each
+     * ManagedClientGroup may be attached, detached, and reattached dynamically
+     * from different client views, with its contents automatically displayed
+     * in a tiled arrangment if needed.
+     * 
+     * @constructor
+     * @param {ManagedClientGroup|Object} [template={}]
+     *     The object whose properties should be copied within the new
+     *     ManagedClientGroup.
+     */
+    var ManagedClientGroup = function ManagedClientGroup(template) {
+
+        // Use empty object by default
+        template = template || {};
+
+        /**
+         * The time that this group was last brought to the foreground of
+         * the current tab, as the number of milliseconds elapsed since
+         * midnight of January 1, 1970 UTC. If the group has not yet been
+         * viewed, this will be 0.
+         *
+         * @type Number
+         */
+        this.lastUsed = template.lastUsed || 0;
+
+        /**
+         * Whether this ManagedClientGroup is currently attached to the client
+         * interface (true) or is running in the background (false).
+         *
+         * @type {boolean}
+         * @default false
+         */
+        this.attached = template.attached || false;
+
+        /**
+         * The clients that should be displayed within the client interface
+         * when this group is attached.
+         *
+         * @type {ManagedClient[]}
+         * @default []
+         */
+        this.clients = template.clients || [];
+
+        /**
+         * The number of rows that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.rows = template.rows || ManagedClientGroup.getRows(this);
+
+        /**
+         * The number of columns that should be used when arranging the clients
+         * within this group in a grid. By default, this value is automatically
+         * calculated from the number of clients.
+         *
+         * @type {number}
+         */
+        this.columns = template.columns || ManagedClientGroup.getColumns(this);
+
+    };
+
+    /**
+     * Updates the number of rows and columns stored within the given
+     * ManagedClientGroup such that the clients within the group are evenly
+     * distributed. This function should be called whenever the size of a
+     * group changes.
+     *
+     * @param {ManagedClientGroup} group
+     *     The ManagedClientGroup that should be updated.
+     */
+    ManagedClientGroup.recalculateTiles = function recalculateTiles(group) {
+
+        var recalculated = new ManagedClientGroup({
+            clients : group.clients
+        });
+
+        group.rows = recalculated.rows;
+        group.columns = recalculated.columns;
+
+    };
+
+    /**
+     * Returns the unique ID representing the given ManagedClientGroup or set
+     * of client IDs. The ID of a ManagedClientGroup consists simply of the
+     * IDs of all its ManagedClients, separated by periods.
+     *
+     * @param {ManagedClientGroup|string[]} group
+     *     The ManagedClientGroup or array of client IDs to determine the
+     *     ManagedClientGroup ID of.
+     *
+     * @returns {string}
+     *     The unique ID representing the given ManagedClientGroup, or the
+     *     unique ID that would represent a ManagedClientGroup containing the
+     *     clients with the given IDs.
+     */
+    ManagedClientGroup.getIdentifier = function getIdentifier(group) {
+
+        if (!_.isArray(group))
+            group = _.map(group.clients, client => client.id);
+
+        return group.join('.');
+
+    };
+
+    /**
+     * Returns an array of client identifiers for all clients contained within
+     * the given ManagedClientGroup. Orcer of the identifiers is preserved

Review comment:
       Orcer?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org