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 1iU5gL-0003ye-Dc for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 09:08:22 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1iU5gJ-0007yl-UK for pgadmin-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 09:08:19 +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 1iU5gJ-0007ye-CE for pgadmin-hackers@lists.postgresql.org; Mon, 11 Nov 2019 09:08:19 +0000 Received: from mail-wm1-x341.google.com ([2a00:1450:4864:20::341]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iU5gF-0001yJ-DC for pgadmin-hackers@postgresql.org; Mon, 11 Nov 2019 09:08:18 +0000 Received: by mail-wm1-x341.google.com with SMTP id z19so12353474wmk.3 for ; Mon, 11 Nov 2019 01:08:14 -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=HSPB6l/sX2i9YJaaAOixgIfzPOf3rh55D8+VNioV13k=; b=JBjQs+H8002SlrECn+lWgLwKZLX41cmE3j9QUi7tpfxh+Z9WKUzw0+S85Xv79Cup8O 1kk9ZHiqJTudt1EdV1EgXhycaA9NJLTRqCagZ+EvgwGgG3f2vPmo2NHchPCYkbzZr7fl uCzzRFt13gUS3Bet7tGXZveTwolFV0xWJtcGDcYi67adj9mmXXrTXPn9/eVB1eoiPtQy 07jTlpruyToGAWa5wpvdVG6pZ593PdC4yq6bfI7PKJEOZW/zsgC8fbC9MUuWbI/CLEUZ L5OFxUsv0Y5cLPLsxgWxSc6vGp8V6ESbKJswQyo11y6JaFQvQLTR55aNV8KvQ0cLYa41 QN4w== 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=HSPB6l/sX2i9YJaaAOixgIfzPOf3rh55D8+VNioV13k=; b=bgmOkgOsy2sUAXQJbhzKpm8cyUnUMORBVlk8cncYA4pB160S2JTD9bpsHQAm0yKyIs 3OrROnPIReuwPHA7T8oz6j6Us5fC1+fNLGirlHWjEVMTPTRYgYNdNNrgMh/hFiYoVpAo cyLrL8YNGrCz2MqkTyBK59NPgMktUU/jdFAZjUURab9XTLCqy4geXxfaPruqIy+Ia4LS LlqZwAAG7QzgU24cnZtlqpvj8zBMNwkgfrVTzfrqXz2ViOzpzbeYbaq6GmhJy8wbbmQi aptisrI2ebqMPcZKH8cuoQvd9JAAMz/TSJ6BZkO10w6CZMddwSlMfzc89diJa6hg9r3p LvxQ== X-Gm-Message-State: APjAAAVZOa7a5Ep2mYroUyP325xLVzu2duYn7LyD4chlpbXt7mwO0G4I xd2zJF+d0AVF+ueKmTSdKHLqAD2U/i9mg7ocwCzrwDP1v2c= X-Google-Smtp-Source: APXvYqwXkT3O1st8GHxZ9YJ3Is8AkUB8Bg4cRAHNlgaTNriWz2qP533nxE6DcjfLWvXhOAKU/uSrMS4dnzmUwdzo+6c= X-Received: by 2002:a7b:c38c:: with SMTP id s12mr20289997wmj.84.1573463293726; Mon, 11 Nov 2019 01:08:13 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Mon, 11 Nov 2019 09:08:01 +0000 Message-ID: Subject: Re: [pgAdmin][RM4348] Theme options in pgAdmin and dark theme To: Aditya Toshniwal Cc: Akshay Joshi , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000b0a5af05970e78c5" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000b0a5af05970e78c5 Content-Type: text/plain; charset="UTF-8" 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. 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 --000000000000b0a5af05970e78c5 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

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

On Thu, Nov 7, 2019 at 7:56 PM Dave Page &l= t;dpage@pgadmin.org<= /a>> wrote:
<= div dir=3D"ltr">


<= div dir=3D"ltr" class=3D"gmail_attr">On Thu, Nov 7, 2019 at 2:18 PM Dave Pa= ge <dpage@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@enterprisedb.= com> wrote:
Hi Hacker= s,

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 spac= e, e.g.
gettext('A page refresh is required to apply the theme. Do you =
wish to refresh the page now ?'),
is now:
<= div>
gettext('A page refresh is required to apply the theme. Do you wish to refres=
h the page now?'),
And another change to fix the= word wrapping in the README which was totally different from the rest of t= he file.

Thanks!

Oh, and do we need all the code in config.py? I really did= n't even want a config option in there to turn theming on or off (what&= #39;s the point?), let alone 20 new lines.
T= he code is added after the config_local and config_distro is loaded. So, us= er 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 all= ow the themes to be updated or disabled at all? It's not like a non-dev= eloper can add new ones, and it's not a security issue that an administ= rator might need to control. In fact, it's arguably an accessibility fe= ature, for those whose eyes (like mine) last the day better with a darker t= heme.

Let's remove it entirely please. I don&#= 39;t see any good reason to have any of that in config.py.


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

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