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 2022/01/23 07:19:19 UTC

[GitHub] [guacamole-client] mike-jumper opened a new pull request #690: GUACAMOLE-1509: Add contextual CSS classes to reduce template ambiguity.

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


   This change adds several CSS classes for the sole purpose of providing convenient hooks for theming/branding, including automatically-generated classes from field and form names, as well as straightforward use of the `name` attribute where appropriate for input fields.


-- 
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 #690: GUACAMOLE-1509: Add contextual CSS classes to reduce template ambiguity.

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



##########
File path: guacamole/src/main/frontend/src/app/form/templates/usernameField.html
##########
@@ -0,0 +1,15 @@
+<div class="username-field">
+    <input type="text"
+           ng-attr-id="{{ fieldId }}"
+           ng-attr-list="{{ dataListId }}"
+           ng-attr-name="{{ field.name }}"
+           ng-model="model"
+           ng-disabled="disabled"
+           guac-focus="focused"
+           autocorrect="off"
+           autocapitalize="off">
+    <datalist ng-if="dataListId" ng-attr-id="{{ dataListId }}">
+        <option ng-repeat="option in field.options | orderBy: option"
+                value="{{ option }}">{{ getFieldOption(option) | translate }}</option>
+    </datalist>

Review comment:
       That's an optional set of valid values that the browser may provide as specific choices in addition to free-form entry. This comes from the original `TEXT` field template, which provides the same functionality:
   
   https://github.com/apache/guacamole-client/blob/c079b31dd5c699431b7fb13287dcaad92a29e7fd/guacamole/src/main/frontend/src/app/form/templates/textField.html#L10-L13
   
   The reference to the relevant controller is apparently missing from the field definition, though, so this incorrectly will not have any effect. See:
   
   https://github.com/apache/guacamole-client/blob/11c1575daf58111049f4c5cc1cd6ea1b01d5ee9c/guacamole/src/main/frontend/src/app/form/services/formService.js#L46-L50
   
   vs:
   
   https://github.com/apache/guacamole-client/blob/11c1575daf58111049f4c5cc1cd6ea1b01d5ee9c/guacamole/src/main/frontend/src/app/form/services/formService.js#L93-L95
   
   I'll add 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: 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 #690: GUACAMOLE-1509: Add contextual CSS classes to reduce template ambiguity.

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


   


-- 
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 #690: GUACAMOLE-1509: Add contextual CSS classes to reduce template ambiguity.

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



##########
File path: guacamole/src/main/frontend/src/app/form/templates/usernameField.html
##########
@@ -0,0 +1,15 @@
+<div class="username-field">
+    <input type="text"
+           ng-attr-id="{{ fieldId }}"
+           ng-attr-list="{{ dataListId }}"
+           ng-attr-name="{{ field.name }}"
+           ng-model="model"
+           ng-disabled="disabled"
+           guac-focus="focused"
+           autocorrect="off"
+           autocapitalize="off">
+    <datalist ng-if="dataListId" ng-attr-id="{{ dataListId }}">
+        <option ng-repeat="option in field.options | orderBy: option"
+                value="{{ option }}">{{ getFieldOption(option) | translate }}</option>
+    </datalist>

Review comment:
       Fixed 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] necouchman commented on a change in pull request #690: GUACAMOLE-1509: Add contextual CSS classes to reduce template ambiguity.

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



##########
File path: guacamole/src/main/frontend/src/app/form/templates/usernameField.html
##########
@@ -0,0 +1,15 @@
+<div class="username-field">
+    <input type="text"
+           ng-attr-id="{{ fieldId }}"
+           ng-attr-list="{{ dataListId }}"
+           ng-attr-name="{{ field.name }}"
+           ng-model="model"
+           ng-disabled="disabled"
+           guac-focus="focused"
+           autocorrect="off"
+           autocapitalize="off">
+    <datalist ng-if="dataListId" ng-attr-id="{{ dataListId }}">
+        <option ng-repeat="option in field.options | orderBy: option"
+                value="{{ option }}">{{ getFieldOption(option) | translate }}</option>
+    </datalist>

Review comment:
       What's 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 #690: GUACAMOLE-1509: Add contextual CSS classes to reduce template ambiguity.

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



##########
File path: guacamole/src/main/frontend/src/app/form/templates/usernameField.html
##########
@@ -0,0 +1,15 @@
+<div class="username-field">
+    <input type="text"
+           ng-attr-id="{{ fieldId }}"
+           ng-attr-list="{{ dataListId }}"
+           ng-attr-name="{{ field.name }}"
+           ng-model="model"
+           ng-disabled="disabled"
+           guac-focus="focused"
+           autocorrect="off"
+           autocapitalize="off">
+    <datalist ng-if="dataListId" ng-attr-id="{{ dataListId }}">
+        <option ng-repeat="option in field.options | orderBy: option"
+                value="{{ option }}">{{ getFieldOption(option) | translate }}</option>
+    </datalist>

Review comment:
       That's an optional set of valid values that the browser may provide as specific choices in addition to free-form entry. This comes from the original `TEXT` field template, which provides the same functionality:
   
   https://github.com/apache/guacamole-client/blob/c079b31dd5c699431b7fb13287dcaad92a29e7fd/guacamole/src/main/frontend/src/app/form/templates/textField.html#L10-L13
   
   The reference to the relevant controller is apparently missing from the field definition, though, so this will not any effect. See:
   
   https://github.com/apache/guacamole-client/blob/11c1575daf58111049f4c5cc1cd6ea1b01d5ee9c/guacamole/src/main/frontend/src/app/form/services/formService.js#L46-L50
   
   vs:
   
   https://github.com/apache/guacamole-client/blob/11c1575daf58111049f4c5cc1cd6ea1b01d5ee9c/guacamole/src/main/frontend/src/app/form/services/formService.js#L93-L95
   
   I'll recheck whether any other fields have unnecessary data lists and correct 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