public inbox for [email protected]  
help / color / mirror / Atom feed
From: Neel Patel <[email protected]>
To: Akshay Joshi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: RM4292 - Dark mode support for Windows/macOS
Date: Mon, 13 Apr 2020 18:37:51 +0530
Message-ID: <CACCA4P1WBjWVV2FnGcCL2Gc-Gay7JyK4pK1wsxexZLqWkZcG6g@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDc0-DtukYJPbt_3oQURW=R4dD__ZRincR3R1=jh4NzD3g@mail.gmail.com>
References: <CA+OCxoyiLYcN0N00nFPT9YCOFo8HT47rcoidccq2N5fDJFuqtQ@mail.gmail.com>
	<CANxoLDdFpBpAg_V5r5M2ni9ycQzr-vhRkeQmMUfqEMYZXq-pZw@mail.gmail.com>
	<CACCA4P1U8iLJWDM4_by0+HC9oPyUrrfArsu522FgWC6+sjjzWA@mail.gmail.com>
	<CA+OCxowhYTCS0g7q=9BVi-Nd0=++3dWugAeZQTRzEVikwSdosA@mail.gmail.com>
	<CACCA4P0L-0PS_8-9i-sa4ECvH16kK6eJSgUcMHP57fTyzBHCBA@mail.gmail.com>
	<CACCA4P2z6SSa_yCq_omQswoxJ4YVtZXjZxQs+dw9Jdo61dLnfQ@mail.gmail.com>
	<CANxoLDdqEgjbmzdqvd=rMygNHqdG4Ey3J1sPh5Zx+JKtixB6MQ@mail.gmail.com>
	<CA+OCxoy__HmGHgF2zK_bqnd24q7ET3imBfsjQCtwYVknHDE_4w@mail.gmail.com>
	<CANxoLDcEZbZzoT_AD3W5A6Bm1w+A39STsS5TcYvT2oTgjFtfHw@mail.gmail.com>
	<CA+OCxowq_aipmgNXT-6ttD1L88stBQp4UbE4jDLsbpmDRC0Jsg@mail.gmail.com>
	<CANxoLDc0-DtukYJPbt_3oQURW=R4dD__ZRincR3R1=jh4NzD3g@mail.gmail.com>

Hi,

I have tried to fix the bundling issue for Windows and Mac. Attached is the
initial patch. Do review it and let me know if I missed anything.

Thanks,
Neel Patel

On Mon, Apr 13, 2020 at 5:08 PM Akshay Joshi <[email protected]>
wrote:

>
>
> On Mon, Apr 13, 2020 at 5:05 PM Dave Page <[email protected]> wrote:
>
>> That's why it's still on my todo, but feel free to fix it if you like :-p
>>
>
>    Currently having too much of review task, once done will fix that.
>
>>
>> On Mon, Apr 13, 2020 at 12:30 PM Akshay Joshi <
>> [email protected]> wrote:
>>
>>> No, I think I missed that.
>>>
>>> On Mon, Apr 13, 2020 at 4:57 PM Dave Page <[email protected]> wrote:
>>>
>>>> Did you fix the Windows installer and macOS appbundle to ship the
>>>> additional required files?
>>>>
>>>> On Mon, Apr 13, 2020 at 9:36 AM Akshay Joshi <
>>>> [email protected]> wrote:
>>>>
>>>>> Thanks, patch applied.
>>>>>
>>>>> On Fri, Apr 10, 2020 at 5:41 PM Neel Patel <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Dave/Akshay,
>>>>>>
>>>>>> I had manually added the theme parameter in registry but after
>>>>>> changing those from the control panel as suggested by Dave, all control
>>>>>> looks fine in dark theme.
>>>>>> Please find the attached patch for review.
>>>>>>
>>>>>> Thanks,
>>>>>> Neel Patel
>>>>>>
>>>>>> On Thu, Apr 9, 2020 at 10:26 PM Neel Patel <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> On Thu, Apr 9, 2020 at 10:19 PM Dave Page <[email protected]> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Thu, Apr 9, 2020 at 2:28 PM Neel Patel <
>>>>>>>> [email protected]> wrote:
>>>>>>>>
>>>>>>>>> Hi Dave,
>>>>>>>>>
>>>>>>>>> I reviewed and tested the code on Windows and fixed below issues.
>>>>>>>>> Except below, it looks good to me.
>>>>>>>>>
>>>>>>>>>    - Compilation error on windows
>>>>>>>>>
>>>>>>>>> Oops, thanks.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>    - In configuration window, "maximumSize" of the control is
>>>>>>>>>    provided along with "minimumSize" so it prevents control from expanding
>>>>>>>>>    when the user resizes the window. Removed maxSize and set as default.
>>>>>>>>>
>>>>>>>>> OK.
>>>>>>>>
>>>>>>>>
>>>>>>>>> A Couple of points for discussion.
>>>>>>>>>
>>>>>>>>>    - AFAIK - Users need to manually set the "AppsUseLightTheme"
>>>>>>>>>    value in the registry, right ? Do we need to document that somewhere ?
>>>>>>>>>
>>>>>>>>> No, it's an option if you go to personalise your desktop:
>>>>>>>>
>>>>>>>
>>>>>>> OK
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> [image: Screenshot 2020-04-09 at 17.45.02.png]
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>    - As we have introduced 2 new "dark.qss" & "light.qss" files,
>>>>>>>>>    so during packaging we need to make sure that it should reside along with
>>>>>>>>>    "pgAdmin4" application binary otherwise those css will not be applied at
>>>>>>>>>    runtime. right ?
>>>>>>>>>
>>>>>>>>> Hmm, that's a good point. I'll look at that.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>    - Checkbox is not visible in Configure window UI as per below
>>>>>>>>>    screenshot. Are you able to see on Mac ? If no, I can take a look at it.
>>>>>>>>>
>>>>>>>>> [image: Screenshot 2020-04-09 at 5.05.25 PM.png]
>>>>>>>>>
>>>>>>>>
>>>>>>>> No, that works on Mac. If you can look at it that would be great
>>>>>>>> (are the SVGs from the patch in your filesystem?):
>>>>>>>>
>>>>>>>
>>>>>>> Yes, all SVGs from the patch are there. I will take a look.
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> [image: Screenshot 2020-04-09 at 17.47.39.png]
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Attached is the updated patch.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Neel Patel
>>>>>>>>>
>>>>>>>>> On Thu, Apr 9, 2020 at 11:12 AM Akshay Joshi <
>>>>>>>>> [email protected]> wrote:
>>>>>>>>>
>>>>>>>>>> Neel,
>>>>>>>>>>
>>>>>>>>>> Can you please review/test it?
>>>>>>>>>>
>>>>>>>>>> On Wed, Apr 8, 2020 at 9:08 PM Dave Page <[email protected]>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> The attached patch detects if dark mode is enabled on Windows
>>>>>>>>>>> (10+)/macOS (10.14+) at server start, and styles the runtime accordingly.
>>>>>>>>>>> It doesn't dynamically switch if the user changes their preferences.
>>>>>>>>>>>
>>>>>>>>>>> Linux builds continue to use the standard styling from Qt.
>>>>>>>>>>>
>>>>>>>>>>> NOTE: I've tested this on macOS, but my Windows build system is
>>>>>>>>>>> playing up at the moment. Akshay, can you have someone on your team test it
>>>>>>>>>>> please? I believe the code should work; it's just a case of reading a reg
>>>>>>>>>>> key and then doing the same thing as on macOS to set the theme accordingly.
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Dave Page
>>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>
>>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> *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
>>>>>>>>
>>>>>>>
>>>>>
>>>>> --
>>>>> *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
>>>>
>>>
>>>
>>> --
>>> *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
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>


Attachments:

  [image/png] Screenshot 2020-04-09 at 5.05.25 PM.png (74.0K, 3-Screenshot%202020-04-09%20at%205.05.25%20PM.png)
  download | view image

  [image/png] Screenshot 2020-04-09 at 17.45.02.png (103.6K, 4-Screenshot%202020-04-09%20at%2017.45.02.png)
  download | view image

  [image/png] Screenshot 2020-04-09 at 17.47.39.png (398.3K, 5-Screenshot%202020-04-09%20at%2017.47.39.png)
  download | view image

  [application/octet-stream] runtime_dark_light_packaging.patch (2.0K, 6-runtime_dark_light_packaging.patch)
  download | inline diff:
diff --git a/Make.bat b/Make.bat
index 11b6f6d82..884778981 100644
--- a/Make.bat
+++ b/Make.bat
@@ -265,6 +265,12 @@ REM Main build sequence Ends
     ECHO Staging pgAdmin4.exe...
     COPY "%WD%\runtime\release\pgAdmin4.exe" "%PGBUILDPATH%\runtime" > nul || EXIT /B 1
 
+    ECHO Staging dark and light theme components...
+    COPY "%WD%\runtime\dark.qss" "%PGBUILDPATH%\runtime" > nul || EXIT /B 1
+    COPY "%WD%\runtime\light.qss" "%PGBUILDPATH%\runtime" > nul || EXIT /B 1
+    XCOPY /S /I /E /H /Y "%WD%\runtime\dark" "%PGBUILDPATH%\runtime\dark" > nul || EXIT /B 1
+    XCOPY /S /I /E /H /Y "%WD%\runtime\light" "%PGBUILDPATH%\runtime\light" > nul || EXIT /B 1
+
     ECHO Staging Qt components...
     COPY "%QTDIR%\bin\Qt5Core.dll"   "%PGBUILDPATH%\runtime" > nul || EXIT /B 1
     COPY "%QTDIR%\bin\Qt5Gui.dll"    "%PGBUILDPATH%\runtime" > nul || EXIT /B 1
diff --git a/pkg/mac/build.sh b/pkg/mac/build.sh
index c8b443b70..0f40c255a 100755
--- a/pkg/mac/build.sh
+++ b/pkg/mac/build.sh
@@ -78,6 +78,7 @@ _get_version() {
 _cleanup() {
     echo "Cleaning up the old environment and app bundle"
     rm -rf ${SOURCEDIR}/runtime/pgAdmin4.app
+    rm -rf ${SOURCEDIR}/runtime/dark ${SOURCEDIR}/runtime/light ${SOURCEDIR}/runtime/light.qss ${SOURCEDIR}/runtime/dark.qss
     rm -rf ${BUILDROOT}
     rm -f ${DISTROOT}/pgadmin4*.dmg
 }
@@ -153,6 +154,7 @@ _build_runtime() {
     ${QMAKE} || { echo qmake failed; exit 1; }
     make || { echo make failed; exit 1; }
     cp -r pgAdmin4.app "${BUILDROOT}/${APP_BUNDLE_NAME}"
+    cp -r dark light dark.qss light.qss "${BUILDROOT}/"
 }
 
 _build_doc() {
diff --git a/pkg/win32/installer.iss.in b/pkg/win32/installer.iss.in
index 7deb925f3..560f00601 100644
--- a/pkg/win32/installer.iss.in
+++ b/pkg/win32/installer.iss.in
@@ -294,6 +294,8 @@ begin
       begin
         DelWebfolder(ExpandConstant('{app}\web'));
         DelFolder(ExpandConstant('{app}\venv'));
+        DelFolder(ExpandConstant('{app}\runtime\dark'));
+        DelFolder(ExpandConstant('{app}\runtime\light'));
       end;
 	end;
   end;


view thread (26+ 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: RM4292 - Dark mode support for Windows/macOS
  In-Reply-To: <CACCA4P1WBjWVV2FnGcCL2Gc-Gay7JyK4pK1wsxexZLqWkZcG6g@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