Re: [pgAdmin][RM5653]: High Contrast UI Theme

Thanks, patch applied.

On Mon, Jul 20, 2020 at 1:40 PM Nikhil Mohite <
nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:

> Hi Akshay/Team,
>
>
> I have fixed a few issues in high-contrast theme, PFA patch.
>
> Following issues are resolved in new patch:
>
> 1. Schema diff compare button icon is not visible on hover.
> 2. Schema diff for identical section text is not readable due to the
> same color as the background .
> 3. Removed extra variables used for set text color for explain -
> Analysis section (Issue related dark theme.).
> 4. Issue related to the disabled toggle button.
>
>
> Regards,
> Nikhil Mohite
>
> On Mon, Jul 20, 2020 at 12:10 PM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)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(dot)mohite(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Team,
>>>
>>> I have resolve review comments, PFA updated patch for RM-5653.
>>>
>>> I have also resolved the RM-#5677
>>> <https://redmine.postgresql.org/issues/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(dot)toshniwal(at)enterprisedb(dot)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(dot)mohite(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)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: 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.
>>>>>>
>>>>>
>>>> [image: Screenshot 2020-07-17 at 1.28.08 PM.png][image: Screenshot
>>>> 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: image.png]
>>>>
>>>>
>>>>>
>>>>>> 15) One issue, when hovered inside a subnode, the text disappears.
>>>>>>
>>>>> [image: image.png]
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Fri, Jul 17, 2020 at 1:36 PM Akshay Joshi <
>>>>>> akshay(dot)joshi(at)enterprisedb(dot)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
>>>>>>>
>>>>>>> [image: Screenshot 2020-07-17 at 1.29.53 PM.png]
>>>>>>>
>>>>>>> On Fri, Jul 17, 2020 at 1:06 PM Akshay Joshi <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)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
>>>>>>>>
>>>>>>>> [image: Dialog Border Color.png]
>>>>>>>>
>>>>>>>> @Aradhana Birewar <aradhana(dot)birewar(at)enterprisedb(dot)com> 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
>>>>>>>>
>>>>>>>> [image: 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(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Aditya, can you please do the code review.
>>>>>>>>>
>>>>>>>>> On Thu, Jul 16, 2020 at 6:54 PM Nikhil Mohite <
>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Team,
>>>>>>>>>>
>>>>>>>>>> I have completed changes for the support of the new
>>>>>>>>>> High-Contrast UI Theme RM-#5653
>>>>>>>>>> <https://redmine.postgresql.org/issues/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*
>>>>>> <http://edbpostgres.com>
>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>
>>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Aditya Toshniwal
>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>> <http://edbpostgres.com>
>>>> "Don't Complain about Heat, Plant a TREE"
>>>>
>>>
>>
>> --
>> *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*