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

[GitHub] [syncope] DimaAy opened a new pull request #241: [SYNCOPE-1435] CSS modification for Console

DimaAy opened a new pull request #241:
URL: https://github.com/apache/syncope/pull/241


   


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

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



[GitHub] [syncope] mat-ale commented on a change in pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
mat-ale commented on a change in pull request #241:
URL: https://github.com/apache/syncope/pull/241#discussion_r566727032



##########
File path: client/idm/console/src/main/resources/org/apache/syncope/client/console/wizards/resources/AbstractConnConfPanel.html
##########
@@ -21,7 +21,7 @@
     <div id="emptyPlaceholder"/>
     <span wicket:id="connectorPropertiesContainer">
       <div class="pull-right">
-        <a style="position: fixed; top: 65px; right:60px;" wicket:id="check"  href="#">
+        <a style="position: fixed; top: 95px; right:420px;" wicket:id="check"  href="#">

Review comment:
       after a deeper check this solution is not good, since the `Override?` flag bothers the check icon.
   A totally better solution is to edit that html file as following:
   ```html
   <html xmlns="http://www.w3.org/1999/xhtml" xmlns:wicket="http://wicket.apache.org">
     <wicket:panel>
       <div id="emptyPlaceholder"/>
       <span wicket:id="connectorPropertiesContainer">
         <a class="conn-check-wrapper" wicket:id="check"  href="#">
           <i class="fa fa-heartbeat fa-2x"></i>
         </a>
   <!-- ... -->
   ```
   
   and the `syncopeUi.scss` file needs to include:
   ```scss
   .scrollable-tab-content {
     padding-top: 30px;
     /* other already existing rules */
   }
   .conn-check-wrapper {
     position: absolute;
     top: 0;
     right: 25px;
     z-index: 1;
   }
   ```




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

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



[GitHub] [syncope] ilgrosso commented on a change in pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
ilgrosso commented on a change in pull request #241:
URL: https://github.com/apache/syncope/pull/241#discussion_r566138845



##########
File path: client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/DomainWizardBuilder.java
##########
@@ -103,7 +103,7 @@ public Storage(final Domain domain) {
             add(new AjaxTextFieldPanel(
                     "jdbcURL",
                     "jdbcURL",
-                    new PropertyModel<>(domain, "jdbcURL")).setRequired(true));
+                    new PropertyModel<>(domain, "jdbcURL")).setRequired(true).addRequiredLabel());

Review comment:
       for all occurrences of course

##########
File path: client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/DomainWizardBuilder.java
##########
@@ -103,7 +103,7 @@ public Storage(final Domain domain) {
             add(new AjaxTextFieldPanel(
                     "jdbcURL",
                     "jdbcURL",
-                    new PropertyModel<>(domain, "jdbcURL")).setRequired(true));
+                    new PropertyModel<>(domain, "jdbcURL")).setRequired(true).addRequiredLabel());

Review comment:
       ```.setRequired(true).addRequiredLabel()``` should be replaced by ```.addRequiredLabel()``` as the latter's implementation contains the former call




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

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



[GitHub] [syncope] mat-ale commented on a change in pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
mat-ale commented on a change in pull request #241:
URL: https://github.com/apache/syncope/pull/241#discussion_r566253804



##########
File path: client/idrepo/common-ui/src/main/resources/META-INF/resources/ui-commons/css/syncopeUI.scss
##########
@@ -665,6 +702,36 @@ div.listview-actions a {
   padding: 0px 4px 0px 4px;
 }
 
+
+.attribute.right {
+  float: right;
+  width: 50%;
+}
+
+.attribute.left {
+  width: 50%;
+  float: left;
+}
+
+.left.page-header{
+  padding-bottom: 9px;
+  border-bottom: 1px solid #dee2e6;
+  width: 50%;
+  float: left;
+}
+
+.right.page-header{
+  padding-bottom: 9px;
+  border-bottom: 1px solid #dee2e6;
+  width: 50%;
+  float: right;
+}

Review comment:
       Selectors with the same content could be appended with `,` e.g.:
   ```css
   .left.page-header,
   .right.page-header {
     padding-bottom: 9px;
     border-bottom: 1px solid #dee2e6;
     width: 50%;
   }
   
   .left.page-header{
     float: left;
   }
   .right.page-header{
     float: right;
   }
   ```




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

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



[GitHub] [syncope] mat-ale commented on a change in pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
mat-ale commented on a change in pull request #241:
URL: https://github.com/apache/syncope/pull/241#discussion_r566236747



##########
File path: client/idrepo/common-ui/src/main/resources/META-INF/resources/ui-commons/css/search.scss
##########
@@ -36,25 +35,44 @@
     float: left;
     padding: 0 3px 0px 0px;
     display: inline-block !important;
+    .form-check{
+      margin-bottom: -50px;
+    }
+    .toggle.btn-xs {

Review comment:
       as an improvement, for this kind of rules the related `.scss` style is:
   
   ```scss
   .toggle {
     &.btn-xs {/*...*/}
   }
   ```
   and for this specific case I would do:
   ```scss
   .toggle {
     &.btn {/*...*/}
   }
   ```
   instead, in order to avoid overriding a rule related to the default css grid classes (`xs`, `md` 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.

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



[GitHub] [syncope] mat-ale commented on a change in pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
mat-ale commented on a change in pull request #241:
URL: https://github.com/apache/syncope/pull/241#discussion_r566218719



##########
File path: client/idm/console/src/main/resources/org/apache/syncope/client/console/wizards/resources/AbstractConnConfPanel.html
##########
@@ -21,7 +21,7 @@
     <div id="emptyPlaceholder"/>
     <span wicket:id="connectorPropertiesContainer">
       <div class="pull-right">
-        <a style="position: fixed; top: 65px; right:60px;" wicket:id="check"  href="#">
+        <a style="position: fixed; top: 95px; right:420px;" wicket:id="check"  href="#">

Review comment:
       Here it would be better to use `position: absolute; top: 0; right: 20px;`, because the rule with `fixed` seems to be not responsive when changing device or just resizing the window.




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

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



[GitHub] [syncope] DimaAy commented on pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
DimaAy commented on pull request #241:
URL: https://github.com/apache/syncope/pull/241#issuecomment-769747335


   Thank you @ilgrosso and @mat-ale 


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

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



[GitHub] [syncope] DimaAy merged pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
DimaAy merged pull request #241:
URL: https://github.com/apache/syncope/pull/241


   


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

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



[GitHub] [syncope] mat-ale commented on a change in pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
mat-ale commented on a change in pull request #241:
URL: https://github.com/apache/syncope/pull/241#discussion_r566242181



##########
File path: client/idrepo/common-ui/src/main/resources/META-INF/resources/ui-commons/css/syncopeUI.scss
##########
@@ -382,6 +399,16 @@ section#notifications .modal-lg {
   display: block;
 }
 
+.details-footer {

Review comment:
       I think that:
   ```css
   .details-footer {
      margin: 30px 0px 0px 0px;
      border: 1px solid #EEE;
      color: #888;
   }
   ```
   should be enough 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.

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



[GitHub] [syncope] mat-ale commented on pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
mat-ale commented on pull request #241:
URL: https://github.com/apache/syncope/pull/241#issuecomment-769746493


   LGTM too, good job!


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

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



[GitHub] [syncope] mat-ale commented on a change in pull request #241: [SYNCOPE-1435] CSS modification for Console

Posted by GitBox <gi...@apache.org>.
mat-ale commented on a change in pull request #241:
URL: https://github.com/apache/syncope/pull/241#discussion_r566226525



##########
File path: client/idm/console/src/main/resources/org/apache/syncope/client/console/wizards/resources/AbstractConnConfPanel.html
##########
@@ -21,7 +21,7 @@
     <div id="emptyPlaceholder"/>
     <span wicket:id="connectorPropertiesContainer">
       <div class="pull-right">
-        <a style="position: fixed; top: 65px; right:60px;" wicket:id="check"  href="#">
+        <a style="position: fixed; top: 95px; right:420px;" wicket:id="check"  href="#">

Review comment:
       ...and a `z-index: 1` would be good to have in order not to hide the icon behind other 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.

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