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 1iU629-0004sq-Ha for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 09:30:53 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1iU627-0003vw-97 for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 09:30:51 +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 1iU626-0003vp-NK for pgadmin-hackers@lists.postgresql.org; Mon, 11 Nov 2019 09:30:51 +0000 Received: from mail-oi1-x243.google.com ([2607:f8b0:4864:20::243]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1iU623-0006ni-EV for pgadmin-hackers@postgresql.org; Mon, 11 Nov 2019 09:30:49 +0000 Received: by mail-oi1-x243.google.com with SMTP id 14so4922063oir.12 for ; Mon, 11 Nov 2019 01:30:47 -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=mKejo3xcpBwxOx6c7VU8MechwI5g2tYaoywQS3UXW90=; b=n7mquLK5pEplsZKpBGt4ocISh9jVq1EjsoAB5F+DiyVfxxMe0M9AjM334vgjjSpKTL G1U9nbtKn4mqzdGRx2ClyBNkyBLxmVVCH6ixl9r1cZ4Pi9y8hVI+is9zdoODHyHAXsxL qWc9kGMUjUrEcNUsiHkFpP+8fk41qymli19gXL3Fn4TWsY6n3vx3neDVA2zHDcY6YfP5 oRjoIkhxXWb1JO+9bnn5y5uWBvzcXe5KLxIgJZRoC+DM+//ULCcR1aGNLmRa9KpGsQVv tdSSbiv/Te3JO+j1oiZch3aD32ukiwdtu3BcmhOT0emF8+3kdpV25oKLWCtzXQC7D2DB CJlA== 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=mKejo3xcpBwxOx6c7VU8MechwI5g2tYaoywQS3UXW90=; b=aioBrHUTrag/vKCwfp2MezGz3hLNpAig/1hy+ygC/5UsdqXTqTJ5coFq2X8PcvRy5L /C0TIoUm1OLOvEDU8KNrfCttV9aRGH5wv/gINDRur1HvVx8mfyQMSASoPRmp48gjXmr6 YEIv4QvswpoJgV+fTA46PoJNkhVeyWti0lk/VNiw07aE4MARj85/9F0sA57SU6NOE/9x 2VAfeJxCb1qnM/3HWYSNUMaXjOEoTIy6odyc/XVUrFTmLNAs6y6tEd0584JIDDthm53X wZorVHTc2GiymJiYw1U1fClfTSBtjsXi/Kepped9iAKti/KxDqt8E90VpO/Luzk2cDBp Mk8w== X-Gm-Message-State: APjAAAXq8xzs4K3JJi8QIQzxiejg3UuyYGYrFTsrTRQJh/mXh4yQGBxX Gmp/aNZrpEQcxzZBsuAtWtxdhWJVMqlBpGjoSAzWFXJZ0fhoLYzVRGeSLD8KfZFtvvOwHBDa65K igQWhnRkmCj/lbu8ib6eNsijAJ+9S7IdMY829vzs7I9U4DT1V8rQYHz/pTFlL1uFPRN+yy8fs+a 7BPBxDISZs0nNRESctz7M9xm0RfxG3Vu4E8UiMHB5UWMqfuEbh70E= X-Google-Smtp-Source: APXvYqxkap4p1vtUtDxsrt/11nTTp6+FJcG7QUIEavxbcKUfGgiRTdXZnhgcfnyKZQSpSggRPLwxKWCCKaQW6uGYJ3w= X-Received: by 2002:aca:d558:: with SMTP id m85mr21665362oig.43.1573464646557; Mon, 11 Nov 2019 01:30:46 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Aditya Toshniwal Date: Mon, 11 Nov 2019 15:00:10 +0530 Message-ID: Subject: Re: [pgAdmin][RM4348] Theme options in pgAdmin and dark theme To: Dave Page Cc: Akshay Joshi , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000535dde05970ec949" 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 --000000000000535dde05970ec949 Content-Type: text/plain; charset="UTF-8" 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" --000000000000535dde05970ec949 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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.to= shniwal@enterprisedb.com> wrote:
Hi Dave,

On Thu, Nov 7, 2019 at 7:56 PM D= ave Page <dpage@p= gadmin.org> wrote:


On Thu, Nov 7, 2019 at 2:1= 8 PM Dave Page <d= page@pgadmin.org> 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@en= terprisedb.com> wrote:
Hi Hackers,

=
Attached is the updated patch= with few more changes and corrections.
Kindly review.
<= div>
I've committed a couple of minor tweaks - one to rem= ove a space, e.g.
gettext('A page refresh is required to apply the the=
me. Do you wish to refresh the page now ?'),
is = now:
gettext('A page refresh is required to apply the theme. Do you wis=
h to refresh the page now?'),
And another change= to fix the word wrapping in the README which was totally different from th= e 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 loa= ded. So, user won't be able to disable it unless he directly changes th= e config.py.

That= is clearly wrong and needs to be fixed. config_local and config_distro sho= uld be able to override anything in config.py.

But= ... why allow the themes to be updated or disabled at all? It's not lik= e 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 acces= sibility feature, for those whose eyes (like mine) last the day better with= a darker theme.

Let's remove it entirely plea= se. I don't see any good reason to have any of that in config.py.
=
Intention is not to allow disabling the themes, b= ut it's the feature implementation code.=C2=A0I'll move out the = code.

Thanks.
= =C2=A0
I'll reduce the code a bit.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitte= r: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgr= eSQL 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

EnterpriseDB= UK: http://www.e= nterprisedb.com
The Enterprise PostgreSQL Company


--
Thanks and Regards,
Aditya Toshniwal
Sr. Software Engineer |=C2=A0EnterpriseDB In= dia |=C2=A0Pune
"Don't Complain about Heat, Plant a TREE"
=
--000000000000535dde05970ec949--