Re: [pgAdmin][RM4348] Theme options in pgAdmin and dark theme

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Aditya Toshniwal <[email protected]>
To: Dave Page <[email protected]>
Cc: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Ashesh Vashi <[email protected]>
Subject: Re: [pgAdmin][RM4348] Theme options in pgAdmin and dark theme
Date: Tue, 12 Nov 2019 15:20:04 +0530
Message-ID: <CAM9w-_mfFq2wKZwTvzRaEPre+Shz-ESSEhy3CA-SuFmcvNCAzQ@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoz=TRA8UHjaQycqPcdsRmHwLz72gwBPitY2nQMuvBw3-g@mail.gmail.com>
References: <CAM9w-_m5mfGiu1mUH28Hngi1riYTmTurNoxz-qWOH7g+zRFOhA@mail.gmail.com>
	<CAM9w-_nNp1BAy9usd1j9xP+gKSrR-_BLMwoCUzngx0-8HcbRmw@mail.gmail.com>
	<CANxoLDcAnDBA-a5577wo0QcXVq0bmk+DPKv=K7nekRiejcPXzA@mail.gmail.com>
	<CA+OCxozmGJmLvH1g0vCn09OOTiUTnZbZ_nPHU0dNU1pRXYjQnA@mail.gmail.com>
	<CA+OCxow-ZLN8sKETj5Bf7Y4gTWm+q7NtugfpG3_zt-KHzpvbJA@mail.gmail.com>
	<CAM9w-_=fuDGpPmFLg=w--LCF1jACMuWcqGb5BFX57AhE-Vm4xQ@mail.gmail.com>
	<CA+OCxoxKgZFwtqhc89Pu3T_aPWkTmt4_rO32WuewaDLqoLM7wg@mail.gmail.com>
	<CAM9w-_=OHmdrWRJWnwN5_cfNPfak+34=MwbiHpez9HBUtk2ZzQ@mail.gmail.com>
	<CAM9w-_kma2zGbk-3dr68z2O4ZvfJEmt5i8y9tgQwcMOvshWtaw@mail.gmail.com>
	<CAM9w-_=M1G-m-vb_7j1G1LtEis6aT8KakHmWGxJmRfUV-oZ5ug@mail.gmail.com>
	<CAM9w-_ntcD1bJkm3h3n3S9TO+GxGxmJPY17Qi079Z87x9b_W4g@mail.gmail.com>
	<CANxoLDfgiKoaZyHuDAMier0yjrmnYCc9Diht+yvhWQau1Xr05Q@mail.gmail.com>
	<CA+OCxowQ_qPmaLoCwq6ca0KzojaO7U49yJna3_rNJqUp-wTYJA@mail.gmail.com>
	<CAM9w-_=18dvyzS4wB=8es_dMTr9O0bnVq7VD0OC1JgxVsXJqXA@mail.gmail.com>
	<CA+OCxowN-7O=LoKoDjfEUp8J_cij7zd+=rn99n4=rYbEJXbuvw@mail.gmail.com>
	<CAM9w-_=iOUWtvLJ3vq=McZZYyWRkjoF_HcpDdHE8=fUE-jGMyw@mail.gmail.com>
	<CA+OCxoz=TRA8UHjaQycqPcdsRmHwLz72gwBPitY2nQMuvBw3-g@mail.gmail.com>

Hi,

On Tue, Nov 12, 2019 at 2:47 PM Dave Page <[email protected]> wrote:

> Hi
>
> On Tue, Nov 12, 2019 at 4:59 AM Aditya Toshniwal <
> [email protected]> wrote:
>
>> Hi Hackers,
>>
>> Attached is a patch to address the review comments.
>>
>> On Mon, Nov 11, 2019 at 9:13 PM Dave Page <[email protected]> wrote:
>>
>>> Hi
>>>
>>> On Mon, Nov 11, 2019 at 3:11 PM Aditya Toshniwal <
>>> [email protected]> wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Mon, Nov 11, 2019, 20:33 Dave Page <[email protected]> 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.
>>>
>> [image: Screenshot 2019-11-12 at 10.03.59.png]
>> I'm not seeing it. Pulled latest, bundled and restarted. :/
>>
>
> So I eventually found that I had a rogue Python process running pgAdmin
> that somehow got detached from PyCharm. Killed that, restarted, and now it
> looks good.
>
>
>>
>>
>>>
>>>> - 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?
>>>
>> Or better embed the SVG so that we can change colors using CSS. Patch
>> attached. You can tweak $color-brand to change the welcome dashboards blue
>> color.
>>
>
> Looks good.
>
>
>>
>>>
>>>>
>>>>> 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!
>>>
>> I didn't find anyone using white color. Could you please share a
>> screenshot ? I've changed the foreground from #ddd to #d4d4d4 to soften a
>> bit.
>>
>
> Attached. It may just be that it looks a little sharp because it's a long
> line. Maybe we should just make it a darker shade entirely.
>
> [image: Screenshot 2019-11-12 at 09.09.42.png]
>

Why can't I see it :/
Maybe it got fixed along with the dark drop down for no limit. Could you
please check again.
[image: image.png]

>
> BTW, what was the reason for removing the styling from the scroll bars?
>
We had given gray shades to scrollbar which were not clearly visible for
some tables. Plus, for systems were scrollbar is not always enabled was
showing scrollbar. It should appear only on scroll on those systems.

>
>
>>>
>>>>
>>>>> Thanks!
>>>>>
>>>>> On Mon, Nov 11, 2019 at 1:22 PM Akshay Joshi <
>>>>> [email protected]> 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 <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi Hackers,
>>>>>>>
>>>>>>> Attached is the updated patch.
>>>>>>> Kindly review.
>>>>>>>
>>>>>>> On Mon, Nov 11, 2019 at 3:42 PM Aditya Toshniwal <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Kindly hold on with the patch. Few more changes required per review
>>>>>>>> by @Ashesh Vashi <[email protected]> .
>>>>>>>>
>>>>>>>> On Mon, Nov 11, 2019 at 3:07 PM Aditya Toshniwal <
>>>>>>>> [email protected]> 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 <[email protected]> for inputs on that.
>>>>>>>>>
>>>>>>>>> Kindly review.
>>>>>>>>>
>>>>>>>>> On Mon, Nov 11, 2019 at 3:00 PM Aditya Toshniwal <
>>>>>>>>> [email protected]> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Dave,
>>>>>>>>>>
>>>>>>>>>> On Mon, Nov 11, 2019 at 2:38 PM Dave Page <[email protected]>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Nov 11, 2019 at 7:01 AM Aditya Toshniwal <
>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Dave,
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, Nov 7, 2019 at 7:56 PM Dave Page <[email protected]>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, Nov 7, 2019 at 2:18 PM Dave Page <[email protected]>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Thu, Nov 7, 2019 at 1:25 PM Akshay Joshi <
>>>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks, patch applied.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Thu, Nov 7, 2019 at 6:39 PM Aditya Toshniwal <
>>>>>>>>>>>>>>> [email protected]> 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
>>>
>>
>>
>> --
>> 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"


Attachments:

  [image/png] Screenshot 2019-11-12 at 10.03.59.png (72.3K, 3-Screenshot%202019-11-12%20at%2010.03.59.png)
  download | view image

  [image/png] Screenshot 2019-11-12 at 09.09.42.png (23.5K, 4-Screenshot%202019-11-12%20at%2009.09.42.png)
  download | view image

  [image/png] image.png (60.7K, 5-image.png)
  download | view image

view thread (31+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [pgAdmin][RM4348] Theme options in pgAdmin and dark theme
  In-Reply-To: <CAM9w-_mfFq2wKZwTvzRaEPre+Shz-ESSEhy3CA-SuFmcvNCAzQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox