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 | CAOBg0ANZ2HZeBDY+gzEk7R10C7Hg5GcF-epK2=r-uPVLNZ44bQ@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
|
List | pgadmin-hackers |
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-icon2) 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.
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.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.15) One issue, when hovered inside a subnode, the text disappears.
On Fri, Jul 17, 2020 at 1:36 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi NikhilFound 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
On Fri, Jul 17, 2020 at 1:06 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi NikhilI 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
@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 screenshotAditya 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 & RegardsAkshay JoshiSr. Software ArchitectEnterpriseDB Software India Private LimitedMobile: +91 976-788-8246--Thanks & RegardsAkshay JoshiSr. Software ArchitectEnterpriseDB Software India Private LimitedMobile: +91 976-788-8246--Thanks & RegardsAkshay JoshiSr. Software ArchitectEnterpriseDB Software India Private LimitedMobile: +91 976-788-8246--Regards,Aditya ToshniwalpgAdmin hacker | Sr. Software Engineer | edbpostgres.com"Don't Complain about Heat, Plant a TREE"
Attachment
pgadmin-hackers by date: