Re: [pgAdmin4][Patch]: Allow user to choose background colour for server

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: Allow user to choose background colour for server
Date: 2017-11-16 13:04:18
Message-ID: CA+OCxozNayUyqDdCRAc9mz64oeB33O1JcN1Pe1xXqqdN+Jnu3g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

Looks good. A few changes/suggestions:

- There seem to be some debugger calls left in the code, e.g. in editors.js
- Instead of bgcolor and font_color, lets use bgcolor and fgcolor
(foreground).
- The docs are (technically) en_US, so we should use color not colour in
them.
- If the colours have been set for a server, I think we should also colour
the title bar in the query tool. The only possible problem there is that
the current default colours are reversed in comparison to the treeview
(e.g. the treeview defaults to black text, whilst the query tool title bar
is white on blue. Not sure if that is really an issue or not.

What do you think?

On Wed, Nov 15, 2017 at 12:15 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> Hi,
>
> Apologies but please find updated patch. Earlier I forgot to add copyright
> information in migration file.
>
> On Wed, Nov 15, 2017 at 4:58 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find updated patch.​
>>
>> On Wed, Nov 15, 2017 at 3:01 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Wed, Nov 15, 2017 at 9:26 AM, Ashesh Vashi <
>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> On Wed, Nov 15, 2017 at 2:28 PM, Murtuza Zabuawala <
>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> On Wed, Nov 15, 2017 at 1:20 PM, Ashesh Vashi <
>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Murtuza,
>>>>>>
>>>>>> On Wed, Nov 15, 2017 at 12:58 PM, Murtuza Zabuawala <
>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> PFA new patch.
>>>>>>>
>>>>>> If I have reviewed correctly, the current patch allows to select the
>>>>>> colour for background color for server, and its children.
>>>>>> I believe - we should allow to select text color too for better
>>>>>> readability.
>>>>>>
>>>>>> ​Yes Ashesh, After our discussion I tried that but with all the
>>>>> different colours in browser tree it looks cartoonish.
>>>>>
>>>> I am not sure - what looked like cartoonish to you (totally depend on
>>>> the color choices made by the users). :-)
>>>>
>>>> If I choose any dark color as background color, text in nodes won't be
>>>> clear to the user, and that will make the tree nodes completely unusable.
>>>> If we allow to choose the foreground/text color, those dark colors will
>>>> become usable by choosing suitable light color as text colors.
>>>>
>>>> I don't want to make any decision here, just asking for opinions from
>>>> others.
>>>>
>>>> So - Again ball is back in Dave's courtyard :-).
>>>>
>>>
>>> pgAdmin 3 didn't allow selection of the foreground colour, and I don't
>>> recall anyone ever asking for it. It's clearly up to the user to not select
>>> colours that won't work.
>>>
>>> I'm fine with just setting the background colour to satisfy this
>>> requirement, though if it's trivial to do, I have no objection to allowing
>>> changes to the foreground colour.
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-11-16 13:28:59 Re: Next release
Previous Message Murtuza Zabuawala 2017-11-16 12:52:42 [pgAdmin4][Patch]: To upgrade Selenium