Re: [pgAdmin][RM5653]: High Contrast UI Theme - Mailing list pgadmin-hackers

From Nikhil Mohite
Subject Re: [pgAdmin][RM5653]: High Contrast UI Theme
Date
Msg-id CAOBg0AO+7azV1gAus8=fGhv5zQV=G1=tJ=7krrLvphU8sM3Z8Q@mail.gmail.com
Whole thread Raw
In response to Re: [pgAdmin][RM5653]: High Contrast UI Theme  (Aditya Toshniwal <aditya.toshniwal@enterprisedb.com>)
Responses Re: [pgAdmin][RM5653]: High Contrast UI Theme  (Akshay Joshi <akshay.joshi@enterprisedb.com>)
List pgadmin-hackers
Hi Team,

I have resolve review comments, PFA updated patch for RM-5653.

I have also resolved the RM-#5677 issue in a dark theme and included changes in the same patch.


Regards,
Nikhil Mohite.

On Fri, Jul 17, 2020 at 3:15 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Nikhil,

Please find the inline images along with the review.

On Fri, Jul 17, 2020 at 3:01 PM Nikhil Mohite <nikhil.mohite@enterprisedb.com> wrote:
Hi Aditya,

I am facing some issue to load the images for point 12, 13, 14 and 15, can you please share them once again.


Regards,
Nikhil Mohite.

On Fri, Jul 17, 2020 at 1:52 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Nikhil,

I agree with Akshay's input. Apart from that, below are my suggestions:
1) Rename btn-primary-icon to something else are btn-icononly or just btn-icon. It has no relation with primary.
 btn-primary-icon
2) Below change is not required.

Change not required;

-$body-color: $color-fg;

+$body-color: $color-fg !default;


3) Below variables should be moved along with other btn variables defined. Rename btn-ternary-disabled-border-bg to btn-ternary-disabled-border-color.

+$btn-ternary-disabled-bg: $color-ternary !default;

+$btn-ternary-disabled-fg: $color-bg !default;

+$btn-ternary-disabled-border-bg: $btn-ternary-disabled-bg !default;


4) Do not use a bg color variable to a fg color for the sake of convention. Declare a new local variable if required and assign the hash value to the new variable.

+$color-success-disabled-fg: $color-bg !default;

+$color-success-hover-fg: $color-bg !default;


5) The variable name is *-bg but used for color. Rename it to -color or -fg.

   & .select2-selection__choice__remove {

-    color: $dropdown-link-hover-bg;

+    color: $dropdown-link-remove-bg;

     margin-right: 0.25rem;


6) Rename *-border-bg to *-border-color

+$card-header-border-bg: $border-color !default;

+$btn-primary-icon-border-bg: $color-gray !default;


7) Too many blank lines:

+.alert-text-body {

+  color: $alert-color-fg;

+}

+

+

+


8) Variable name says - maximize-bg but is used for -pin, -maximize, -close. Rename it to be generalized.

   border: $input-btn-border-width solid $btn-secondary-border !important;

-  background-color: $color-secondary !important;

+  background-color: $alert-maximize-bg !important;

   font-size: 12px;

   border-radius: $btn-border-radius;

   position: relative;

@@ -157,7 +157,7 @@

 }

 

 .ajs-pin:hover, .ajs-maximize:hover, .ajs-close:hover {

-  background-color: $btn-secondary-hover-bg !important;

+  background-color: $alert-maximize-hover-bg !important;

 }


9) Replacing component-active-bg with color primary has no effect as it is marked default and you've overridden the variable for the new theme.

-$input-focus-border-color: lighten($component-active-bg, 25%);

+$input-focus-border-color: lighten($color-primary, 25%) !default;

 10) Should be $select2-*

+$select-container-hover-fg: $tree-fg-hover !default;
11) Do not change this variable directly. Change the shadow-base-color instead.

-$dialog-box-shadow: 0 0.5rem 3rem $shadow-base-color;

+$dialog-box-shadow: 0 0.5rem 3rem $shadow-base-color !default;

Same for input focus:

-$input-btn-focus-box-shadow: 0 0 0 $input-btn-focus-width $input-btn-focus-color;

+$input-btn-focus-box-shadow: 0 0 0 $input-btn-focus-width $input-btn-focus-color !default;

Plus, I can see the shadow is completely removed for the dialogs. This makes it difficult to identify the dialog in the backdrop. And focus shadow is also removed, which I think makes it difficult to see which textbox is focussed.

 
image.png

12) When I hovered on the close button on the above dialog, it does not change any background and remains as is, which I think is not a good UI feedback.

13) When hovered over items in the list the icon is not visible. The hover bg color is also different in list mode and grid mode which should be not.

 
Screenshot 2020-07-17 at 1.28.08 PM.pngScreenshot 2020-07-17 at 1.27.38 PM.png


14) Please check the grid lines of graphs once as they can be customized with the theme now. Although don't spend much time on it as they are going to be replaced with a new lib.
image.png
 

15) One issue, when hovered inside a subnode, the text disappears.
image.png
 


 


On Fri, Jul 17, 2020 at 1:36 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Nikhil

Found two more issues:
  • Focus is not visible when we hover on the close button of any dialog.
  • Schema Diff tool fixes refer the below screenshot
            Screenshot 2020-07-17 at 1.29.53 PM.png

On Fri, Jul 17, 2020 at 1:06 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Nikhil

I have applied the patch and run the pgAdmin in High Contrast Theme. Following issues, I have found 
  • Add (Beta) suffix in "High Contrast" in the preferences dialog. 
  • Update the documentation for High Contrast Theme in Preferences.
  • Three different colors used for dialog header (Properties, Grant Wizard, Maintenance). Refer the below screenshot 
                            Dialog Border Color.png

@Aradhana Birewar Can we use the same light blue color for the properties, grant wizard, and maintenance dialog?
  • In the Query History tab, SQL is not readable. Refer the below screenshot          
                           Query_History.png

Aditya is doing the code review so wait for his review comments and then send the combined patch.


On Fri, Jul 17, 2020 at 12:25 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Aditya, can you please do the code review.

On Thu, Jul 16, 2020 at 6:54 PM Nikhil Mohite <nikhil.mohite@enterprisedb.com> wrote:
Hi Team,

I have completed changes for the support of the new High-Contrast  UI Theme RM-#5653.

PFA patch.


Regards,
Nikhil Mohite.



--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


--
Thanks & Regards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


--
Regards,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"


--
Regards,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"
Attachment

pgadmin-hackers by date:

Previous
From: Yogesh Mahajan
Date:
Subject: [pgAdmin] : Patch fixing Feature Test failures.
Next
From: Akshay Joshi
Date:
Subject: pgAdmin 4 commit: Update French translation.