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

Hi Dave,

PFA updated patch.

On Thu, Nov 16, 2017 at 6:34 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> Looks good. A few changes/suggestions:
>
> - There seem to be some debugger calls left in the code, e.g. in editors.js
>
​Fixed

> - Instead of bgcolor and font_color, lets use bgcolor and fgcolor
> (foreground).
>
​Fixed​

> - The docs are (technically) en_US, so we should use color not colour in
> them.
>
​Fixed​

> - 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?
>
​I have added logic to set the background & foreground colour in query tool
& datagrid title bar.
- Default title bar colours, b
ackground
​: blue
foreground
​: white
[
​w​
hat we have right now]
- When user has custom background colour for the server​,
b
ackground
​: <
custom background colour
>
foreground
​: black
- When user has custom background colour as well as foreground colour for
the server​,
b
ackground
​: <
custom background colour
>
foreground
​:
<
custom
​foreground
colour
>

>
>
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(dot)zabuawala(at)enterprisedb(dot)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
>