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 1iU6iL-0006TN-JS for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 10:14:29 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1iU6hM-0001kS-F8 for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 10:13:28 +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 1iU6hM-0001kJ-0L for pgadmin-hackers@lists.postgresql.org; Mon, 11 Nov 2019 10:13:28 +0000 Received: from mail-ot1-x343.google.com ([2607:f8b0:4864:20::343]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1iU6hE-00077e-Vf for pgadmin-hackers@postgresql.org; Mon, 11 Nov 2019 10:13:26 +0000 Received: by mail-ot1-x343.google.com with SMTP id v24so10805426otp.5 for ; Mon, 11 Nov 2019 02:13:20 -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=8REIQnhpXOPbIYIgu0saUxKdoE/F7onnn6zf0vA/bKw=; b=c5z1tSf0xZczvAc3Z8gsa4/b1jwDI4U1UMSaxIaKSvQbYJ3SgA0UKX2Bf60k+uw6/a AMOJ2fn8PZyzl+xhiq36iQjW9A4tuhiIj+rFQ/dHItXOcaEAYAepGliPLRQ8MFQ8maum l0hOlCQrmm6dvP24YlLZIZ8j2k0s3wHkyiHlPGEzn7EdM9TaYyiiN1B3q8IOENNu/3oy 85Oai302vyP8wPytX5CYm4UVXeqOFyHq7w1WhMGT8JCJnahXnDmEmLFi2LQkm9XT1ce1 z1FBjVbBePk+LXyQTRjZbTVjyHRd5tscJAER+s63JfKanSJcdAHoeNpXPlvjHsGLNL3O K6Zw== 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=8REIQnhpXOPbIYIgu0saUxKdoE/F7onnn6zf0vA/bKw=; b=BCmd8y221a2/v1NhnJJ2kHlsGF1KKPwq28wifU1XZIsyM11sKWFwctlNiH09IiC2dB qoMB9JVsyu+1oiBNJSY40vh7k4qjkOdCPhwZjlRkqoGiBbP7vzSdiTJnVKCgPdIjb8om ViUgErnK26BCt5lQucdrAU8Bf+TLQlxCD4Xq4CBKnnA3va1+D93xtiFFStScWnARBUKE srF1W/22F2AtbtBYq57vSg+YSosjMNvamjOsrZ2PpcuqA2hiRaxGIQUgBSGGlHdgtj8O 8qtzgX9L9IXwOIDuPGEBltZskjnzZmULVkhIqhqBmES1tsr2ycp+SZoqHsEL0wZlaVFA r3pg== X-Gm-Message-State: APjAAAW2zdHogQ4ECaqJXDyONTUzPkzKrse8ybT4Cv6r7QdDgQteZPva TieADNTJ4BJ1r/OeMiJ68o79wu4cfjpacIS6t5kwWCEhYoD9bPdvaQuKE7lytOwVkY+S7j4Gy3k Oza9bdx//UZFmJgL9jLJMvSy/zDEHd/N+rbhT0TXP9dW11lx9IcmBODGiXQRFiPfOys/RJWcngV jx21bovdSgs3RivXHQyz6XWiLZNwW2RYfgIYxvexY7Ha9larjjwppkq2l8 X-Google-Smtp-Source: APXvYqzLMB4hK9xT+JnzRS4hoXawKperZxcyYbFcxjaA4mY+Zpm0leGwmsuAheZ2RgW/n/J1AcBT0yAynb461B3S0Jk= X-Received: by 2002:a9d:648f:: with SMTP id g15mr16464364otl.195.1573467199916; Mon, 11 Nov 2019 02:13:19 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Aditya Toshniwal Date: Mon, 11 Nov 2019 15:42:43 +0530 Message-ID: Subject: Re: [pgAdmin][RM4348] Theme options in pgAdmin and dark theme To: pgadmin-hackers , Ashesh Vashi Cc: Akshay Joshi , Dave Page Content-Type: multipart/alternative; boundary="00000000000084896605970f6137" 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 --00000000000084896605970f6137 Content-Type: text/plain; charset="UTF-8" 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" --00000000000084896605970f6137 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,

Kindly hold on with the patch. Few more changes requir= ed per review by=C2=A0@Ashesh Vashi<= /a>=C2=A0.

On Mon, Nov 11, 2019 at 3:07 PM Aditya Toshniwal <aditya.toshniwal@enterprised= b.com> wrote:
Hi Hackers,

Attached is the patch for further=C2=A0= improvements in the Dark theme colors.
Gray shades and other colors are cha= nged to identify different components more clearly. Few of the controls wer= e missing the privileges of dark theme, fixed that.
Few dashboard=C2=A0grap= h=C2=A0related changes.
As suggested, theme related code changes is removed= from config.py and moved to miscellaneous under a new package - Themes. Th= ank 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@enterpris= edb.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 A= ditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex">
Hi Dave,
=
On Thu= , Nov 7, 2019 at 7:56 PM Dave Page <dpage@pgadmin.org> wrote:

<= /div>
O= n Thu, Nov 7, 2019 at 2:18 PM Dave Page <dpage@pgadmin.org> wrote:
=
= On Thu, Nov 7, 2019 at 1:25 PM Akshay Joshi <akshay.joshi@enterprisedb.com&g= t; wrote:
Thanks, patch applied.

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

Atta= ched 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 r=
equired to apply the theme. Do you wish to refresh the page now ?'),
is now:
gettext(<=
span style=3D"color:rgb(106,135,89)">'A page refresh is required to app=
ly the theme. Do you wish to refresh the page now?'),
And another change to fix the word wrapping in the README which was t= otally 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 the= re to turn theming on or off (what's the point?), let alone 20 new line= s.
The code is added after the config_loca= l and config_distro is loaded. So, user won't be able to disable it unl= ess he directly changes the config.py.

That is clearly wrong and needs to be fixed. config_l= ocal and config_distro should be able to override anything in config.py.

But... why allow the themes to be updated or disable= d 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 o= f that in config.py.
Intention is not to all= ow disabling the themes, but it's the feature implementation code.=C2=A0I'll move out the code.


--
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 and Regards,
Aditya Toshniwal
Sr. Software Engineer |=C2=A0EnterpriseDB In= dia |=C2=A0Pune
"Don't Complain about Heat, Plant a TREE"
=
--00000000000084896605970f6137--