Re: [pgAdmin][RM5653]: High Contrast UI Theme - Mailing list pgadmin-hackers
Hi Akshay/Team,I have fixed a few issues in high-contrast theme, PFA patch.Following issues are resolved in new patch:
- Schema diff compare button icon is not visible on hover.
- Schema diff for identical section text is not readable due to the same color as the background .
- Removed extra variables used for set text color for explain - Analysis section (Issue related dark theme.).
- Issue related to the disabled toggle button.
Regards,Nikhil MohiteOn Mon, Jul 20, 2020 at 12:10 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Thanks, patch applied.Please send the separate patch for few small changes that we discussed on Chat.On Mon, Jul 20, 2020 at 8:38 AM Nikhil Mohite <nikhil.mohite@enterprisedb.com> wrote: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-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?
Attachment
pgadmin-hackers by date: