Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iUBM6-0000t4-C2 for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 15:11:50 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1iUBM4-000481-He for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 15:11:48 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iUBM3-00047t-U4 for pgadmin-hackers@lists.postgresql.org; Mon, 11 Nov 2019 15:11:48 +0000 Received: from mail-oi1-x242.google.com ([2607:f8b0:4864:20::242]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1iUBM0-0002IO-2e for pgadmin-hackers@postgresql.org; Mon, 11 Nov 2019 15:11:46 +0000 Received: by mail-oi1-x242.google.com with SMTP id j7so11779143oib.3 for ; Mon, 11 Nov 2019 07:11:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=0eXOfO5bqZovf+9o1GNalPJcDpSvgC5jRo+NC/vAaow=; b=Dp/N/v6ylnieNqHqAFKlVbMBK2rlfMk02tlephxmn3Pc/Q11eS1PpMU+K2rKRHyx6Y cqf4tOkeqnTQ8g8CkyigOIcFSnzZfW42RnSZJdS8xHZXmcQ5Ns7Gl/zu7YAY1LirVRl4 vbY8hSRceszTc6q7kxBiAtWSXLIJl5fEp9ZPhFCz9R5UPl5l9ykcPSoM6ccYpUjw790P gCN0skingm2cOHirC4S7ezSVOWR67f4vc0pzfFowYhkehFevB37pUuyQ+YsfUBMzKC+k H214h1NhEIs+Z30zsPYAg7y+AMEhrRl/Xi0fBD0XI0P7qsjHWqQ3ImQor1gw3PtpNldZ Ij0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0eXOfO5bqZovf+9o1GNalPJcDpSvgC5jRo+NC/vAaow=; b=rEt9Bk3G3EHHknMsf2Kr72IdQHMbY5XAf2MwkFYvVoJAwxsBXF3Wf05SG8omRB4blb BlSXo0wip6U+eRRW2rA3l6WG85/zpT+9I2fIpXskrNdV5zrCkz7+ITLI1L1MNrmS+KH+ 0kBf43V3d/scN9YysFaH7xLCRfQQPWDJtOq9vFF68IP39szGX5aelHPeaUIuWQR6pdRP S8eDbROaecKWBLkhTeY01GkazuU/okP+axJUU/ztFqtGwETi/NKVWLNJ8Hwd9IVWQ/QZ Nzv2R8fEFIQp1gZJkayTnx4ceE8ypLHQE65KBtTHYrC4bKs/h3BE7mfkT/cuVZhgckKA gd1w== X-Gm-Message-State: APjAAAUPAurRToWNKbcja5u6tGE9gdPiLME732HEnRgxbJ42fwP2KN1A 5sRXZZvvVlFI0G1mHG60/AF3beCVWD2oIeIsW+ttKCCn12gCoitPGnXF8V7tNKDr92qxmH5Ef/Y 4LY7RsMPqHIqM1eHCpIJBOIb3Jr8jxM7whKfblTJjCfFdlV4JfIx+hGcP7cgo+Ck3zRr/4/smsO elh/2zdl1nFi95z1FrGa0f043i403N18ZNepjJEteHlXufzrKGJ+Q= X-Google-Smtp-Source: APXvYqzEZfp/mmRufgEYkOWisdhHStQNT3vy8qqFCjIoiOwryljTO1q+y+5nPSFSLqGX1d58ClFbKoUBHXQp3aY0RBQ= X-Received: by 2002:aca:4dcc:: with SMTP id a195mr24056864oib.172.1573485103150; Mon, 11 Nov 2019 07:11:43 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Aditya Toshniwal Date: Mon, 11 Nov 2019 20:41:29 +0530 Message-ID: Subject: Re: [pgAdmin][RM4348] Theme options in pgAdmin and dark theme To: Dave Page Cc: Akshay Joshi , pgadmin-hackers , Ashesh Vashi Content-Type: multipart/alternative; boundary="000000000000a2122c0597138c92" X-CLOUD-SEC-AV-Info: edb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000a2122c0597138c92 Content-Type: text/plain; charset="UTF-8" Hi Dave, On Mon, Nov 11, 2019, 20:33 Dave Page wrote: > Hi > > That's looking a lot better. I see a couple of issues (screenshots > attached): > > - The combo box on the query tool toolbar isn't dark. > I'm not sure why you are not able to see it but I've changed it. Try a python restart. > - The tagline on the dashboard logo isn't readable. > Yes because that's an SVG image. We may need to make it completely from HTML. > > A couple of other thoughts: > > - The guide lines on the graphs are still too bright. Let's make them the > same colour as the graph boarders please. > There is technical limitation here. To change those grid lines based on theme we need to import variables in JS. I didn't find a way to dynamically load SASS variables based on theme. So I chose color which would work for both dark and black backgrounds and is hard coded in JS. > > - The header bar foreground colour and the dividing white line below it > seem quite harsh. Are they pure white? They should be softened a little bit. > I'll check the color, but AFAIR it's not pure white. > > Thanks! > > On Mon, Nov 11, 2019 at 1:22 PM Akshay Joshi < > akshay.joshi@enterprisedb.com> wrote: > >> Thanks, patch applied. >> >> As per discussion with Aditya, we have removed customized scroll bars for >> the time being as they are not clearly visible with some of the components. >> >> On Mon, Nov 11, 2019 at 5:25 PM Aditya Toshniwal < >> aditya.toshniwal@enterprisedb.com> wrote: >> >>> Hi Hackers, >>> >>> Attached is the updated patch. >>> Kindly review. >>> >>> On Mon, Nov 11, 2019 at 3:42 PM Aditya Toshniwal < >>> aditya.toshniwal@enterprisedb.com> wrote: >>> >>>> Hi, >>>> >>>> Kindly hold on with the patch. Few more changes required per review by @Ashesh >>>> Vashi . >>>> >>>> On Mon, Nov 11, 2019 at 3:07 PM Aditya Toshniwal < >>>> aditya.toshniwal@enterprisedb.com> wrote: >>>> >>>>> Hi Hackers, >>>>> >>>>> Attached is the patch for further improvements in the Dark theme >>>>> colors. >>>>> Gray shades and other colors are changed to identify different >>>>> components more clearly. Few of the controls were missing the privileges of >>>>> dark theme, fixed that. >>>>> Few dashboard graph related changes. >>>>> As suggested, theme related code changes is removed from config.py and >>>>> moved to miscellaneous under a new package - Themes. Thank you @Ashesh >>>>> Vashi for inputs on that. >>>>> >>>>> Kindly review. >>>>> >>>>> On Mon, Nov 11, 2019 at 3:00 PM Aditya Toshniwal < >>>>> aditya.toshniwal@enterprisedb.com> wrote: >>>>> >>>>>> Hi Dave, >>>>>> >>>>>> On Mon, Nov 11, 2019 at 2:38 PM Dave Page wrote: >>>>>> >>>>>>> Hi >>>>>>> >>>>>>> On Mon, Nov 11, 2019 at 7:01 AM Aditya Toshniwal < >>>>>>> aditya.toshniwal@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Hi Dave, >>>>>>>> >>>>>>>> On Thu, Nov 7, 2019 at 7:56 PM Dave Page wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Thu, Nov 7, 2019 at 2:18 PM Dave Page >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Thu, Nov 7, 2019 at 1:25 PM Akshay Joshi < >>>>>>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> Thanks, patch applied. >>>>>>>>>>> >>>>>>>>>>> On Thu, Nov 7, 2019 at 6:39 PM Aditya Toshniwal < >>>>>>>>>>> aditya.toshniwal@enterprisedb.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Hackers, >>>>>>>>>>>> >>>>>>>>>>>> Attached is the updated patch with few more changes and >>>>>>>>>>>> corrections. >>>>>>>>>>>> Kindly review. >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> I've committed a couple of minor tweaks - one to remove a space, >>>>>>>>>> e.g. >>>>>>>>>> >>>>>>>>>> gettext('A page refresh is required to apply the theme. Do you wish to refresh the page now ?'), >>>>>>>>>> >>>>>>>>>> is now: >>>>>>>>>> >>>>>>>>>> gettext('A page refresh is required to apply the theme. Do you wish to refresh the page now?'), >>>>>>>>>> >>>>>>>>>> And another change to fix the word wrapping in the README which >>>>>>>>>> was totally different from the rest of the file. >>>>>>>>>> >>>>>>>>>> Thanks! >>>>>>>>>> >>>>>>>>> >>>>>>>>> Oh, and do we need all the code in config.py? I really didn't even >>>>>>>>> want a config option in there to turn theming on or off (what's the >>>>>>>>> point?), let alone 20 new lines. >>>>>>>>> >>>>>>>> The code is added after the config_local and config_distro is >>>>>>>> loaded. So, user won't be able to disable it unless he directly changes the >>>>>>>> config.py. >>>>>>>> >>>>>>> >>>>>>> That is clearly wrong and needs to be fixed. config_local and >>>>>>> config_distro should be able to override anything in config.py. >>>>>>> >>>>>>> But... why allow the themes to be updated or disabled at all? It's >>>>>>> not like a non-developer can add new ones, and it's not a security issue >>>>>>> that an administrator might need to control. In fact, it's arguably an >>>>>>> accessibility feature, for those whose eyes (like mine) last the day better >>>>>>> with a darker theme. >>>>>>> >>>>>>> Let's remove it entirely please. I don't see any good reason to have >>>>>>> any of that in config.py. >>>>>>> >>>>>> Intention is not to allow disabling the themes, but it's the feature >>>>>> implementation code. I'll move out the code. >>>>>> >>>>>>> >>>>>>> Thanks. >>>>>>> >>>>>>> >>>>>>>> I'll reduce the code a bit. >>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Dave Page >>>>>>>>> Blog: http://pgsnake.blogspot.com >>>>>>>>> Twitter: @pgsnake >>>>>>>>> >>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>>>>> The Enterprise PostgreSQL Company >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Thanks and Regards, >>>>>>>> Aditya Toshniwal >>>>>>>> Sr. Software Engineer | EnterpriseDB India | Pune >>>>>>>> "Don't Complain about Heat, Plant a TREE" >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Dave Page >>>>>>> Blog: http://pgsnake.blogspot.com >>>>>>> Twitter: @pgsnake >>>>>>> >>>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>>> The Enterprise PostgreSQL Company >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Thanks and Regards, >>>>>> Aditya Toshniwal >>>>>> Sr. Software Engineer | EnterpriseDB India | Pune >>>>>> "Don't Complain about Heat, Plant a TREE" >>>>>> >>>>> >>>>> >>>>> -- >>>>> Thanks and Regards, >>>>> Aditya Toshniwal >>>>> Sr. Software Engineer | EnterpriseDB India | Pune >>>>> "Don't Complain about Heat, Plant a TREE" >>>>> >>>> >>>> >>>> -- >>>> Thanks and Regards, >>>> Aditya Toshniwal >>>> Sr. Software Engineer | EnterpriseDB India | Pune >>>> "Don't Complain about Heat, Plant a TREE" >>>> >>> >>> >>> -- >>> Thanks and Regards, >>> Aditya Toshniwal >>> Sr. Software Engineer | EnterpriseDB India | Pune >>> "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* >> > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > --000000000000a2122c0597138c92 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Dave,

On Mon, Nov 11, 2019, 20:33 Dave Page <dpage@pgadmin.org> wrote:
Hi

That= 's looking a lot better. I see a couple of issues (screenshots attached= ):

- The combo box on the query tool toolbar isn&#= 39;t dark.
I'm no= t sure why you are not able to see it but I've changed it. Try a python= restart.
- The tagline on the dashboard logo i= sn't readable.
Ye= s because that's an SVG image. We may need to make it completely from H= TML.

A couple of other thoughts:=

- The guide lines on the graphs are still too bri= ght. Let's make them the same colour as the graph boarders please.
There is technical limita= tion here. To change those grid lines based on theme we need to import vari= ables in JS. I didn't find a way to dynamically load SASS variables bas= ed on theme.
So I chose color which would work for b= oth dark and black backgrounds and is hard coded in JS.

- The header bar foreground colour and the dividing= white line below it seem quite harsh. Are they pure white? They should be = softened a little bit.
I'll check the color, but AFAIR it's not pure white.

Thanks!

On Mon, Nov 11, 2019 at 1:22 PM= Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:=
Thanks, patch applied.

As per discussion with Aditya, = we have removed customized scroll bars for the time=C2=A0being as they are = not clearly=C2=A0visible with some of the components.

On Mon, Nov 11, = 2019 at 5:25 PM Aditya Toshniwal <aditya.toshniwal@enterp= risedb.com> wrote:
Hi Hackers,

Attached is the updated patch.
Kin= dly review.

On Mon, Nov 11, 2019 at 3:42 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

Kindly hol= d on with the patch. Few more changes required per review by=C2=A0@Ashesh Vashi=C2=A0.

On Mon, Nov 11, 2019 at 3:07 PM Aditya T= oshniwal <aditya.toshniwal@enterprisedb.com> wrote= :
Hi= Hackers,

Attached is the patch for further=C2=A0improvements in the D= ark theme colors.
Gray shades and other colors are changed to identify diff= erent components more clearly. Few of the controls were missing the privile= ges of dark theme, fixed that.
Few dashboard=C2=A0graph=C2=A0related change= s.
As suggested, theme related code changes is removed from config.py and m= oved to miscellaneous under a new package - Themes. Thank you=C2=A0@Ashesh Vashi=C2=A0for inputs on that.=

Kindly review.

On Mon, Nov 11, 2019 at 3:00 PM Aditya Toshniwal = <aditya.toshniwal@enterprisedb.com> wrote:
Hi Dave,

On Mon, Nov 11, 2019 at 2:38 PM Dave Page <dpage@pg= admin.org> wrote:
Hi

On Mon, Nov 11, 2019 at 7:01 AM Aditya T= oshniwal <aditya.toshniwal@enterprisedb.com> wrote= :
Hi Dave,<= /div>

On Thu, Nov 7, 2019 at 7:56 PM Dave Page <dpage@pgadmin.org> = wrote:


On Thu, Nov 7, 2019 at 2:18 PM Dave Page <= d= page@pgadmin.org> wrote:

On Thu, Nov 7, 2019 at 1:= 25 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wro= te:
Thanks, patch applied.

On Thu, Nov 7, 2019 at 6:39 PM Aditya Toshniwal = <aditya.toshniwal@enterprisedb.com> wrote:
Hi Hackers,

Attached is the updated patch with few more changes and correction= s.
Kindly review.
<= /div>

I've committed= a couple of minor tweaks - one to remove a space, e.g.
gettext('A page=
 refresh is required to apply the theme. Do you wish to refresh the page no=
w ?'),
is now:
gettext('A page refresh is re=
quired to apply the theme. Do you wish to refresh the page now?'=
),
And another change to fix the word wrapping in the READM= E which was totally different from the rest of the file.

Thanks!

Oh, and do= we need all the code in config.py? I really didn't even want a config = option in there to turn theming on or off (what's the point?), let alon= e 20 new lines.
The code is added after the = config_local and config_distro is loaded. So, user won't be able to dis= able it unless he directly changes the config.py.
=

That is clearly wrong and needs to be fixe= d. config_local and config_distro should be able to override anything in co= nfig.py.

But... why allow the themes to be updated= or disabled at all? It's not like a non-developer can add new ones, an= d it's not a security issue that an administrator might need to control= . In fact, it's arguably an accessibility feature, for those whose eyes= (like mine) last the day better with a darker theme.

<= div>Let's remove it entirely please. I don't see any good reason to= have any of that in config.py.
Intention is= not to allow disabling the themes, but it's the feature implementation= code.=C2=A0I'll move out the code.
<= div>
Thanks.
=C2=A0
I'= ll reduce the code a bit.

--
Dave P= age
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postg= reSQL Company


--
Th= anks and Regards,
Aditya = Toshniwal
Sr. Software Engineer |=C2=A0EnterpriseDB India |=C2=A0Pune
"Don't Complain a= bout Heat, Plant a TREE"


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake<= br>
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise= PostgreSQL Company


--
Th= anks and Regards,
Aditya = Toshniwal
Sr. Software Engineer |=C2=A0EnterpriseDB India |=C2=A0Pune
"Don't Complain a= bout Heat, Plant a TREE"


--
Th= anks and Regards,
Aditya = Toshniwal
Sr. Software Engineer |=C2=A0EnterpriseDB India |=C2=A0Pune
"Don't Complain a= bout Heat, Plant a TREE"


--
Th= anks and Regards,
Aditya = Toshniwal
Sr. Software Engineer |=C2=A0EnterpriseDB India |=C2=A0Pune
"Don't Complain a= bout Heat, Plant a TREE"


--
Th= anks and Regards,
Aditya = Toshniwal
Sr. Software Engineer |=C2=A0EnterpriseDB India |=C2=A0Pune
"Don't Complain a= bout Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
<= font color=3D"#3333FF">Sr. Software Architect=
<= font color=3D"#000000" face=3D"arial, sans-serif">EnterpriseDB Software = India Private Limited
Mobile: +91 976-788-8246=


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake<= br>
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise= PostgreSQL Company
--000000000000a2122c0597138c92--