You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "mike-jumper (via GitHub)" <gi...@apache.org> on 2023/01/30 18:17:34 UTC

[GitHub] [guacamole-client] mike-jumper opened a new pull request, #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

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

   This change replaces the global connection message list with a per-connection share indicator that appears when at least one user has joined the connection:
   
   ![User count showing 3 current users](https://user-images.githubusercontent.com/4632905/215555397-65eeb27f-b65d-4593-b797-8defcd57ea71.png)
   
   Notifications appear noting whether a user has joined or left. These notifications fade away automatically after around 5 seconds. Up to 3 of these notifications will appear at any given time:
   
   ![User join notification](https://user-images.githubusercontent.com/4632905/215556076-9d09e00a-6070-4b7a-8033-0dee42476806.png)
   
   The user can hover the mouse over the status indicator to see a list of all users currently on the connection, along with counts if that user has more than one copy of the connection open:
   
   ![Tooltip showing the current users of the current connection](https://user-images.githubusercontent.com/4632905/215556684-0c8e7946-5746-4623-b6dc-d1e1187977c1.png)
   
   If multiple connections are tiled within the same browser tab, this indicator is part of the connection title:
   
   ![Connection title containing number of current users](https://user-images.githubusercontent.com/4632905/215557649-2ee93ba9-9a0d-46f4-9e6c-1b96c6f45139.png)
   
   This change also adds `onjoin` and `onleave` handlers to `Guacamole.Client` so that developers need not implement their own message code handling within `onmsg`.


-- 
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 merged pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman merged PR #791:
URL: https://github.com/apache/guacamole-client/pull/791


-- 
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 #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#issuecomment-1409183387

   It looks like this is the case, but just to make sure, the referenced "global connection message list" was only used for connection join/leave notifications, correct?
   
   


-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091091043


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,24 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(parameters[1] /* User ID */, parameters[2] /* Name */);

Review Comment:
   Switched over to variables with a new commit.



-- 
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 #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#issuecomment-1413999158

   > I'm good with it. @jmuehlner anything else?
   
   No objections from me! Looks good.


-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1092593724


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,31 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var userID;
+            var username;
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(userID, username);
+                    break;
+
+                case Guacamole.Client.Message.USER_LEFT:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onleave)
+                        guac_client.onleave(userID, username);
+                    break;
+
+            }

Review Comment:
   Commit pushed: bd913274152fcc12a12e26955bb3acdaa71e52e8



-- 
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 a diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091111246


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,31 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var userID;
+            var username;
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(userID, username);
+                    break;
+
+                case Guacamole.Client.Message.USER_LEFT:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onleave)
+                        guac_client.onleave(userID, username);
+                    break;
+
+            }

Review Comment:
   The way this is implemented, this will fire _both_ `guac_client.onmsg()`, if it's defined, as well as `guac_client.onjoin()` or `guac_client.onleave()`, if those are defined. I just want to make sure that's the intended behavior and how you see that implementation working? I'm not sure if there are other instructions that are similarly implemented, or if this is a first of its kind?



-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091032889


##########
guacamole/src/main/frontend/src/app/client/types/ManagedClient.js:
##########
@@ -174,14 +175,25 @@ angular.module('client').factory('ManagedClient', ['$rootScope', '$injector',
          * @type ManagedFilesystem[]
          */
         this.filesystems = template.filesystems || [];
-        
+
         /**
-         * All messages that have been sent to the client that should be
-         * displayed.
-         * 
-         * @type ManagedClientMessage[]
+         * The current number of users sharing this connection, excluding the
+         * user that originally started the connection. Duplicate connections
+         * from the same user are included in this total.
+         */
+        this.userCount = template.userCount || 0;
+
+        /**
+         * All users currently sharing this connection, excluding the user that
+         * originally started the connection. If the connection is not shared,
+         * this object will be empty. This map consists of key/value pairs
+         * where each key is the user's username and each value is an object

Review Comment:
   Too bad we can't just use `Set`. I guess it's still a little too new for some browsers (looking at you IE and Edge < 12)



-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1092457976


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,31 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var userID;
+            var username;
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(userID, username);
+                    break;
+
+                case Guacamole.Client.Message.USER_LEFT:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onleave)
+                        guac_client.onleave(userID, username);
+                    break;
+
+            }

Review Comment:
   Ah, gotcha. Nope - definitely on purpose. I wanted to provide an optional, more convenient way of handling these messages that doesn't require as much low-level familiarity with the Guacamole protocol.



-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091005558


##########
guacamole/src/main/frontend/src/translations/en.json:
##########
@@ -128,10 +128,7 @@
 
         "INFO_CONNECTION_SHARED" : "This connection is now shared.",
         "INFO_NO_FILE_TRANSFERS" : "No file transfers.",
-
-        "MESSAGE_DEFAULT"      : "",
-        "MESSAGE_USER_JOINED"  : "User {ARGS_1} has joined the connection.",

Review Comment:
   Huh, these were just unused?



-- 
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 pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#issuecomment-1409263952

   > It looks like this is the case, but just to make sure, the referenced "global connection message list" was only used for connection join/leave notifications, correct?
   
   Yes, that's correct.


-- 
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 #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#issuecomment-1409287389

   I've always struggled with UX Design...looks much better... :smile: 


-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091006963


##########
guacamole/src/main/frontend/src/translations/en.json:
##########
@@ -128,10 +128,7 @@
 
         "INFO_CONNECTION_SHARED" : "This connection is now shared.",
         "INFO_NO_FILE_TRANSFERS" : "No file transfers.",
-
-        "MESSAGE_DEFAULT"      : "",
-        "MESSAGE_USER_JOINED"  : "User {ARGS_1} has joined the connection.",

Review Comment:
   Nope, but they are now. They were used by the message display directive that these changes replace.



-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091010377


##########
guacamole/src/main/frontend/src/translations/en.json:
##########
@@ -128,10 +128,7 @@
 
         "INFO_CONNECTION_SHARED" : "This connection is now shared.",
         "INFO_NO_FILE_TRANSFERS" : "No file transfers.",
-
-        "MESSAGE_DEFAULT"      : "",
-        "MESSAGE_USER_JOINED"  : "User {ARGS_1} has joined the connection.",

Review Comment:
   Ah, nevermind. I see where they were used now.



-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091549683


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,31 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var userID;
+            var username;
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(userID, username);
+                    break;
+
+                case Guacamole.Client.Message.USER_LEFT:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onleave)
+                        guac_client.onleave(userID, username);
+                    break;
+
+            }

Review Comment:
   What about firing `onmsg` first, and then `onjoin`/`onleave` if the default behavior isn't prevented by the `onmsg` handler?



-- 
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 a diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1092426743


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,31 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var userID;
+            var username;
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(userID, username);
+                    break;
+
+                case Guacamole.Client.Message.USER_LEFT:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onleave)
+                        guac_client.onleave(userID, username);
+                    break;
+
+            }

Review Comment:
   I'd be fine with that - or adding a `default:` to the `switch()` statement?
   
   I don't know that I have a strong preference one way or the other - mainly I just wanted to make sure that firing both the `onmsg()` handler and then the `onjoin()` or `onleave()` handler was the intentional and not any sort of an "oops, forgot to `return` the `onmsg()` call, etc.



-- 
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 a diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1092426743


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,31 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var userID;
+            var username;
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(userID, username);
+                    break;
+
+                case Guacamole.Client.Message.USER_LEFT:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onleave)
+                        guac_client.onleave(userID, username);
+                    break;
+
+            }

Review Comment:
   I'd be fine with that - or adding a `default:` to the `switch()` statement?
   
   I don't know that I have a strong preference one way or the other - mainly I just wanted to make sure that firing both the `onmsg()` handler and then the `onjoin()` or `onleave()` handler was the intentional and not any sort of an "oops, forgot to handle `return` the `onmsg()` call, etc.



-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1092458414


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,31 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var userID;
+            var username;
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(userID, username);
+                    break;
+
+                case Guacamole.Client.Message.USER_LEFT:
+                    userID = parameters[1];
+                    username = parameters[2];
+                    if (guac_client.onleave)
+                        guac_client.onleave(userID, username);
+                    break;
+
+            }

Review Comment:
   I'll push a commit with the revised behavior momentarily - `onmsg` first, followed by `onjoin`/`onleave` if default behavior is not prevented.



-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091016760


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,24 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(parameters[1] /* User ID */, parameters[2] /* Name */);

Review Comment:
   These comments are helpful to know what these parameters are, but why not just put the values in appropriately named variables? That seems more straightforward.



-- 
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 diff in pull request #791: GUACAMOLE-1293: Add list/count of current users joined to a connection.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #791:
URL: https://github.com/apache/guacamole-client/pull/791#discussion_r1091087945


##########
guacamole-common-js/src/main/webapp/modules/Client.js:
##########
@@ -1416,8 +1450,24 @@ Guacamole.Client = function(tunnel) {
         },
         
         "msg" : function(parameters) {
-            
-            if (guac_client.onmsg) guac_client.onmsg(parseInt(parameters[0]), parameters.slice(1));
+
+            var msgid = parseInt(parameters[0]);
+            if (guac_client.onmsg)
+                guac_client.onmsg(msgid, parameters.slice(1));
+
+            switch (msgid) {
+
+                case Guacamole.Client.Message.USER_JOINED:
+                    if (guac_client.onjoin)
+                        guac_client.onjoin(parameters[1] /* User ID */, parameters[2] /* Name */);

Review Comment:
   Sure.



-- 
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