jmuehlner commented on code in PR #908:
URL: https://github.com/apache/guacamole-client/pull/908#discussion_r1310887853
##########
guacamole-common-js/src/main/webapp/modules/KeyEventInterpreter.js:
##########
@@ -424,6 +452,29 @@ Guacamole.KeyEventInterpreter = function
KeyEventInterpreter(batchSeperation, st
var keyText;
var typed;
+ // If this key should be both printed and rendered
+ if (keyDefinition.value != null &&
keyDefinition.printAndRender) {
+
+ // The printed name of the key
+ var printedValue = getPrintedKeyName(keyDefinition);
+
+ // The directly rendered value
+ var renderedValue = keyDefinition.value;
+
+ // Display the name first, followed by the value
+ currentBatch.simpleValue += printedValue;
+ currentBatch.simpleValue += renderedValue;
+
+ // Add pair of key events in the same order, with the name
+ // displayed as non-typed to ensure proper styling
+ currentBatch.events.push(new
Guacamole.KeyEventInterpreter.KeyEvent(
+ printedValue, false, relativeTimestap));
+ currentBatch.events.push(new
Guacamole.KeyEventInterpreter.KeyEvent(
+ renderedValue, true, relativeTimestap));
Review Comment:
So currently the `KeyEventIntepreter` is documented to "produce human
readable text batches".
It sounds like you're advocating removing all of the rendering logic out of
this service (i.e. everything that produces the `text` or `simpleValue` fields,
modifier/shortcut tracking, probably even batch separation). This would be
probably all move to a service in the webapp.
I think that separation of concerns makes a lot of sense, though I was
unsure how much heavy handed I should be in changing the way the interpreter
works.
##########
guacamole-common-js/src/main/webapp/modules/KeyEventInterpreter.js:
##########
@@ -424,6 +452,29 @@ Guacamole.KeyEventInterpreter = function
KeyEventInterpreter(batchSeperation, st
var keyText;
var typed;
+ // If this key should be both printed and rendered
+ if (keyDefinition.value != null &&
keyDefinition.printAndRender) {
+
+ // The printed name of the key
+ var printedValue = getPrintedKeyName(keyDefinition);
+
+ // The directly rendered value
+ var renderedValue = keyDefinition.value;
+
+ // Display the name first, followed by the value
+ currentBatch.simpleValue += printedValue;
+ currentBatch.simpleValue += renderedValue;
+
+ // Add pair of key events in the same order, with the name
+ // displayed as non-typed to ensure proper styling
+ currentBatch.events.push(new
Guacamole.KeyEventInterpreter.KeyEvent(
+ printedValue, false, relativeTimestap));
+ currentBatch.events.push(new
Guacamole.KeyEventInterpreter.KeyEvent(
+ renderedValue, true, relativeTimestap));
Review Comment:
So currently the `KeyEventIntepreter` is documented to "produce human
readable text batches".
It sounds like you're advocating removing all of the rendering logic out of
this service (i.e. everything that produces the `text` or `simpleValue` fields,
modifier/shortcut tracking, probably even batch separation). This would be
probably all move to a service in the webapp.
I think that separation of concerns makes a lot of sense, though I was
unsure how heavy handed I should be in changing the way the interpreter works.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
