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 1iUBqW-00020H-5E for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 15:43:16 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1iUBqU-00011l-QU for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 15:43:14 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iUBqU-00011e-GS for pgadmin-hackers@lists.postgresql.org; Mon, 11 Nov 2019 15:43:14 +0000 Received: from mail-wr1-x441.google.com ([2a00:1450:4864:20::441]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iUBqM-0006AN-Nc for pgadmin-hackers@postgresql.org; Mon, 11 Nov 2019 15:43:13 +0000 Received: by mail-wr1-x441.google.com with SMTP id p4so15137537wrm.8 for ; Mon, 11 Nov 2019 07:43:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+D8HnfTnJrQEQHgzLav5LPEPI7nelRI1wolrmMyomV4=; b=mj+nKx2mtkRrtcDN1lD2kh99TdqHDp8rJQdm1TJKkJzD7fesk+sGcccU1q4sxF1EsE 1V6XxOGcC7FQm06WQS8fVUkZKlkTSS++jtU7wnpV1d+joTfRYaAaDdRH9LG627OlJCZ4 kSlpMuAT+OHir5K5QYaYerZzqDLWfpXNGGJUnXLcZ4FBjabBjAXs9GHb0V5KPWiPYrRd DHxOmualIbttqwft7Ys0cWYt2YtXxyNQNiO+98r51mqET/5gMgd8fOT/n/EFjSP5d/SI IZ6RY/ACw+V7oqCH5PmOw+TDhzXxIAgtZtpwEqEISIHhk4dzK58YmEchpVpy/P02ruq7 EqLg== 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=+D8HnfTnJrQEQHgzLav5LPEPI7nelRI1wolrmMyomV4=; b=cCHLHDSQcUFi9uVtkKnXhCVOtybYs78eF+WlOnZOuUZG4PrJvkszHEq4k9xk8Cycdc zv9A+e8FdMWuMIAkQ55oi9xXTVZMuXKQjLGCwKuLmJd9IKqIu32CxhF2WyJn5zVQzb/0 bIqNrmItSESZisJuXdDnNudLKkaTNTN34t/fcSCpTnVjsdcEi1JNXOqQ86iQ0zIB3tAk KpNfIaa+jRUZAc9B4mGoZpBqQMUe76SLBdCMZF6mhNXnMvyGS6hKYx/eI9vi6g8bpntv xbNph1Lillt4zsaK7u2H4yb3+weikP2sNn0t/7KRnrJMzkla/LhyT6q4P5VxAdNn5Mi9 aBuw== X-Gm-Message-State: APjAAAW36SzW8aVfJF61PafNVEg4N9HdLS+Y7/Bp4UVHm1K5fwep5A82 dWs1gGtdZjh4NytDRxtnjOwL31Q7bKl1E2tviZMH1Q== X-Google-Smtp-Source: APXvYqxLOqmqOmdrtYUjoIaG8vShIbGHvMx0pwEdBr9Zkublekja6cyaWfK68hb7ewghWFYkxNkbglCRJ2ectHMpLdc= X-Received: by 2002:adf:9381:: with SMTP id 1mr21039710wrp.10.1573486984106; Mon, 11 Nov 2019 07:43:04 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Mon, 11 Nov 2019 15:42:51 +0000 Message-ID: Subject: Re: [pgAdmin][RM4348] Theme options in pgAdmin and dark theme To: Aditya Toshniwal Cc: Akshay Joshi , pgadmin-hackers , Ashesh Vashi Content-Type: multipart/alternative; boundary="000000000000bf2d7e059713fcbc" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000bf2d7e059713fcbc Content-Type: text/plain; charset="UTF-8" Hi On Mon, Nov 11, 2019 at 3:11 PM Aditya Toshniwal < aditya.toshniwal@enterprisedb.com> wrote: > 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. > I've done a make clean/make bundle, restarted Python, and done a hard reload in the browser to no joy. FYI, I'm running in Chrome 78.0.3904.70 on macOS. > - 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. > Can we have an alternate image? Maybe if it's served through a flask route and we look for /logo.png?t=dark or something like that to select the image version? > >> 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. > Urgh, OK. We'll need to think about that then. > >> - 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. > OK, thanks! > >> 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 >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --000000000000bf2d7e059713fcbc Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Mon, Nov 11, 2019 at 3:11 PM Aditya Toshniwal &l= t;aditya.toshniwal@ent= erprisedb.com> wrote:
Hi Dave,

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

That's looking a lot bette= r. 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 abl= e to see it but I've changed it. Try a python restart.

I've done a make clean/make bundle, restart= ed Python, and done a hard reload in the browser to no joy. FYI, I'm ru= nning in Chrome=C2=A078.0.3904.70 on macOS.
=C2=A0
- The tagline on the dashboard logo isn't rea= dable.
Yes because th= at's an SVG image. We may need to make it completely from HTML.

Can we have an alternate image? Maybe = if it's served through a flask route and we look for /logo.png?t=3Ddark= or something like that to select the image version?
=C2=A0
=

A couple of other thought= s:

- The guide lines on the graphs are still too b= right. Let's make them the same colour as the graph boarders please.
There is technical limi= tation here. To change those grid lines based on theme we need to import va= riables in JS. I didn't find a way to dynamically load SASS variables b= ased on theme.
So I chose color which would work for= both dark and black backgrounds and is hard coded in JS.

Urgh, OK. We'll need to think about that the= n.
=C2=A0
=

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

OK, thanks!
=C2=A0

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 P= M Aditya Toshniwal <aditya.toshniwal@enterprisedb.com= > wrote:
Hi Hackers,

Attached is the updated patch.
Kindly review.

On Mon, Nov 11, 201= 9 at 3:42 PM Aditya Toshniwal <aditya.toshniwal@enterpris= edb.com> wrote:
Hi,

Kindly hold on with the patch. Few more chan= ges required per review by=C2=A0@A= shesh Vashi=C2=A0.

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

Attached is the patch for further=C2=A0improvements in the Dar= k theme colors.
Gray sha= des and other colors are changed to identify different components more clea= rly. Few of the controls were missing the privileges of dark theme, fixed t= hat.
Few dashboard=C2=A0= graph=C2=A0related changes.
As suggested, theme related code changes is removed from config.py and = moved 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@pgadmin.org> wrote:
Hi

On Mon, Nov = 11, 2019 at 7:01 AM Aditya Toshniwal <aditya.toshniwal@en= terprisedb.com> wrote:
Hi Dave,

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


On Thu, Nov 7, 201= 9 at 2:18 PM Dave Page <dpage@pgadmin.org> wrote:

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

On Thu, Nov 7, 2019 = at 6:39 PM Aditya Toshniwal <aditya.toshniwal@enterprised= b.com> wrote:
Hi Hack= ers,

Attached is the updated patch with few= more changes and corrections.
Kindly review.

<= /div>
I've committed a couple of minor tweaks - one to remove a spa= ce, 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 refre=
sh the page now?'),
And another change to fix th= e 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 di= dn't even want a config option in there to turn theming on or off (what= 's the point?), let alone 20 new lines.
<= div>= The code is added after the config_local and config_distro is loaded. So, u= ser won't be able to disable it unless he directly changes the config.p= y.

That is clearl= y wrong and needs to be fixed. config_local and config_distro should be abl= e to override anything in config.py.

But... why al= low the themes to be updated or disabled at all? It's not like a non-de= veloper can add new ones, and it's not a security issue that an adminis= trator might need to control. In fact, it's arguably an accessibility f= eature, for those whose eyes (like mine) last the day better with a darker = theme.

Let's remove it entirely please. I don&= #39;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.


--
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


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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL = Company
--000000000000bf2d7e059713fcbc--