date:20171121

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

2017-11-21 Thread Neel Patel

Hi Dave,

On Mon, Nov 20, 2017 at 8:20 PM, Dave Page  wrote:

> Hi
>
> On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel 
> wrote:
>
>> Hi,
>>
>> Please find attached patch to fix RM#2679.
>>
>> *Issue:-*
>> Getting started links does not open second time from "Dashboard" panel if
>> User close runtime tab and open any URL again.
>>
>> *Analysis:-*
>> As in runtime Qt application, when user defined "target=_new" then
>> "createWindow" signal is called but when user close that new windows and
>> again click on link then "createWindow" signal is not getting called so
>> from user point view nothing will happen.
>>
>> *Solution:-*
>> To make it work in both runtime and web application, changed "target"
>> attribute to "_blank" so that "createWindow" signal will be called every
>> time when user click on any link.
>>
>
> I think this is a partial workaround for the problem. We link to external
> sites such as postgresql.org - what happens if that tries to open
> something with target="_new"? It should be expected to work as well. I
> think we need to fix the underlying problem, not try to work around it.
>

Yes. You are right. We can implement the actual underlying problem but
curious to know the difference between "_blank" and "_new" target attribute.

I didn't find any reference document for target attribute value "_new". I
searched below links.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target

https://msdn.microsoft.com/en-us/library/system.web.ui.webcontrols.hyperlink.target(v=vs.110).aspx#Anchor_0
Thoughts ?


>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

2017-11-21 Thread Dave Page

On Tue, Nov 21, 2017 at 9:02 AM, Neel Patel 
wrote:

> Hi Dave,
>
> On Mon, Nov 20, 2017 at 8:20 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel 
>> wrote:
>>
>>> Hi,
>>>
>>> Please find attached patch to fix RM#2679.
>>>
>>> *Issue:-*
>>> Getting started links does not open second time from "Dashboard" panel
>>> if User close runtime tab and open any URL again.
>>>
>>> *Analysis:-*
>>> As in runtime Qt application, when user defined "target=_new" then
>>> "createWindow" signal is called but when user close that new windows and
>>> again click on link then "createWindow" signal is not getting called so
>>> from user point view nothing will happen.
>>>
>>> *Solution:-*
>>> To make it work in both runtime and web application, changed "target"
>>> attribute to "_blank" so that "createWindow" signal will be called every
>>> time when user click on any link.
>>>
>>
>> I think this is a partial workaround for the problem. We link to external
>> sites such as postgresql.org - what happens if that tries to open
>> something with target="_new"? It should be expected to work as well. I
>> think we need to fix the underlying problem, not try to work around it.
>>
>
> Yes. You are right. We can implement the actual underlying problem but
> curious to know the difference between "_blank" and "_new" target attribute.
>
> I didn't find any reference document for target attribute value "_new". I
> searched below links.
>
> https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target
>
> https://msdn.microsoft.com/en-us/library/system.web.ui.
> webcontrols.hyperlink.target(v=vs.110).aspx#Anchor_0
> Thoughts ?
>

Hmm, interesting. So, per the spec we should be using _blank - I'll commit
the patch to fix that.

That means that _new *should* work as any other named target - open it if
it doesn't exist and navigate within it.

In other words, I think we still have a bug here; using _new (much like
using, say, "foo") should still open a new tab if an earlier one has been
closed - much as a browser would.

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][Patch]: Allow user to choose background colour for server

2017-11-21 Thread Dave Page

Hi

On Mon, Nov 20, 2017 at 5:47 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> On Mon, Nov 20, 2017 at 10:40 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Nov 20, 2017 at 4:19 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> PFA updated patch.
>>>
>>
>> OK, that's looking better. I just found a couple of smaller issue this
>> time:
>>
>> - Why can't I select a foreground colour unless I select a non-white
>> background? That seems inconvenient and unnecessary
>>
> Because we can only pass CSS classes
> 
> to aciTree node which it stores in html element and we are passing bg & fg
> colors as additional classes like   ,
> When we fetch the class list from html element, we get array of classes,
> so if we have any value at index 1 then it is bgcolor and we have value at
> index 2 then it is fgcolor, and using these css colors then we create
> dynamic style tag for the acitree node which inject at runtime in DOM.
>
> So we can only identify them by index, so we have disabled the fgcolor
> field if bgcolor field is not provided.
>

Implementation details like this are really not a good reason to confuse
the user. Why can't we just default the background colour to white if
unspecified (which it will be anyway), thus ensuring there is always the
expected number of elements? Then, the user can specify (for example) red
on white - which I suspect would be a popular choice (or variations
thereof).


>
> - If I set a foreground and background colour, and then later set the
>> background to a new colour and the foreground to "no colour", the
>> background change takes effect immediately, but the foreground change
>> requires a refresh (changing to a different foreground colour seems to work
>> fine though).
>>
> Fixed, Older style tag was not cleaning up properly and it was pickking
> up color from previous style tag.
>
>
>>
>>
>>
>>>
>>> On Mon, Nov 20, 2017 at 6:48 PM, Dave Page  wrote:
>>>
 Hi

 On Fri, Nov 17, 2017 at 9:30 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> PFA updated patch.
>
> On Thu, Nov 16, 2017 at 6:34 PM, Dave Page  wrote:
>
>> Hi
>>
>> Looks good. A few changes/suggestions:
>>
>> - There seem to be some debugger calls left in the code, e.g. in
>> editors.js
>>
> Fixed
>
>> - Instead of bgcolor and font_color, lets use bgcolor and fgcolor
>> (foreground).
>>
> Fixed
>
>
>> - The docs are (technically) en_US, so we should use color not colour
>> in them.
>>
> Fixed
>
>
>> - If the colours have been set for a server, I think we should also
>> colour the title bar in the query tool. The only possible problem there 
>> is
>> that the current default colours are reversed in comparison to the 
>> treeview
>> (e.g. the treeview defaults to black text, whilst the query tool title 
>> bar
>> is white on blue. Not sure if that is really an issue or not.
>>
> What do you think?
>>
> I have added logic to set the background & foreground colour in query
> tool & datagrid title bar.
> - Default title bar colours, b
> ackground
> : blue
> foreground
> : white
>  [
> w
> hat we have right now]
> - When user has custom background colour for the server,
> b
> ackground
> : <
> custom background colour
> >
> foreground
> : black
> - When user has custom background colour as well as foreground colour
> for the server,
> b
> ackground
> : <
> custom background colour
> >
> foreground
> :
> <
> custom
> foreground
>  colour
> >
>

 I think this looks good now for the most part, except:

 - The default colours for the title bar on the query tool seem to be
 black on white. See the first screenshot.

>>> Fixed
>>>
>>>

 - If I just set the foreground colour for a connection, it only shows
 up in the query tool title, not on the treeview (and the query tool title
 has a white background (which does seem reasonable, as the treeview would
 as well). See the second screenshot.

>>> Fixed.
>>>
>>>

 --
 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
>>
>
>


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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

2017-11-21 Thread Neel Patel

Hi Dave,


On Tue, Nov 21, 2017 at 2:36 PM, Dave Page  wrote:

>
>
> On Tue, Nov 21, 2017 at 9:02 AM, Neel Patel 
> wrote:
>
>> Hi Dave,
>>
>> On Mon, Nov 20, 2017 at 8:20 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel >> > wrote:
>>>
 Hi,

 Please find attached patch to fix RM#2679.

 *Issue:-*
 Getting started links does not open second time from "Dashboard" panel
 if User close runtime tab and open any URL again.

 *Analysis:-*
 As in runtime Qt application, when user defined "target=_new" then
 "createWindow" signal is called but when user close that new windows and
 again click on link then "createWindow" signal is not getting called so
 from user point view nothing will happen.

 *Solution:-*
 To make it work in both runtime and web application, changed "target"
 attribute to "_blank" so that "createWindow" signal will be called every
 time when user click on any link.

>>>
>>> I think this is a partial workaround for the problem. We link to
>>> external sites such as postgresql.org - what happens if that tries to
>>> open something with target="_new"? It should be expected to work as well. I
>>> think we need to fix the underlying problem, not try to work around it.
>>>
>>
>> Yes. You are right. We can implement the actual underlying problem but
>> curious to know the difference between "_blank" and "_new" target attribute.
>>
>> I didn't find any reference document for target attribute value "_new". I
>> searched below links.
>>
>> https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target
>>
>> https://msdn.microsoft.com/en-us/library/system.web.ui.webco
>> ntrols.hyperlink.target(v=vs.110).aspx#Anchor_0
>> Thoughts ?
>>
>
> Hmm, interesting. So, per the spec we should be using _blank - I'll commit
> the patch to fix that.
>
Yes. We should use _blank instead of _new.

>
> That means that _new *should* work as any other named target - open it if
> it doesn't exist and navigate within it.
>
Yes - As per my knowledge, "target=_new" means URL to be opened in new
frame with name="_new".

>
> In other words, I think we still have a bug here; using _new (much like
> using, say, "foo") should still open a new tab if an earlier one has been
> closed - much as a browser would.
>
As per the docs, we should use "_blank". Do you feel we should fix in
runtime as well for _new, If yes - I can work on that.


>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

pgAdmin 4 commit: _blank is the correct HTML target for anchors, not _n

2017-11-21 Thread Dave Page

_blank is the correct HTML target for anchors, not _new. Fixes #2679.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=5b5363e2c4c0d00bf215aa9f3c5369801fe39ae4
Author: Neel Patel 

Modified Files
--
web/pgadmin/dashboard/templates/dashboard/welcome_dashboard.html | 8 
1 file changed, 4 insertions(+), 4 deletions(-)

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

2017-11-21 Thread Dave Page

Hi

On Tue, Nov 21, 2017 at 9:33 AM, Neel Patel 
wrote:

> Hi Dave,
>
>
> On Tue, Nov 21, 2017 at 2:36 PM, Dave Page  wrote:
>
>>
>>
>> On Tue, Nov 21, 2017 at 9:02 AM, Neel Patel 
>> wrote:
>>
>>> Hi Dave,
>>>
>>> On Mon, Nov 20, 2017 at 8:20 PM, Dave Page  wrote:
>>>
 Hi

 On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel <
 neel.pa...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached patch to fix RM#2679.
>
> *Issue:-*
> Getting started links does not open second time from "Dashboard" panel
> if User close runtime tab and open any URL again.
>
> *Analysis:-*
> As in runtime Qt application, when user defined "target=_new" then
> "createWindow" signal is called but when user close that new windows and
> again click on link then "createWindow" signal is not getting called so
> from user point view nothing will happen.
>
> *Solution:-*
> To make it work in both runtime and web application, changed "target"
> attribute to "_blank" so that "createWindow" signal will be called every
> time when user click on any link.
>

 I think this is a partial workaround for the problem. We link to
 external sites such as postgresql.org - what happens if that tries to
 open something with target="_new"? It should be expected to work as well. I
 think we need to fix the underlying problem, not try to work around it.

>>>
>>> Yes. You are right. We can implement the actual underlying problem but
>>> curious to know the difference between "_blank" and "_new" target attribute.
>>>
>>> I didn't find any reference document for target attribute value "_new".
>>> I searched below links.
>>>
>>> https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target
>>>
>>> https://msdn.microsoft.com/en-us/library/system.web.ui.webco
>>> ntrols.hyperlink.target(v=vs.110).aspx#Anchor_0
>>> Thoughts ?
>>>
>>
>> Hmm, interesting. So, per the spec we should be using _blank - I'll
>> commit the patch to fix that.
>>
> Yes. We should use _blank instead of _new.
>

Committed.


>
>> That means that _new *should* work as any other named target - open it if
>> it doesn't exist and navigate within it.
>>
> Yes - As per my knowledge, "target=_new" means URL to be opened in new
> frame with name="_new".
>

Right - and "_new" could be any string at all. It doesn't have special
meaning like _blank.


>
>> In other words, I think we still have a bug here; using _new (much like
>> using, say, "foo") should still open a new tab if an earlier one has been
>> closed - much as a browser would.
>>
> As per the docs, we should use "_blank". Do you feel we should fix in
> runtime as well for _new, If yes - I can work on that.
>

Right - _blank fixes the initial bug, however _new (or anything else)
should also open a new tab if needed. We may not use that in pgAdmin
itself, but the sites the app links to might.

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

2017-11-21 Thread Dave Page

HI

On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> PFA updated patch with custom tristate boolean editor for SlickGrid to
> make it compatible with Qt runtime.
> I have tested it web mode & also modified the feature test accordingly.
>
> Also thanks to Neel for testing the patch with latest runtime code.
>

Cool - so a few thoughts...

- Can we make the null symbol a question mark?

- I think the feature tests should be enhanced to ensure we verify the
clickthrough sequence of the new control. I don't think we need a new test
- maybe just an enhancement to the existing editor test?

- With a table of the definition shown below, if I add a row with a default
value for the ID, and false, true, null and hit save, then immediately
(without refreshing) try to change the first boolean value to true and hit
save, then I get the following error:

UPDATE public.bools SET
b1 = %(b1)s::boolean WHERE
;
2017-11-21 10:34:57,378: ERROR pgadmin:
Failed to execute query (execute_void) for the server #1 - DB:postgres
(Query-id: 4249364):
Error Message:ERROR:  syntax error at or near ";"
LINE 3: ;


Table:

CREATE TABLE public.bools
(
id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
b1 boolean,
b2 boolean,
b3 boolean,
CONSTRAINT bools_pkey PRIMARY KEY (id)
)

Thanks!

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgAdmin 4 commit: Remove the artificial limit of 4000 characters from t

2017-11-21 Thread Dave Page

Remove the artificial limit of 4000 characters from text areas. Fixes #2877

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=a85538a1fcc6a9c40c3b0f46d111b16e56974819
Author: Murtuza Zabuawala 

Modified Files
--
web/pgadmin/static/js/backform.pgadmin.js | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)

Re: [pgAdmin4][Patch]: Remove limit of 4000 characters from Textarea

2017-11-21 Thread Dave Page

Thanks, patch applied.

On Tue, Nov 21, 2017 at 7:27 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the issue where we have hard coded limit of 4000
> characters in Backform Textarea control, with new implementation we won't
> limit input unless and until maxlength is provided in particular field.
> RM#2877
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: 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

pgAdmin 4 commit: Quote the owner name when creating types. Fixes #2787

2017-11-21 Thread Dave Page

Quote the owner name when creating types. Fixes #2787

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=dd8e2fe8a6cd88421a0aca9cb7abcdd28d2bfbc5
Author: Murtuza Zabuawala 

Modified Files
--
.../databases/schemas/types/templates/type/sql/default/create.sql | 2 +-
.../databases/schemas/types/templates/type/sql/default/update.sql | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

Re: [pgAdmin4][Patch]: Properly quote owner in Type definition

2017-11-21 Thread Dave Page

Thanks, applied.

On Tue, Nov 21, 2017 at 7:50 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA minor patch to fix the issue where owner is not properly quoted
> causing Create/Update type sql to fail.
> RM#2787
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: 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

Fix for issue RM2760

2017-11-21 Thread Harshal Dhumal

Hi,

Please find attached patch to fix RM2760 and RM2867



-- 
*Harshal Dhumal*
*Sr. Software Engineer*

EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/web/pgadmin/misc/file_manager/static/js/file_manager.js b/web/pgadmin/misc/file_manager/static/js/file_manager.js
index a88ed67..fefa1b0 100644
--- a/web/pgadmin/misc/file_manager/static/js/file_manager.js
+++ b/web/pgadmin/misc/file_manager/static/js/file_manager.js
@@ -159,6 +159,7 @@ define('misc.file_manager', [
 $(innerbody).find('*').off();
 innerbody.remove();
 removeTransId(trans_id);
+pgAdmin.Browser.Events.trigger('pgadmin-storage:cancel_btn:storage_dialog');
   }
 },
 build: function() {
@@ -285,6 +286,7 @@ define('misc.file_manager', [
 $(innerbody).find('*').off();
 innerbody.remove();
 removeTransId(trans_id);
+pgAdmin.Browser.Events.trigger('pgadmin-storage:cancel_btn:select_file');
   }
 },
 build: function() {
@@ -409,6 +411,7 @@ define('misc.file_manager', [
 $(innerbody).find('*').off();
 innerbody.remove();
 removeTransId(trans_id);
+pgAdmin.Browser.Events.trigger('pgadmin-storage:cancel_btn:select_folder');
   }
 },
 build: function() {
@@ -632,6 +635,7 @@ define('misc.file_manager', [
 $(innerbody).find('*').off();
 innerbody.remove();
 removeTransId(trans_id);
+pgAdmin.Browser.Events.trigger('pgadmin-storage:cancel_btn:create_file');
   }
 },
 build: function() {
diff --git a/web/pgadmin/static/js/backform.pgadmin.js b/web/pgadmin/static/js/backform.pgadmin.js
index acde87b..7d446fe 100644
--- a/web/pgadmin/static/js/backform.pgadmin.js
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -2163,9 +2163,6 @@
 },
 initialize: function(){
   Backform.InputControl.prototype.initialize.apply(this, arguments);
-
-  // Listen click events of Storage Manager dialog buttons
-  pgAdmin.Browser.Events.on('pgadmin-storage:finish_btn:'+this.field.get('dialog_type'), this.storage_dlg_hander, this);
 },
 template: _.template([
   '<%=label%>',
@@ -2199,15 +2196,30 @@
 
   pgAdmin.FileManager.init();
   pgAdmin.FileManager.show_dialog(params);
+  // Listen click events of Storage Manager dialog buttons
+  this.listen_file_dlg_events();
 },
 storage_dlg_hander: function(value) {
   var field = _.defaults(this.field.toJSON(), this.defaults),
   attrArr = this.field.get("name").split('.'),
   name = attrArr.shift();
 
+  this.remove_file_dlg_event_listeners();
+
   // Set selected value into the model
   this.model.set(name, decodeURI(value));
 },
+storage_close_dlg_hander: function() {
+  this.remove_file_dlg_event_listeners();
+},
+listen_file_dlg_events: function() {
+  pgAdmin.Browser.Events.on('pgadmin-storage:finish_btn:'+this.field.get('dialog_type'), this.storage_dlg_hander, this);
+  pgAdmin.Browser.Events.on('pgadmin-storage:cancel_btn:'+this.field.get('dialog_type'), this.storage_close_dlg_hander, this);
+},
+remove_file_dlg_event_listeners: function() {
+  pgAdmin.Browser.Events.off('pgadmin-storage:finish_btn:'+this.field.get('dialog_type'), this.storage_dlg_hander, this);
+  pgAdmin.Browser.Events.off('pgadmin-storage:cancel_btn:'+this.field.get('dialog_type'), this.storage_close_dlg_hander, this);
+},
 clearInvalid: function() {
   Backform.InputControl.prototype.clearInvalid.apply(this, arguments);
   this.$el.removeClass("pgadmin-file-has-error");

Re: [pgAdmin4][Patch]: Allow user to choose background colour for server

2017-11-21 Thread Khushboo Vashi

On Tue, Nov 21, 2017 at 5:37 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> PFA updated patch with changes suggested by Ashesh.
> Hopefully the last revision :)
>
> +1 :)

>
> On Tue, Nov 21, 2017 at 4:12 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Please find updated patch.
>>
>> On Tue, Nov 21, 2017 at 2:43 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Mon, Nov 20, 2017 at 5:47 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 On Mon, Nov 20, 2017 at 10:40 PM, Dave Page  wrote:

> Hi
>
> On Mon, Nov 20, 2017 at 4:19 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> PFA updated patch.
>>
>
> OK, that's looking better. I just found a couple of smaller issue this
> time:
>
> - Why can't I select a foreground colour unless I select a non-white
> background? That seems inconvenient and unnecessary
>
 Because we can only pass CSS classes
 
 to aciTree node which it stores in html element and we are passing bg & fg
 colors as additional classes like   ,
 When we fetch the class list from html element, we get array of
 classes, so if we have any value at index 1 then it is bgcolor and we have
 value at index 2 then it is fgcolor, and using these css colors then we
 create dynamic style tag for the acitree node which inject at runtime in
 DOM.

 So we can only identify them by index, so we have disabled the fgcolor
 field if bgcolor field is not provided.

>>>
>>> Implementation details like this are really not a good reason to confuse
>>> the user. Why can't we just default the background colour to white if
>>> unspecified (which it will be anyway), thus ensuring there is always the
>>> expected number of elements? Then, the user can specify (for example) red
>>> on white - which I suspect would be a popular choice (or variations
>>> thereof).
>>>
>>>

 - If I set a foreground and background colour, and then later set the
> background to a new colour and the foreground to "no colour", the
> background change takes effect immediately, but the foreground change
> requires a refresh (changing to a different foreground colour seems to 
> work
> fine though).
>
 Fixed, Older style tag was not cleaning up properly and it was
 pickking up color from previous style tag.


>
>
>
>>
>> On Mon, Nov 20, 2017 at 6:48 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Fri, Nov 17, 2017 at 9:30 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 PFA updated patch.

 On Thu, Nov 16, 2017 at 6:34 PM, Dave Page 
 wrote:

> Hi
>
> Looks good. A few changes/suggestions:
>
> - There seem to be some debugger calls left in the code, e.g. in
> editors.js
>
 Fixed

> - Instead of bgcolor and font_color, lets use bgcolor and fgcolor
> (foreground).
>
 Fixed


> - The docs are (technically) en_US, so we should use color not
> colour in them.
>
 Fixed


> - If the colours have been set for a server, I think we should
> also colour the title bar in the query tool. The only possible problem
> there is that the current default colours are reversed in comparison 
> to the
> treeview (e.g. the treeview defaults to black text, whilst the query 
> tool
> title bar is white on blue. Not sure if that is really an issue or 
> not.
>
 What do you think?
>
 I have added logic to set the background & foreground colour in
 query tool & datagrid title bar.
 - Default title bar colours, b
 ackground
 : blue
 foreground
 : white
  [
 w
 hat we have right now]
 - When user has custom background colour for the server,
 b
 ackground
 : <
 custom background colour
 >
 foreground
 : black
 - When user has custom background colour as well as foreground
 colour for the server,
 b
 ackground
 : <
 custom background colour
 >
 foreground
 :
 <
 custom
 foreground
  colour
 >

>>>
>>> I think this looks good now for the most part, except:
>>>
>>> - The default colours for the title bar on the query tool seem to be
>>> black on white. See the first screenshot.
>>>
>> Fixed
>>
>>
>>>
>>> - If I

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

2017-11-21 Thread Murtuza Zabuawala

Hi Dave,

PFA updated patch.

On Tue, Nov 21, 2017 at 4:16 PM, Dave Page  wrote:

> HI
>
> On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> PFA updated patch with custom tristate boolean editor for SlickGrid to
>> make it compatible with Qt runtime.
>> I have tested it web mode & also modified the feature test accordingly.
>>
>> Also thanks to Neel for testing the patch with latest runtime code.
>>
>
> Cool - so a few thoughts...
>
> - Can we make the null symbol a question mark?
>
instead of question mark we will make square
gray color
.

>
> - I think the feature tests should be enhanced to ensure we verify the
> clickthrough sequence of the new control. I don't think we need a new test
> - maybe just an enhancement to the existing editor test?
>
Done


>
> - With a table of the definition shown below, if I add a row with a
> default value for the ID, and false, true, null and hit save, then
> immediately (without refreshing) try to change the first boolean value to
> true and hit save, then I get the following error:
>
> UPDATE public.bools SET
> b1 = %(b1)s::boolean WHERE
> ;
> 2017-11-21 10:34:57,378: ERROR pgadmin:
> Failed to execute query (execute_void) for the server #1 - DB:postgres
> (Query-id: 4249364):
> Error Message:ERROR:  syntax error at or near ";"
> LINE 3: ;
>
>
> Table:
>
> CREATE TABLE public.bools
> (
> id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
> b1 boolean,
> b2 boolean,
> b3 boolean,
> CONSTRAINT bools_pkey PRIMARY KEY (id)
> )
>
> This issue is not related to given editor changes.
I have opened the separate ticket for the issue
https://redmine.postgresql.org/issues/2886

> Thanks!
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/web/pgadmin/feature_tests/test_data.json 
b/web/pgadmin/feature_tests/test_data.json
index ac00b0b..6c53cc8 100644
--- a/web/pgadmin/feature_tests/test_data.json
+++ b/web/pgadmin/feature_tests/test_data.json
@@ -13,13 +13,14 @@
   "10": ["[61,62]", "[61,62]", "json"],
   "11": ["", "true", "bool"],
   "12": ["", "[null]", "bool"],
-  "13": ["", "[null]", "text[]"],
-  "14": ["{}", "{}", "text[]"],
-  "15": ["{data,,'',\"\",\\'\\',\\\"\\\"}", "{data,[null],,,'',\"\"}", 
"text[]"],
-  "16": ["{}", "{}", "int[]"],
-  "17": ["{123,,456}", "{123,[null],456}", "int[]"],
-  "18": ["", "[null]", "boolean[]"],
-  "19": ["{false,,true}", "{false,[null],true}", "boolean[]"]
+  "13": ["", "false", "bool"],
+  "14": ["", "[null]", "text[]"],
+  "15": ["{}", "{}", "text[]"],
+  "16": ["{data,,'',\"\",\\'\\',\\\"\\\"}", "{data,[null],,,'',\"\"}", 
"text[]"],
+  "17": ["{}", "{}", "int[]"],
+  "18": ["{123,,456}", "{123,[null],456}", "int[]"],
+  "19": ["", "[null]", "boolean[]"],
+  "20": ["{false,,true}", "{false,[null],true}", "boolean[]"]
 }
   }
 }
diff --git a/web/pgadmin/feature_tests/view_data_dml_queries.py 
b/web/pgadmin/feature_tests/view_data_dml_queries.py
index 153d796..e5b0c09 100644
--- a/web/pgadmin/feature_tests/view_data_dml_queries.py
+++ b/web/pgadmin/feature_tests/view_data_dml_queries.py
@@ -65,8 +65,9 @@ CREATE TABLE public.defaults
 text_null4 text COLLATE pg_catalog."default",
 json_defaults json DEFAULT '[51, 52]'::json,
 json_null json,
-boolean_defaults boolean DEFAULT true,
+boolean_true boolean DEFAULT true,
 boolean_null boolean,
+boolean_false boolean,
 text_arr text[],
 text_arr_empty text[],
 text_arr_null text[],
@@ -194,12 +195,17 @@ CREATE TABLE public.defaults
 self.page.find_by_xpath("//*[contains(@class, 'pg_text_editor')]"
 "//button[contains(@class, 
'fa-save')]").click()
 else:
+# Boolean editor test for to True click
 if data[1] == 'true':
-checkbox_el = cell_el.find_element_by_xpath(".//input")
+checkbox_el = 
cell_el.find_element_by_xpath(".//*[contains(@class, 'multi-checkbox')]")
 checkbox_el.click()
-
ActionChains(self.driver).move_to_element(checkbox_el).double_click(
-checkbox_el
-).perform()
+# Boolean editor test for to False click
+elif data[1] == 'false':
+checkbox_el = 
cell_el.find_element_by_xpath(".//*[contains(@class, 'multi-checkbox')]")
+# Sets true
+checkbox_el.click()
+# Sets false
+ActionChains(self.driver).click(checkbox_el).perform()
 
 def _tables_node_expandable(self):
 self.page.toggle_open_tree_item(self.server['name'])
diff --git a/web/pgadmin/static/css/bootstrap.overrides.css 
b/web/pgadmin/static/css/bootstrap.overrides.css
index 73b4fc7..0ea676f 100755
--- a/web/pgadmin/stat

Re: Fix for issue RM2760

2017-11-21 Thread Dave Page

Thanks, applied.

On Tue, Nov 21, 2017 at 12:06 PM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached patch to fix RM2760 and RM2867
>
>
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: 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

pgAdmin 4 commit: When selecting an SSL cert or key, update only the ex

2017-11-21 Thread Dave Page

When selecting an SSL cert or key, update only the expected path in the UI, not 
all of them. Fixes #2760. Fixes #2867

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=921269993645205c1667ab116e4c4201cacf55df
Author: Harshal Dhumal 

Modified Files
--
.../misc/file_manager/static/js/file_manager.js|  4 
web/pgadmin/static/js/backform.pgadmin.js  | 18 +++---
2 files changed, 19 insertions(+), 3 deletions(-)

pgAdmin 4 commit: Allow connections to be coloured in the treeview and

2017-11-21 Thread Dave Page

Allow connections to be coloured in the treeview and query tool. Fixes #1383. 
Fixes #2802

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=b284572afee33196aa4b41adaf33a399e985b4b7
Author: Murtuza Zabuawala 

Modified Files
--
docs/en_US/images/server_general.png   | Bin 39091 -> 105252 bytes
docs/en_US/server_dialog.rst   |   2 +
libraries.txt  |   1 +
web/migrations/versions/02b9dccdcfcb_.py   |  35 
web/package.json   |   1 +
.../browser/server_groups/servers/__init__.py  |  67 ++
.../server_groups/servers/static/js/server.js  |  24 +
web/pgadmin/browser/static/js/browser.js   |  10 ++-
web/pgadmin/browser/static/js/node.js  |  44 ++
web/pgadmin/model/__init__.py  |   2 +
web/pgadmin/static/css/aci_tree.overrides.css  |  12 +++
web/pgadmin/static/css/bootstrap.overrides.css |  63 +++--
web/pgadmin/static/css/style.css   |   1 +
web/pgadmin/static/js/backform.pgadmin.js  |  97 -
web/pgadmin/tools/datagrid/__init__.py |  23 -
.../tools/datagrid/templates/datagrid/index.html   |   2 +-
.../tools/sqleditor/static/css/sqleditor.css   |   2 -
web/webpack.shim.js|   5 ++
web/yarn.lock  |   4 +
19 files changed, 365 insertions(+), 30 deletions(-)

Re: [pgAdmin4][Patch]: Allow user to choose background colour for server

2017-11-21 Thread Dave Page

Thanks! Patch applied, with a minor change to default the query tool header
to a white background if a custom foreground colour is specified (otherwise
you'd get blue).

On Tue, Nov 21, 2017 at 12:07 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> PFA updated patch with changes suggested by Ashesh.
> Hopefully the last revision :)
>
>
> On Tue, Nov 21, 2017 at 4:12 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Please find updated patch.
>>
>> On Tue, Nov 21, 2017 at 2:43 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Mon, Nov 20, 2017 at 5:47 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 On Mon, Nov 20, 2017 at 10:40 PM, Dave Page  wrote:

> Hi
>
> On Mon, Nov 20, 2017 at 4:19 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> PFA updated patch.
>>
>
> OK, that's looking better. I just found a couple of smaller issue this
> time:
>
> - Why can't I select a foreground colour unless I select a non-white
> background? That seems inconvenient and unnecessary
>
 Because we can only pass CSS classes
 
 to aciTree node which it stores in html element and we are passing bg & fg
 colors as additional classes like   ,
 When we fetch the class list from html element, we get array of
 classes, so if we have any value at index 1 then it is bgcolor and we have
 value at index 2 then it is fgcolor, and using these css colors then we
 create dynamic style tag for the acitree node which inject at runtime in
 DOM.

 So we can only identify them by index, so we have disabled the fgcolor
 field if bgcolor field is not provided.

>>>
>>> Implementation details like this are really not a good reason to confuse
>>> the user. Why can't we just default the background colour to white if
>>> unspecified (which it will be anyway), thus ensuring there is always the
>>> expected number of elements? Then, the user can specify (for example) red
>>> on white - which I suspect would be a popular choice (or variations
>>> thereof).
>>>
>>>

 - If I set a foreground and background colour, and then later set the
> background to a new colour and the foreground to "no colour", the
> background change takes effect immediately, but the foreground change
> requires a refresh (changing to a different foreground colour seems to 
> work
> fine though).
>
 Fixed, Older style tag was not cleaning up properly and it was
 pickking up color from previous style tag.


>
>
>
>>
>> On Mon, Nov 20, 2017 at 6:48 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Fri, Nov 17, 2017 at 9:30 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 PFA updated patch.

 On Thu, Nov 16, 2017 at 6:34 PM, Dave Page 
 wrote:

> Hi
>
> Looks good. A few changes/suggestions:
>
> - There seem to be some debugger calls left in the code, e.g. in
> editors.js
>
 Fixed

> - Instead of bgcolor and font_color, lets use bgcolor and fgcolor
> (foreground).
>
 Fixed


> - The docs are (technically) en_US, so we should use color not
> colour in them.
>
 Fixed


> - If the colours have been set for a server, I think we should
> also colour the title bar in the query tool. The only possible problem
> there is that the current default colours are reversed in comparison 
> to the
> treeview (e.g. the treeview defaults to black text, whilst the query 
> tool
> title bar is white on blue. Not sure if that is really an issue or 
> not.
>
 What do you think?
>
 I have added logic to set the background & foreground colour in
 query tool & datagrid title bar.
 - Default title bar colours, b
 ackground
 : blue
 foreground
 : white
  [
 w
 hat we have right now]
 - When user has custom background colour for the server,
 b
 ackground
 : <
 custom background colour
 >
 foreground
 : black
 - When user has custom background colour as well as foreground
 colour for the server,
 b
 ackground
 : <
 custom background colour
 >
 foreground
 :
 <
 custom
 foreground
  colour
 >

>>>
>>> I think this looks good now for the most part, except:
>>>
>>> - The default co

pgAdmin 4 commit: Some browsers don't properly support tri-state checkb

2017-11-21 Thread Dave Page

Some browsers don't properly support tri-state checkboxes, so create our own 
control to handle true/false/null. Fixes #2848

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=930dd8af1fac500749c7a9efa2302b5f3f52b4d4
Author: Murtuza Zabuawala 

Modified Files
--
web/pgadmin/feature_tests/test_data.json   |  15 +--
web/pgadmin/feature_tests/view_data_dml_queries.py |  16 ++-
web/pgadmin/static/css/bootstrap.overrides.css |  28 +
web/pgadmin/static/js/slickgrid/editors.js | 127 ++---
4 files changed, 157 insertions(+), 29 deletions(-)

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

2017-11-21 Thread Dave Page

Hi

On Tue, Nov 21, 2017 at 1:17 PM, Murtuza Zabuawala  wrote:

> Hi Dave,
>
> PFA updated patch.
>
> On Tue, Nov 21, 2017 at 4:16 PM, Dave Page  wrote:
>
>> HI
>>
>> On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> PFA updated patch with custom tristate boolean editor for SlickGrid to
>>> make it compatible with Qt runtime.
>>> I have tested it web mode & also modified the feature test accordingly.
>>>
>>> Also thanks to Neel for testing the patch with latest runtime code.
>>>
>>
>> Cool - so a few thoughts...
>>
>> - Can we make the null symbol a question mark?
>>
> instead of question mark we will make square
> gray color
> .
>

Sorry - I just realised this is not good from an accessibility perspective
as users may not be able to distinguish between the white and gray in-fill.
I've made it include a ? as well.


>
>> - I think the feature tests should be enhanced to ensure we verify the
>> clickthrough sequence of the new control. I don't think we need a new test
>> - maybe just an enhancement to the existing editor test?
>>
> Done
>
>

Thanks - patch applied.


>
>> - With a table of the definition shown below, if I add a row with a
>> default value for the ID, and false, true, null and hit save, then
>> immediately (without refreshing) try to change the first boolean value to
>> true and hit save, then I get the following error:
>>
>> UPDATE public.bools SET
>> b1 = %(b1)s::boolean WHERE
>> ;
>> 2017-11-21 10:34:57,378: ERROR pgadmin:
>> Failed to execute query (execute_void) for the server #1 - DB:postgres
>> (Query-id: 4249364):
>> Error Message:ERROR:  syntax error at or near ";"
>> LINE 3: ;
>>
>>
>> Table:
>>
>> CREATE TABLE public.bools
>> (
>> id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
>> b1 boolean,
>> b2 boolean,
>> b3 boolean,
>> CONSTRAINT bools_pkey PRIMARY KEY (id)
>> )
>>
>> This issue is not related to given editor changes.
> I have opened the separate ticket for the issue
> https://redmine.postgresql.org/issues/2886
>

OK, thanks.


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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgAdmin 4 commit: Fix a yarn warning: "warning From Yarn 1.0 onwards, s

2017-11-21 Thread Dave Page

Fix a yarn warning: "warning From Yarn 1.0 onwards, scripts don't require "--" 
for options to be forwarded. In a future version, any explicit "--" will be 
forwarded as-is to the scripts."

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=8880826260750ae34e99cc51928d30818b12570f

Modified Files
--
web/package.json | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

2017-11-21 Thread Murtuza Zabuawala

Hi Dave,

On Tue, Nov 21, 2017 at 10:53 PM, Dave Page  wrote:

> Hi
>
> On Tue, Nov 21, 2017 at 1:17 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> PFA updated patch.
>>
>> On Tue, Nov 21, 2017 at 4:16 PM, Dave Page  wrote:
>>
>>> HI
>>>
>>> On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 PFA updated patch with custom tristate boolean editor for SlickGrid to
 make it compatible with Qt runtime.
 I have tested it web mode & also modified the feature test accordingly.

 Also thanks to Neel for testing the patch with latest runtime code.

>>>
>>> Cool - so a few thoughts...
>>>
>>> - Can we make the null symbol a question mark?
>>>
>> instead of question mark we will make square
>> gray color
>> .
>>
>
> Sorry - I just realised this is not good from an accessibility perspective
> as users may not be able to distinguish between the white and gray in-fill.
> I've made it include a ? as well.
>

I have enhanced the the visibility of '?'
little bit
more
, attaching the patch if you prefer it that way. 



>
>
>>
>>> - I think the feature tests should be enhanced to ensure we verify the
>>> clickthrough sequence of the new control. I don't think we need a new test
>>> - maybe just an enhancement to the existing editor test?
>>>
>> Done
>>
>>
>
> Thanks - patch applied.
>
>
>>
>>> - With a table of the definition shown below, if I add a row with a
>>> default value for the ID, and false, true, null and hit save, then
>>> immediately (without refreshing) try to change the first boolean value to
>>> true and hit save, then I get the following error:
>>>
>>> UPDATE public.bools SET
>>> b1 = %(b1)s::boolean WHERE
>>> ;
>>> 2017-11-21 10:34:57,378: ERROR pgadmin:
>>> Failed to execute query (execute_void) for the server #1 - DB:postgres
>>> (Query-id: 4249364):
>>> Error Message:ERROR:  syntax error at or near ";"
>>> LINE 3: ;
>>>
>>>
>>> Table:
>>>
>>> CREATE TABLE public.bools
>>> (
>>> id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
>>> b1 boolean,
>>> b2 boolean,
>>> b3 boolean,
>>> CONSTRAINT bools_pkey PRIMARY KEY (id)
>>> )
>>>
>>> This issue is not related to given editor changes.
>> I have opened the separate ticket for the issue
>> https://redmine.postgresql.org/issues/2886
>>
>
> OK, thanks.
>
Also attached patch for RM#2886, the issue was due to incorrect
conditional logic, As per the current implementation the newly added row
should gets disabled if it is saved with default primary key value until
gird reloaded with actual PK for updation but due to
 incorrect condition
 it was enable in this case.

>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/web/pgadmin/static/css/bootstrap.overrides.css 
b/web/pgadmin/static/css/bootstrap.overrides.css
index fcc030d..b894021 100755
--- a/web/pgadmin/static/css/bootstrap.overrides.css
+++ b/web/pgadmin/static/css/bootstrap.overrides.css
@@ -1404,20 +1404,21 @@ body {
 .multi-checkbox .check {
   display: inline-block;
   vertical-align: top;
-  width: 15px;
-  height: 15px;
+  width: 16px;
+  height: 16px;
   border: 1px solid #333;
   margin: 3px;
   text-align: center;
-  line-height: 15px;
+  line-height: 16px;
 }
 
+.multi-checkbox .check.checked,
 .multi-checkbox .check.unchecked {
   background: #fff;
 }
 
 .multi-checkbox .check.partial {
-  background: #cc;
+  background: #e8e8e8;
 }
 
 .multi-checkbox .check.checked:after {
@@ -1425,5 +1426,5 @@ body {
 }
 
 .multi-checkbox .check.partial:after {
-  content: "\fe56";
+  content: "\003F";
 }
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js 
b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index 9a0c3de..ba0ce67 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -668,12 +668,14 @@ define('tools.querytool', [
 }
   }
 }
+
 // Disable rows having default values
 if (!_.isUndefined(self.handler.rows_to_disable) &&
-  _.indexOf(self.handler.rows_to_disable, i) !== -1
-) {
+self.handler.rows_to_disable.length > 0 &&
+_.indexOf(self.handler.rows_to_disable, i) !== -1) {
   cssClass += ' disabled_row';
 }
+
 return {'cssClasses': cssClass};
   };
 
@@ -825,16 +827,17 @@ define('tools.querytool', [
 // so that cell edit is enabled for that row.
 var grid = args.grid,
   row_data = grid.getDataItem(args.row),
-  is_primary_key = _.all(
-_.values(
-  _.pick(
-row_data, self.primary_keys
-  )
-),
-function (val) {
-  return val != undefined
-}
-  );
+  is_pri

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

2017-11-21 Thread Ashesh Vashi

On Wed, Nov 22, 2017 at 11:00 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Tue, Nov 21, 2017 at 10:53 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Tue, Nov 21, 2017 at 1:17 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> PFA updated patch.
>>>
>>> On Tue, Nov 21, 2017 at 4:16 PM, Dave Page  wrote:
>>>
 HI

 On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> PFA updated patch with custom tristate boolean editor for SlickGrid to
> make it compatible with Qt runtime.
> I have tested it web mode & also modified the feature test accordingly.
>
> Also thanks to Neel for testing the patch with latest runtime code.
>

 Cool - so a few thoughts...

 - Can we make the null symbol a question mark?

>>> instead of question mark we will make square
>>> gray color
>>> .
>>>
>>
>> Sorry - I just realised this is not good from an accessibility
>> perspective as users may not be able to distinguish between the white and
>> gray in-fill. I've made it include a ? as well.
>>
> 
> I have enhanced the the visibility of '?'
> little bit
> more
> , attaching the patch if you prefer it that way.
>

In the call, Dave suggested that, we should have:
Cross (×, i.e. ×) for False
Check (√, i.e. √) for True
Empty (∅, i.e. ∅) for Null.

We discussed it, which I think - Murtuza missed due to connectivity
issue, during the call.

Dave - thoughts?

-- Thanks, Ashesh

>
>

>
>>
>>
>>>
 - I think the feature tests should be enhanced to ensure we verify the
 clickthrough sequence of the new control. I don't think we need a new test
 - maybe just an enhancement to the existing editor test?

>>> Done
>>>
>>>
>>
>> Thanks - patch applied.
>>
>>
>>>
 - With a table of the definition shown below, if I add a row with a
 default value for the ID, and false, true, null and hit save, then
 immediately (without refreshing) try to change the first boolean value to
 true and hit save, then I get the following error:

 UPDATE public.bools SET
 b1 = %(b1)s::boolean WHERE
 ;
 2017-11-21 10:34:57,378: ERROR pgadmin:
 Failed to execute query (execute_void) for the server #1 - DB:postgres
 (Query-id: 4249364):
 Error Message:ERROR:  syntax error at or near ";"
 LINE 3: ;


 Table:

 CREATE TABLE public.bools
 (
 id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
 b1 boolean,
 b2 boolean,
 b3 boolean,
 CONSTRAINT bools_pkey PRIMARY KEY (id)
 )

 This issue is not related to given editor changes.
>>> I have opened the separate ticket for the issue
>>> https://redmine.postgresql.org/issues/2886
>>>
>>
>> OK, thanks.
>>
> Also attached patch for RM#2886, the issue was due to incorrect
> conditional logic, As per the current implementation the newly added row
> should gets disabled if it is saved with default primary key value until
> gird reloaded with actual PK for updation but due to
>  incorrect condition
>  it was enable in this case.
>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

2017-11-21 Thread Neel Patel

Hi Dave,

On Tue, Nov 21, 2017 at 3:52 PM, Dave Page  wrote:

> Hi
>
> On Tue, Nov 21, 2017 at 9:33 AM, Neel Patel 
> wrote:
>
>> Hi Dave,
>>
>>
>> On Tue, Nov 21, 2017 at 2:36 PM, Dave Page  wrote:
>>
>>>
>>>
>>> On Tue, Nov 21, 2017 at 9:02 AM, Neel Patel >> > wrote:
>>>
 Hi Dave,

 On Mon, Nov 20, 2017 at 8:20 PM, Dave Page  wrote:

> Hi
>
> On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel <
> neel.pa...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find attached patch to fix RM#2679.
>>
>> *Issue:-*
>> Getting started links does not open second time from "Dashboard"
>> panel if User close runtime tab and open any URL again.
>>
>> *Analysis:-*
>> As in runtime Qt application, when user defined "target=_new" then
>> "createWindow" signal is called but when user close that new windows and
>> again click on link then "createWindow" signal is not getting called so
>> from user point view nothing will happen.
>>
>> *Solution:-*
>> To make it work in both runtime and web application, changed "target"
>> attribute to "_blank" so that "createWindow" signal will be called every
>> time when user click on any link.
>>
>
> I think this is a partial workaround for the problem. We link to
> external sites such as postgresql.org - what happens if that tries to
> open something with target="_new"? It should be expected to work as well. 
> I
> think we need to fix the underlying problem, not try to work around it.
>

 Yes. You are right. We can implement the actual underlying problem but
 curious to know the difference between "_blank" and "_new" target 
 attribute.

 I didn't find any reference document for target attribute value "_new".
 I searched below links.

 https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target

 https://msdn.microsoft.com/en-us/library/system.web.ui.webco
 ntrols.hyperlink.target(v=vs.110).aspx#Anchor_0
 Thoughts ?

>>>
>>> Hmm, interesting. So, per the spec we should be using _blank - I'll
>>> commit the patch to fix that.
>>>
>> Yes. We should use _blank instead of _new.
>>
>
> Committed.
>
>
>>
>>> That means that _new *should* work as any other named target - open it
>>> if it doesn't exist and navigate within it.
>>>
>> Yes - As per my knowledge, "target=_new" means URL to be opened in new
>> frame with name="_new".
>>
>
> Right - and "_new" could be any string at all. It doesn't have special
> meaning like _blank.
>
>
>>
>>> In other words, I think we still have a bug here; using _new (much like
>>> using, say, "foo") should still open a new tab if an earlier one has been
>>> closed - much as a browser would.
>>>
>> As per the docs, we should use "_blank". Do you feel we should fix in
>> runtime as well for _new, If yes - I can work on that.
>>
>
> Right - _blank fixes the initial bug, however _new (or anything else)
> should also open a new tab if needed. We may not use that in pgAdmin
> itself, but the sites the app links to might.
>

Ok. I will work on to fix this issue.


>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

[pgAdmin4][Patch]: To display leading whitespace properly in columns - Query tool

2017-11-21 Thread Murtuza Zabuawala

Hi,

PFA minor patch to fix the issue where user were not able to see leading
whitespace in their columns.
RM#2880

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/web/pgadmin/tools/sqleditor/static/css/sqleditor.css 
b/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
index 8a8cb5e..8f1ab44 100644
--- a/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
+++ b/web/pgadmin/tools/sqleditor/static/css/sqleditor.css
@@ -521,3 +521,7 @@ input.editor-checkbox:focus {
 .pg_buttons {
   text-align:right;
 }
+
+#datagrid .slick-row .slick-cell {
+  white-space: pre;
+}

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

Re: [pgAdmin4][Patch]: Allow user to choose background colour for server

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

pgAdmin 4 commit: _blank is the correct HTML target for anchors, not _n

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

pgAdmin 4 commit: Remove the artificial limit of 4000 characters from t

Re: [pgAdmin4][Patch]: Remove limit of 4000 characters from Textarea

pgAdmin 4 commit: Quote the owner name when creating types. Fixes #2787

Re: [pgAdmin4][Patch]: Properly quote owner in Type definition

Fix for issue RM2760

Re: [pgAdmin4][Patch]: Allow user to choose background colour for server

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

Re: Fix for issue RM2760

pgAdmin 4 commit: When selecting an SSL cert or key, update only the ex

pgAdmin 4 commit: Allow connections to be coloured in the treeview and

Re: [pgAdmin4][Patch]: Allow user to choose background colour for server

pgAdmin 4 commit: Some browsers don't properly support tri-state checkb

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

pgAdmin 4 commit: Fix a yarn warning: "warning From Yarn 1.0 onwards, s

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

Re: [pgAdmin4][runtime][patch]: Fix for RM#2679

[pgAdmin4][Patch]: To display leading whitespace properly in columns - Query tool

25 matches

Site Navigation

Mail list logo

Footer information