Bug#635748: [pgadmin3] Does not respect system background colors in "Object browser"

[Guillaume Lelarge]

2016-09-17 20:30 GMT+02:00 Rimas Kudelis :

> Hi,
>
> 2016-09-17 20:52, Guillaume Lelarge rašė:
> > You're right on the issue. Unfortunately, there won't be any more
> > pgadmin3 releases. The dev team is working on pgadmin4 right now,
> > which is a complete rewrite of pgAdmin. A GA release should be
> > available at the end of September if all goes according to plan.
> >
> > But still, thank you for your patch and your attention to this kind of
> > details. They are important to us. I'd have been happy to apply it if
> > there would be another release of pgadmin3.
> >
>
> Yeah, I saw that pgadmin4 is on the way. However, perhaps it would make
> sense to apply this at least as a Debian patch, since I doubt that
> pgadmin4 will become available in official Debian or Ubuntu repositories
> anytime soon. Or will it?
>
>
No idea. I don't maintain the Debian package.


-- 
Guillaume.
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com

Bug#635748: [pgadmin3] Does not respect system background colors in "Object browser"

[Rimas Kudelis]

Hi,

2016-09-17 20:52, Guillaume Lelarge rašė:
> You're right on the issue. Unfortunately, there won't be any more
> pgadmin3 releases. The dev team is working on pgadmin4 right now,
> which is a complete rewrite of pgAdmin. A GA release should be
> available at the end of September if all goes according to plan.
>
> But still, thank you for your patch and your attention to this kind of
> details. They are important to us. I'd have been happy to apply it if
> there would be another release of pgadmin3.
>

Yeah, I saw that pgadmin4 is on the way. However, perhaps it would make
sense to apply this at least as a Debian patch, since I doubt that
pgadmin4 will become available in official Debian or Ubuntu repositories
anytime soon. Or will it?

Rimas

Bug#635748: [pgadmin3] Does not respect system background colors in "Object browser"

[Rimas Kudelis]

Hi Guillaume,

I just want to say that your patch doesn't quite work as expected. 
Instead of just using system default colour unless custom colour is set, 
Pgadmin actually stores that default colour in server configuration, and 
always sets it explicitly. This isn't a problem until you change the 
theme you use, but once you do (like I switched from light to dark 
theme), all your servers are suddently using a colour that is no longer 
the default.


I think the correct behavior would be not to store colour in the 
configuration unless the user sets it explicitly. This way, even if 
PgAdmin stores the default upon initial creation, the user could clear 
that setting manually and it would then always be treated as empty 
instead of being reset to the default colour on first run. Ideally, it 
would even be possible to disable custom colour via a checkbox or 
something like that.


I made a small patch, which prevents storing the empty setting. However, 
it is incomplete due to the server dialog now erring upon load, because 
it actually expects a colour value. I believe this should be easy to fix 
for someone familiar with the code though.


I'm attaching this half-baked patch. Feel free to apply it if PgAdmin is 
still to be supported for some time.


Cheers,
Rimas Kudelis

--- pgadmin3-1.22.0.orig/pgadmin/include/schema/pgServer.h
+++ pgadmin3-1.22.0/pgadmin/include/schema/pgServer.h
@@ -47,7 +47,7 @@ public:
 	pgServer(const wxString  = wxT(""), const wxString  = wxT(""), const wxString  = wxT(""),
 	 const wxString  = wxT(""), const wxString  = wxT(""), const wxString  = wxT(""), int newPort = 5432,
 	 bool storePwd = false, const wxString  = wxT(""), bool restore = true, int sslMode = 0,
-	 const wxString  = wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOW).GetAsString(wxC2S_HTML_SYNTAX), const wxString  = wxEmptyString,
+	 const wxString  = wxEmptyString, const wxString  = wxEmptyString,
 	 bool sshTunnel = false, const wxString  = wxEmptyString, const wxString  = wxEmptyString, bool authModePwd = true,
 	 const wxString  = wxEmptyString, const wxString  = wxEmptyString, const wxString  = wxEmptyString,
 	 const int  = DEFAULT_SSH_PORT);
--- pgadmin3-1.22.0.orig/pgadmin/schema/pgServer.cpp
+++ pgadmin3-1.22.0/pgadmin/schema/pgServer.cpp
@@ -1556,12 +1556,5 @@ pgObject *pgServerFactory::CreateObjects
 colour = wxEmptyString;
 		}

-		if (colour.IsEmpty())
-		{
-			wxColour cColour;
-			cColour.Set(wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOW).GetAsString(wxC2S_HTML_SYNTAX));
-			colour = cColour.GetAsString(wxC2S_HTML_SYNTAX);
-		}
-
 		// SSL mode
 #ifdef PG_SSL

Bug#635748: [pgadmin3] Does not respect system background colors in "Object browser"

[Guillaume Lelarge]

Le 17 sept. 2016 7:44 PM, "Rimas Kudelis"  a écrit :
>
> Hi Guillaume,
>
> I just want to say that your patch doesn't quite work as expected.
Instead of just using system default colour unless custom colour is set,
Pgadmin actually stores that default colour in server configuration, and
always sets it explicitly. This isn't a problem until you change the theme
you use, but once you do (like I switched from light to dark theme), all
your servers are suddently using a colour that is no longer the default.
>
> I think the correct behavior would be not to store colour in the
configuration unless the user sets it explicitly. This way, even if PgAdmin
stores the default upon initial creation, the user could clear that setting
manually and it would then always be treated as empty instead of being
reset to the default colour on first run. Ideally, it would even be
possible to disable custom colour via a checkbox or something like that.
>
> I made a small patch, which prevents storing the empty setting. However,
it is incomplete due to the server dialog now erring upon load, because it
actually expects a colour value. I believe this should be easy to fix for
someone familiar with the code though.
>
> I'm attaching this half-baked patch. Feel free to apply it if PgAdmin is
still to be supported for some time.
>

You're right on the issue. Unfortunately, there won't be any more pgadmin3
releases. The dev team is working on pgadmin4 right now, which is a
complete rewrite of pgAdmin. A GA release should be available at the end of
September if all goes according to plan.

But still, thank you for your patch and your attention to this kind of
details. They are important to us. I'd have been happy to apply it if there
would be another release of pgadmin3.

Regards.

Bug#635748: [pgadmin3] Does not respect system background colors in "Object browser"

[Guillaume Lelarge]

2016-01-17 14:55 GMT+01:00 Denis Briand :

> On Sun, Jan 17, 2016 at 11:48:51AM +0100, Guillaume Lelarge wrote:
> > Le 17 janv. 2016 10:12 AM, "Denis Briand"  a
> écrit :
> > > Hello Guillaume,
> > >
> > > Is this patch applied?
> > > I don't found entry about that on the changelog.
> > >
> >
> > For a patch to be applied, there needs to be a patch. As far as I know,
> there's
> > no such patch for this feature.
> >
>
> On Sat, 06 Aug 2011 18:33:06 +0200 Guillaume Lelarge <
> guilla...@lelarge.info> wrote:
> > Actually, I just applied the patch for 1.14 branch and the development
> > branch.
>
> That's what you said :)
>
>
Oops, you're right. Commit c559ff958fbe6a049f381714a249a4c382cade07, pushed
in 2011. Which might explain why I didn't remember it :)


-- 
Guillaume.
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com

Bug#635748: [pgadmin3] Does not respect system background colors in "Object browser"

[Denis Briand]

tags 635748 moreinfo
thanks

On Sat, 2011-08-06 at 18:04 +0200, Guillaume Lelarge wrote:
> On Thu, 2011-07-28 at 16:15 +0200, Yuri D'Elia wrote:
> > Package: pgadmin3
> > Version: 1.12.3-1
> > Severity: normal
> > 
> > The object browser doesn't always seem to respect the system
> > background color
> > for the nodes. Apparently, all nodes beyond "Servers" have a white
> > background,
> > while the text color is still taken from the system settings. This
> > looks like
> > a bug.
> > 
> > I'm using a reversed GTK theme, and this is making reading the
> > text almost
> > impossible. I'm attaching a screenshot.
> > 
> 
> By default, the treeview respects the system background color for
> its
> own background color. The server color node (and its subnodes) is
> white
> by default. So you need to change it in the server's properties.
> 
> Rather than default to white for the server background color node,
> we
> should go to the system default background color. This will be done
> in a
> future release.
> 

Hello Guillaume,

Is this patch applied?
I don't found entry about that on the changelog.

regards

Denis Briand


signature.asc
Description: PGP signature

Bug#635748: [pgadmin3] Does not respect system background colors in "Object browser"

[Denis Briand]

On Sun, Jan 17, 2016 at 11:48:51AM +0100, Guillaume Lelarge wrote:
> Le 17 janv. 2016 10:12 AM, "Denis Briand"  a écrit :
> > Hello Guillaume,
> >
> > Is this patch applied?
> > I don't found entry about that on the changelog.
> >
> 
> For a patch to be applied, there needs to be a patch. As far as I know, 
> there's
> no such patch for this feature.
> 

On Sat, 06 Aug 2011 18:33:06 +0200 Guillaume Lelarge  
wrote:
> Actually, I just applied the patch for 1.14 branch and the development
> branch.

That's what you said :)

Denis


signature.asc
Description: Digital signature

Bug#635748: [pgadmin3] Does not respect system background colors in "Object browser"

[Guillaume Lelarge]

Le 17 janv. 2016 10:12 AM, "Denis Briand"  a écrit :
>
> tags 635748 moreinfo
> thanks
>
> On Sat, 2011-08-06 at 18:04 +0200, Guillaume Lelarge wrote:
> > On Thu, 2011-07-28 at 16:15 +0200, Yuri D'Elia wrote:
> > > Package: pgadmin3
> > > Version: 1.12.3-1
> > > Severity: normal
> > >
> > > The object browser doesn't always seem to respect the system
> > > background color
> > > for the nodes. Apparently, all nodes beyond "Servers" have a white
> > > background,
> > > while the text color is still taken from the system settings. This
> > > looks like
> > > a bug.
> > >
> > > I'm using a reversed GTK theme, and this is making reading the
> > > text almost
> > > impossible. I'm attaching a screenshot.
> > >
> >
> > By default, the treeview respects the system background color for
> > its
> > own background color. The server color node (and its subnodes) is
> > white
> > by default. So you need to change it in the server's properties.
> >
> > Rather than default to white for the server background color node,
> > we
> > should go to the system default background color. This will be done
> > in a
> > future release.
> >
>
> Hello Guillaume,
>
> Is this patch applied?
> I don't found entry about that on the changelog.
>

For a patch to be applied, there needs to be a patch. As far as I know,
there's no such patch for this feature.