jmuehlner commented on code in PR #796:
URL: https://github.com/apache/guacamole-client/pull/796#discussion_r1119289963
##########
guacamole/src/main/frontend/src/app/clipboard/directives/guacClipboard.js:
##########
@@ -49,11 +49,29 @@ angular.module('clipboard').directive('guacClipboard',
['$injector',
/**
* The DOM element which will contain the clipboard contents within the
- * user interface provided by this directive.
+ * user interface provided by this directive. We populate the clipboard
+ * editor via this DOM element rather than updating a model so that we
+ * are prepared for future support of rich text contents.
*
* @type Element
*/
- var element = $element[0];
+ var element = $element[0].querySelector('.clipboard');
Review Comment:
Hmm, now that I look at this, it makes me a little uncomfortable. This is
just selecting the first element with the `clipboard` class, but now that
there's 2 of those, it's only the ordering in the template that makes this work
- if somebody came in later and changed the order around, clipboard sync would
just stop working properly and it might not be obvious why.
How about updating this to specifically only match the active clipboard?
Either by modifying this selector to match only elements that have the
`clipboard` class, but _not_ the `inactive` class, or (probably better) by
adding an `active` class to the active clipboard and querying based on that?
--
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]
