Обсуждение: [pgadmin-hackers][patch] Column selection on SQLEditor

Поиск
Список
Период
Сортировка

[pgadmin-hackers][patch] Column selection on SQLEditor

От
Joao Pedro De Almeida Pereira
Дата:
Hi Hackers,

Attached you can find the patches that finish the following Redmine Issue

Note:
These changes depend on the acceptance of the patch that we submitted on the email thread: "Refactor: clipboard, translations, jasmine"

This makes some changes and adds testing around the way csv is copied to the clipboard.
We had to write a new plugin to handle selecting columns since slickGrid does not care much about columns.

We also fixed a bug with the csv selection where strings were not being quoted. (it was introduced recently.) Now there are jasmine tests around it.

--
Thanks
Joao & Tira
Вложения

Re: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Dave Page
Дата:
Hi

On Tue, Mar 14, 2017 at 10:08 PM, Joao Pedro De Almeida Pereira
<jdealmeidapereira@pivotal.io> wrote:
> Hi Hackers,
>
> Attached you can find the patches that finish the following Redmine Issue
> https://redmine.postgresql.org/issues/2216
>
> Note:
> These changes depend on the acceptance of the patch that we submitted on the
> email thread: "Refactor: clipboard, translations, jasmine"
>
> This makes some changes and adds testing around the way csv is copied to the
> clipboard.
> We had to write a new plugin to handle selecting columns since slickGrid
> does not care much about columns.
>
> We also fixed a bug with the csv selection where strings were not being
> quoted. (it was introduced recently.) Now there are jasmine tests around it.

This doesn't seem to work as one would expect. Given a test query of
"SELECT * FROM pg_class":

- I select the relnamespace column, hit Copy, and I can paste the
results (as expected).

- I then select the relhasindex column as well. I hit Copy, and when I
paste, I only get the relhasindex values.

- I un-check relhasindex, and check relfilenode. This time when I
paste, I see both relnamespace and relhasindex in the output (as
expected).

- I then un-check the second row; this time when I paste the results,
I only get 239 rows (I expect 1298).

In short, behaviour seems quite unpredictable and somewhat broken.

Some additional comments;

- I wonder if the checkbox should be vertically centered to left of
both the column name and type, rather than just the name. I suspect
that would look better.

- Please don't use brand names (or trademarks etc) in test data :-)

Thanks!

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

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


Re: [patch] Column selection on SQLEditor

От
Atira Odhner
Дата:
This doesn't seem to work as one would expect. Given a test query of
"SELECT * FROM pg_class":

- I select the relnamespace column, hit Copy, and I can paste the
results (as expected).

- I then select the relhasindex column as well. I hit Copy, and when I
paste, I only get the relhasindex values.

- I un-check relhasindex, and check relfilenode. This time when I
paste, I see both relnamespace and relhasindex in the output (as
expected).

- I then un-check the second row; this time when I paste the results,
I only get 239 rows (I expect 1298).

In short, behaviour seems quite unpredictable and somewhat broken.

 
Yep! We fixed issues with this. Note that when select-all is checked, all the other checkboxes are now unchecked: this simplifies the behavior for deselection. For example, if all the checkboxes were checked on select-all, and the user unchecks a column, they end up with all but one column checked. That seems like a weirder use case than just replacing the selection with one column.

Some additional comments; 
- I wonder if the checkbox should be vertically centered to left of
both the column name and type, rather than just the name. I suspect
that would look better.

We're going to work on some styling after this patchset. We're talking about removing the checkboxes and creating a more spreadsheet-like experience.
 
- Please don't use brand names (or trademarks etc) in test data :-)

Yep, removed!


We've attached our WIP patchset -- we will add some commits related to styling on top.
Вложения

Re: [patch] Column selection on SQLEditor

От
Atira Odhner
Дата:
Oops, there  was a test issue we missed while de-branding.

Please look at these instead

On Mon, Apr 3, 2017 at 3:12 PM, Atira Odhner <aodhner@pivotal.io> wrote:
This doesn't seem to work as one would expect. Given a test query of
"SELECT * FROM pg_class":

- I select the relnamespace column, hit Copy, and I can paste the
results (as expected).

- I then select the relhasindex column as well. I hit Copy, and when I
paste, I only get the relhasindex values.

- I un-check relhasindex, and check relfilenode. This time when I
paste, I see both relnamespace and relhasindex in the output (as
expected).

- I then un-check the second row; this time when I paste the results,
I only get 239 rows (I expect 1298).

In short, behaviour seems quite unpredictable and somewhat broken.

 
Yep! We fixed issues with this. Note that when select-all is checked, all the other checkboxes are now unchecked: this simplifies the behavior for deselection. For example, if all the checkboxes were checked on select-all, and the user unchecks a column, they end up with all but one column checked. That seems like a weirder use case than just replacing the selection with one column.

Some additional comments; 
- I wonder if the checkbox should be vertically centered to left of
both the column name and type, rather than just the name. I suspect
that would look better.

We're going to work on some styling after this patchset. We're talking about removing the checkboxes and creating a more spreadsheet-like experience.
 
- Please don't use brand names (or trademarks etc) in test data :-)

Yep, removed!


We've attached our WIP patchset -- we will add some commits related to styling on top.

Вложения

Re: [patch] Column selection on SQLEditor

От
Dave Page
Дата:
Can you send me a squashed version as a single patch please?

On Mon, Apr 3, 2017 at 8:32 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> Oops, there  was a test issue we missed while de-branding.
>
> Please look at these instead
>
> On Mon, Apr 3, 2017 at 3:12 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>>>
>>> This doesn't seem to work as one would expect. Given a test query of
>>> "SELECT * FROM pg_class":
>>>
>>> - I select the relnamespace column, hit Copy, and I can paste the
>>> results (as expected).
>>>
>>> - I then select the relhasindex column as well. I hit Copy, and when I
>>> paste, I only get the relhasindex values.
>>>
>>> - I un-check relhasindex, and check relfilenode. This time when I
>>> paste, I see both relnamespace and relhasindex in the output (as
>>> expected).
>>>
>>> - I then un-check the second row; this time when I paste the results,
>>> I only get 239 rows (I expect 1298).
>>>
>>> In short, behaviour seems quite unpredictable and somewhat broken.
>>>
>>
>> Yep! We fixed issues with this. Note that when select-all is checked, all
>> the other checkboxes are now unchecked: this simplifies the behavior for
>> deselection. For example, if all the checkboxes were checked on select-all,
>> and the user unchecks a column, they end up with all but one column checked.
>> That seems like a weirder use case than just replacing the selection with
>> one column.
>>
>>> Some additional comments;
>>>
>>> - I wonder if the checkbox should be vertically centered to left of
>>> both the column name and type, rather than just the name. I suspect
>>> that would look better.
>>
>>
>> We're going to work on some styling after this patchset. We're talking
>> about removing the checkboxes and creating a more spreadsheet-like
>> experience.
>>
>>>
>>> - Please don't use brand names (or trademarks etc) in test data :-)
>>
>>
>> Yep, removed!
>>
>>
>> We've attached our WIP patchset -- we will add some commits related to
>> styling on top.
>
>



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

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


Re: [patch] Column selection on SQLEditor

От
Atira Odhner
Дата:
Hi Dave,

I still think that smaller commits make it easier to handle git history, but here is a squashed patch. We've updated the styling as well.  Shirley okayed this styling for now but is going to look into updating it in the future--investigating whether we should keep the checkboxes.

Also, as a heads up, I will be rolling off the project after this week. :(

Tira

On Tue, Apr 4, 2017 at 4:33 AM, Dave Page <dpage@pgadmin.org> wrote:
Can you send me a squashed version as a single patch please?

On Mon, Apr 3, 2017 at 8:32 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> Oops, there  was a test issue we missed while de-branding.
>
> Please look at these instead
>
> On Mon, Apr 3, 2017 at 3:12 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>>>
>>> This doesn't seem to work as one would expect. Given a test query of
>>> "SELECT * FROM pg_class":
>>>
>>> - I select the relnamespace column, hit Copy, and I can paste the
>>> results (as expected).
>>>
>>> - I then select the relhasindex column as well. I hit Copy, and when I
>>> paste, I only get the relhasindex values.
>>>
>>> - I un-check relhasindex, and check relfilenode. This time when I
>>> paste, I see both relnamespace and relhasindex in the output (as
>>> expected).
>>>
>>> - I then un-check the second row; this time when I paste the results,
>>> I only get 239 rows (I expect 1298).
>>>
>>> In short, behaviour seems quite unpredictable and somewhat broken.
>>>
>>
>> Yep! We fixed issues with this. Note that when select-all is checked, all
>> the other checkboxes are now unchecked: this simplifies the behavior for
>> deselection. For example, if all the checkboxes were checked on select-all,
>> and the user unchecks a column, they end up with all but one column checked.
>> That seems like a weirder use case than just replacing the selection with
>> one column.
>>
>>> Some additional comments;
>>>
>>> - I wonder if the checkbox should be vertically centered to left of
>>> both the column name and type, rather than just the name. I suspect
>>> that would look better.
>>
>>
>> We're going to work on some styling after this patchset. We're talking
>> about removing the checkboxes and creating a more spreadsheet-like
>> experience.
>>
>>>
>>> - Please don't use brand names (or trademarks etc) in test data :-)
>>
>>
>> Yep, removed!
>>
>>
>> We've attached our WIP patchset -- we will add some commits related to
>> styling on top.
>
>



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

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

Вложения

Re: [patch] Column selection on SQLEditor

От
Atira Odhner
Дата:
We updated the image in the docs and removed an unused css class. Here is an updated patch.

On Tue, Apr 4, 2017 at 11:31 AM, Atira Odhner <aodhner@pivotal.io> wrote:
Hi Dave,

I still think that smaller commits make it easier to handle git history, but here is a squashed patch. We've updated the styling as well.  Shirley okayed this styling for now but is going to look into updating it in the future--investigating whether we should keep the checkboxes.

Also, as a heads up, I will be rolling off the project after this week. :(

Tira

On Tue, Apr 4, 2017 at 4:33 AM, Dave Page <dpage@pgadmin.org> wrote:
Can you send me a squashed version as a single patch please?

On Mon, Apr 3, 2017 at 8:32 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> Oops, there  was a test issue we missed while de-branding.
>
> Please look at these instead
>
> On Mon, Apr 3, 2017 at 3:12 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>>>
>>> This doesn't seem to work as one would expect. Given a test query of
>>> "SELECT * FROM pg_class":
>>>
>>> - I select the relnamespace column, hit Copy, and I can paste the
>>> results (as expected).
>>>
>>> - I then select the relhasindex column as well. I hit Copy, and when I
>>> paste, I only get the relhasindex values.
>>>
>>> - I un-check relhasindex, and check relfilenode. This time when I
>>> paste, I see both relnamespace and relhasindex in the output (as
>>> expected).
>>>
>>> - I then un-check the second row; this time when I paste the results,
>>> I only get 239 rows (I expect 1298).
>>>
>>> In short, behaviour seems quite unpredictable and somewhat broken.
>>>
>>
>> Yep! We fixed issues with this. Note that when select-all is checked, all
>> the other checkboxes are now unchecked: this simplifies the behavior for
>> deselection. For example, if all the checkboxes were checked on select-all,
>> and the user unchecks a column, they end up with all but one column checked.
>> That seems like a weirder use case than just replacing the selection with
>> one column.
>>
>>> Some additional comments;
>>>
>>> - I wonder if the checkbox should be vertically centered to left of
>>> both the column name and type, rather than just the name. I suspect
>>> that would look better.
>>
>>
>> We're going to work on some styling after this patchset. We're talking
>> about removing the checkboxes and creating a more spreadsheet-like
>> experience.
>>
>>>
>>> - Please don't use brand names (or trademarks etc) in test data :-)
>>
>>
>> Yep, removed!
>>
>>
>> We've attached our WIP patchset -- we will add some commits related to
>> styling on top.
>
>



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

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


Вложения

Re: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Dave Page
Дата:
Hi

I was just about to commit this (and was quite please to get it in
before next week's release), but unfortunately realised at the last
minute that it breaks pasting of rows when the query tool is in edit
mode. You should be able to copy 1 or more rows, and then hit the
paste button to append them as un-saved rows (un-saved because you may
need to update values - e.g. serials before saving).

The one tweak I made to the patch was to remove the code that disabled
the Copy button from the top of copy_data.js. I think the button
should remain enabled to allow the user to copy again, in case they
use the clipboard for something else and then need to refresh it with
the data. Of course, it should still be disabled when there is nothing
selected that can be copied.

Any chance this can be fixed before Monday? I would like to include it
in the release if possible.

On Tue, Apr 4, 2017 at 7:16 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> We updated the image in the docs and removed an unused css class. Here is an
> updated patch.
>
> On Tue, Apr 4, 2017 at 11:31 AM, Atira Odhner <aodhner@pivotal.io> wrote:
>>
>> Hi Dave,
>>
>> I still think that smaller commits make it easier to handle git history,
>> but here is a squashed patch. We've updated the styling as well.  Shirley
>> okayed this styling for now but is going to look into updating it in the
>> future--investigating whether we should keep the checkboxes.
>>
>> Also, as a heads up, I will be rolling off the project after this week. :(
>>
>> Tira
>>
>> On Tue, Apr 4, 2017 at 4:33 AM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Can you send me a squashed version as a single patch please?
>>>
>>> On Mon, Apr 3, 2017 at 8:32 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>>> > Oops, there  was a test issue we missed while de-branding.
>>> >
>>> > Please look at these instead
>>> >
>>> > On Mon, Apr 3, 2017 at 3:12 PM, Atira Odhner <aodhner@pivotal.io>
>>> > wrote:
>>> >>>
>>> >>> This doesn't seem to work as one would expect. Given a test query of
>>> >>> "SELECT * FROM pg_class":
>>> >>>
>>> >>> - I select the relnamespace column, hit Copy, and I can paste the
>>> >>> results (as expected).
>>> >>>
>>> >>> - I then select the relhasindex column as well. I hit Copy, and when
>>> >>> I
>>> >>> paste, I only get the relhasindex values.
>>> >>>
>>> >>> - I un-check relhasindex, and check relfilenode. This time when I
>>> >>> paste, I see both relnamespace and relhasindex in the output (as
>>> >>> expected).
>>> >>>
>>> >>> - I then un-check the second row; this time when I paste the results,
>>> >>> I only get 239 rows (I expect 1298).
>>> >>>
>>> >>> In short, behaviour seems quite unpredictable and somewhat broken.
>>> >>>
>>> >>
>>> >> Yep! We fixed issues with this. Note that when select-all is checked,
>>> >> all
>>> >> the other checkboxes are now unchecked: this simplifies the behavior
>>> >> for
>>> >> deselection. For example, if all the checkboxes were checked on
>>> >> select-all,
>>> >> and the user unchecks a column, they end up with all but one column
>>> >> checked.
>>> >> That seems like a weirder use case than just replacing the selection
>>> >> with
>>> >> one column.
>>> >>
>>> >>> Some additional comments;
>>> >>>
>>> >>> - I wonder if the checkbox should be vertically centered to left of
>>> >>> both the column name and type, rather than just the name. I suspect
>>> >>> that would look better.
>>> >>
>>> >>
>>> >> We're going to work on some styling after this patchset. We're talking
>>> >> about removing the checkboxes and creating a more spreadsheet-like
>>> >> experience.
>>> >>
>>> >>>
>>> >>> - Please don't use brand names (or trademarks etc) in test data :-)
>>> >>
>>> >>
>>> >> Yep, removed!
>>> >>
>>> >>
>>> >> We've attached our WIP patchset -- we will add some commits related to
>>> >> styling on top.
>>> >
>>> >
>>>
>>>
>>>
>>> --
>>> 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: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Atira Odhner
Дата:

The one tweak I made to the patch was to remove the code that disabled
the Copy button from the top of copy_data.js. I think the button
should remain enabled to allow the user to copy again, in case they
use the clipboard for something else and then need to refresh it with
the data. Of course, it should still be disabled when there is nothing
selected that can be copied.

Yes, the copy button enablement behavior was a bit strange.  I'm glad you found a fix for it. Do you mind sending us your updated patch?

Any chance this can be fixed before Monday? I would like to include it
in the release if possible

I'll drop a bug at the top of our backlog, and Matt will take a look at it today. We'll let you know at the end of the day where we're at with this fix.

Tira

On Fri, Apr 7, 2017, 5:42 AM Dave Page <dpage@pgadmin.org> wrote:
Hi

I was just about to commit this (and was quite please to get it in
before next week's release), but unfortunately realised at the last
minute that it breaks pasting of rows when the query tool is in edit
mode. You should be able to copy 1 or more rows, and then hit the
paste button to append them as un-saved rows (un-saved because you may
need to update values - e.g. serials before saving).

The one tweak I made to the patch was to remove the code that disabled
the Copy button from the top of copy_data.js. I think the button
should remain enabled to allow the user to copy again, in case they
use the clipboard for something else and then need to refresh it with
the data. Of course, it should still be disabled when there is nothing
selected that can be copied.

Any chance this can be fixed before Monday? I would like to include it
in the release if possible.

On Tue, Apr 4, 2017 at 7:16 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> We updated the image in the docs and removed an unused css class. Here is an
> updated patch.
>
> On Tue, Apr 4, 2017 at 11:31 AM, Atira Odhner <aodhner@pivotal.io> wrote:
>>
>> Hi Dave,
>>
>> I still think that smaller commits make it easier to handle git history,
>> but here is a squashed patch. We've updated the styling as well.  Shirley
>> okayed this styling for now but is going to look into updating it in the
>> future--investigating whether we should keep the checkboxes.
>>
>> Also, as a heads up, I will be rolling off the project after this week. :(
>>
>> Tira
>>
>> On Tue, Apr 4, 2017 at 4:33 AM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Can you send me a squashed version as a single patch please?
>>>
>>> On Mon, Apr 3, 2017 at 8:32 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>>> > Oops, there  was a test issue we missed while de-branding.
>>> >
>>> > Please look at these instead
>>> >
>>> > On Mon, Apr 3, 2017 at 3:12 PM, Atira Odhner <aodhner@pivotal.io>
>>> > wrote:
>>> >>>
>>> >>> This doesn't seem to work as one would expect. Given a test query of
>>> >>> "SELECT * FROM pg_class":
>>> >>>
>>> >>> - I select the relnamespace column, hit Copy, and I can paste the
>>> >>> results (as expected).
>>> >>>
>>> >>> - I then select the relhasindex column as well. I hit Copy, and when
>>> >>> I
>>> >>> paste, I only get the relhasindex values.
>>> >>>
>>> >>> - I un-check relhasindex, and check relfilenode. This time when I
>>> >>> paste, I see both relnamespace and relhasindex in the output (as
>>> >>> expected).
>>> >>>
>>> >>> - I then un-check the second row; this time when I paste the results,
>>> >>> I only get 239 rows (I expect 1298).
>>> >>>
>>> >>> In short, behaviour seems quite unpredictable and somewhat broken.
>>> >>>
>>> >>
>>> >> Yep! We fixed issues with this. Note that when select-all is checked,
>>> >> all
>>> >> the other checkboxes are now unchecked: this simplifies the behavior
>>> >> for
>>> >> deselection. For example, if all the checkboxes were checked on
>>> >> select-all,
>>> >> and the user unchecks a column, they end up with all but one column
>>> >> checked.
>>> >> That seems like a weirder use case than just replacing the selection
>>> >> with
>>> >> one column.
>>> >>
>>> >>> Some additional comments;
>>> >>>
>>> >>> - I wonder if the checkbox should be vertically centered to left of
>>> >>> both the column name and type, rather than just the name. I suspect
>>> >>> that would look better.
>>> >>
>>> >>
>>> >> We're going to work on some styling after this patchset. We're talking
>>> >> about removing the checkboxes and creating a more spreadsheet-like
>>> >> experience.
>>> >>
>>> >>>
>>> >>> - Please don't use brand names (or trademarks etc) in test data :-)
>>> >>
>>> >>
>>> >> Yep, removed!
>>> >>
>>> >>
>>> >> We've attached our WIP patchset -- we will add some commits related to
>>> >> styling on top.
>>> >
>>> >
>>>
>>>
>>>
>>> --
>>> 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: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Dave Page
Дата:
On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>> The one tweak I made to the patch was to remove the code that disabled
>> the Copy button from the top of copy_data.js. I think the button
>> should remain enabled to allow the user to copy again, in case they
>> use the clipboard for something else and then need to refresh it with
>> the data. Of course, it should still be disabled when there is nothing
>> selected that can be copied.
>
> Yes, the copy button enablement behavior was a bit strange.  I'm glad you
> found a fix for it. Do you mind sending us your updated patch?

I literally just removed lines 5 & 6 of copy_data.js.

>> Any chance this can be fixed before Monday? I would like to include it
>> in the release if possible
>
> I'll drop a bug at the top of our backlog, and Matt will take a look at it
> today. We'll let you know at the end of the day where we're at with this
> fix.

Thanks - and sorry to hear your moving onto other things :-(

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

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


Re: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Matthew Kleiman
Дата:
Hi Dave

The attached patch now includes a fix for the regression you found. When the query tool is in edit mode, the user can copy rows and paste them.  I've also removed the two lines from copy_data.js that you mentioned so that users can copy from the table even after they have copied once.

I have noticed that the "Paste Rows" button remains enabled even if only columns are copied. Although pressing this button won't do anything unless rows have been copied to the clipboard, this might prove confusing to users.  I am going to look at disabling this button until rows are in the clipboard. I will let you know if I have anything for you by the end of today. 

Best,
Matt

On Fri, Apr 7, 2017 at 10:27 AM, Dave Page <dpage@pgadmin.org> wrote:
On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>> The one tweak I made to the patch was to remove the code that disabled
>> the Copy button from the top of copy_data.js. I think the button
>> should remain enabled to allow the user to copy again, in case they
>> use the clipboard for something else and then need to refresh it with
>> the data. Of course, it should still be disabled when there is nothing
>> selected that can be copied.
>
> Yes, the copy button enablement behavior was a bit strange.  I'm glad you
> found a fix for it. Do you mind sending us your updated patch?

I literally just removed lines 5 & 6 of copy_data.js.

>> Any chance this can be fixed before Monday? I would like to include it
>> in the release if possible
>
> I'll drop a bug at the top of our backlog, and Matt will take a look at it
> today. We'll let you know at the end of the day where we're at with this
> fix.

Thanks - and sorry to hear your moving onto other things :-(

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

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

Вложения

Re: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Matthew Kleiman
Дата:
Hi Dave,

I've updated the attached patch to include a change to the "Paste Rows" button. It will now be enabled only if there are rows on the clipboard.

Good luck with the release on Monday morning!

Matt

On Fri, Apr 7, 2017 at 4:12 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
Hi Dave

The attached patch now includes a fix for the regression you found. When the query tool is in edit mode, the user can copy rows and paste them.  I've also removed the two lines from copy_data.js that you mentioned so that users can copy from the table even after they have copied once.

I have noticed that the "Paste Rows" button remains enabled even if only columns are copied. Although pressing this button won't do anything unless rows have been copied to the clipboard, this might prove confusing to users.  I am going to look at disabling this button until rows are in the clipboard. I will let you know if I have anything for you by the end of today. 

Best,
Matt

On Fri, Apr 7, 2017 at 10:27 AM, Dave Page <dpage@pgadmin.org> wrote:
On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>> The one tweak I made to the patch was to remove the code that disabled
>> the Copy button from the top of copy_data.js. I think the button
>> should remain enabled to allow the user to copy again, in case they
>> use the clipboard for something else and then need to refresh it with
>> the data. Of course, it should still be disabled when there is nothing
>> selected that can be copied.
>
> Yes, the copy button enablement behavior was a bit strange.  I'm glad you
> found a fix for it. Do you mind sending us your updated patch?

I literally just removed lines 5 & 6 of copy_data.js.

>> Any chance this can be fixed before Monday? I would like to include it
>> in the release if possible
>
> I'll drop a bug at the top of our backlog, and Matt will take a look at it
> today. We'll let you know at the end of the day where we're at with this
> fix.

Thanks - and sorry to hear your moving onto other things :-(

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

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


Вложения

Re: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Dave Page
Дата:
Hi Matt,

On Fri, Apr 7, 2017 at 10:56 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
> Hi Dave,
>
> I've updated the attached patch to include a change to the "Paste Rows"
> button. It will now be enabled only if there are rows on the clipboard.

Unfortunately I found another regression; I have a test table called
موسيقى (Arabic for music) which if included in a selection of rows,
seems to ensure that it and any preceding rows are excluded from the
copy. If I include it in a selection of columns, then things get weird
as the pasted output for that row will sometimes - but not always -
seem to have the column order moved around (maybe because it's an RTL
language).

Obviously the second issue doesn't apply to the current code, but
copying rows does seem to work properly at present, so I'm afraid I
need to bump this back to you again.

> Good luck with the release on Monday morning!

Thanks!

> On Fri, Apr 7, 2017 at 4:12 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
>>
>> Hi Dave
>>
>> The attached patch now includes a fix for the regression you found. When
>> the query tool is in edit mode, the user can copy rows and paste them.  I've
>> also removed the two lines from copy_data.js that you mentioned so that
>> users can copy from the table even after they have copied once.
>>
>> I have noticed that the "Paste Rows" button remains enabled even if only
>> columns are copied. Although pressing this button won't do anything unless
>> rows have been copied to the clipboard, this might prove confusing to users.
>> I am going to look at disabling this button until rows are in the clipboard.
>> I will let you know if I have anything for you by the end of today.
>>
>> Best,
>> Matt
>>
>> On Fri, Apr 7, 2017 at 10:27 AM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>>> >> The one tweak I made to the patch was to remove the code that disabled
>>> >> the Copy button from the top of copy_data.js. I think the button
>>> >> should remain enabled to allow the user to copy again, in case they
>>> >> use the clipboard for something else and then need to refresh it with
>>> >> the data. Of course, it should still be disabled when there is nothing
>>> >> selected that can be copied.
>>> >
>>> > Yes, the copy button enablement behavior was a bit strange.  I'm glad
>>> > you
>>> > found a fix for it. Do you mind sending us your updated patch?
>>>
>>> I literally just removed lines 5 & 6 of copy_data.js.
>>>
>>> >> Any chance this can be fixed before Monday? I would like to include it
>>> >> in the release if possible
>>> >
>>> > I'll drop a bug at the top of our backlog, and Matt will take a look at
>>> > it
>>> > today. We'll let you know at the end of the day where we're at with
>>> > this
>>> > fix.
>>>
>>> Thanks - and sorry to hear your moving onto other things :-(
>>>
>>> --
>>> 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: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Matthew Kleiman
Дата:
Hi Dave,

Unfortunately I found another regression; I have a test table called
موسيقى (Arabic for music) which if included in a selection of rows,
Could you share the queries that:
- create this table
- populate it with data
- access it leading to the regression

What user interaction are you performing to trigger the behavior (is it the query tool or something else)?
It would also help if you could attach a screenshot of the issue.

Thanks,
George & Matt



On Mon, Apr 10, 2017 at 5:52 AM Dave Page <dpage@pgadmin.org> wrote:
Hi Matt,

On Fri, Apr 7, 2017 at 10:56 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
> Hi Dave,
>
> I've updated the attached patch to include a change to the "Paste Rows"
> button. It will now be enabled only if there are rows on the clipboard.

Unfortunately I found another regression; I have a test table called
موسيقى (Arabic for music) which if included in a selection of rows,
seems to ensure that it and any preceding rows are excluded from the
copy. If I include it in a selection of columns, then things get weird
as the pasted output for that row will sometimes - but not always -
seem to have the column order moved around (maybe because it's an RTL
language).

Obviously the second issue doesn't apply to the current code, but
copying rows does seem to work properly at present, so I'm afraid I
need to bump this back to you again.

> Good luck with the release on Monday morning!

Thanks!

> On Fri, Apr 7, 2017 at 4:12 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
>>
>> Hi Dave
>>
>> The attached patch now includes a fix for the regression you found. When
>> the query tool is in edit mode, the user can copy rows and paste them.  I've
>> also removed the two lines from copy_data.js that you mentioned so that
>> users can copy from the table even after they have copied once.
>>
>> I have noticed that the "Paste Rows" button remains enabled even if only
>> columns are copied. Although pressing this button won't do anything unless
>> rows have been copied to the clipboard, this might prove confusing to users.
>> I am going to look at disabling this button until rows are in the clipboard.
>> I will let you know if I have anything for you by the end of today.
>>
>> Best,
>> Matt
>>
>> On Fri, Apr 7, 2017 at 10:27 AM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.io> wrote:
>>> >> The one tweak I made to the patch was to remove the code that disabled
>>> >> the Copy button from the top of copy_data.js. I think the button
>>> >> should remain enabled to allow the user to copy again, in case they
>>> >> use the clipboard for something else and then need to refresh it with
>>> >> the data. Of course, it should still be disabled when there is nothing
>>> >> selected that can be copied.
>>> >
>>> > Yes, the copy button enablement behavior was a bit strange.  I'm glad
>>> > you
>>> > found a fix for it. Do you mind sending us your updated patch?
>>>
>>> I literally just removed lines 5 & 6 of copy_data.js.
>>>
>>> >> Any chance this can be fixed before Monday? I would like to include it
>>> >> in the release if possible
>>> >
>>> > I'll drop a bug at the top of our backlog, and Matt will take a look at
>>> > it
>>> > today. We'll let you know at the end of the day where we're at with
>>> > this
>>> > fix.
>>>
>>> Thanks - and sorry to hear your moving onto other things :-(
>>>
>>> --
>>> 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: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Dave Page
Дата:
Hi

On Mon, Apr 10, 2017 at 4:01 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
> Hi Dave,
>
>> Unfortunately I found another regression; I have a test table called
>> موسيقى (Arabic for music) which if included in a selection of rows,
>
> Could you share the queries that:
> - create this table

CREATE TABLE public."موسيقى"
(
)

> - populate it with data

It doesn't have any - there are no columns in it.

> - access it leading to the regression

SELECT * FROM pg_tables

or

SELECT * FROM pg_class

in the Query Tool.

> What user interaction are you performing to trigger the behavior (is it the
> query tool or something else)?

Run the select query in the query tool, select the rows, then hit Copy.

Note that I think the issue with columns is the editor I was using. If
I paste into vim, I can't reproduce that. The row issue is
consistently reproducible though.

> It would also help if you could attach a screenshot of the issue.

Attached.

>
> Thanks,
> George & Matt
>
>
>
> On Mon, Apr 10, 2017 at 5:52 AM Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi Matt,
>>
>> On Fri, Apr 7, 2017 at 10:56 PM, Matthew Kleiman <mkleiman@pivotal.io>
>> wrote:
>> > Hi Dave,
>> >
>> > I've updated the attached patch to include a change to the "Paste Rows"
>> > button. It will now be enabled only if there are rows on the clipboard.
>>
>> Unfortunately I found another regression; I have a test table called
>> موسيقى (Arabic for music) which if included in a selection of rows,
>> seems to ensure that it and any preceding rows are excluded from the
>> copy. If I include it in a selection of columns, then things get weird
>> as the pasted output for that row will sometimes - but not always -
>> seem to have the column order moved around (maybe because it's an RTL
>> language).
>>
>> Obviously the second issue doesn't apply to the current code, but
>> copying rows does seem to work properly at present, so I'm afraid I
>> need to bump this back to you again.
>>
>> > Good luck with the release on Monday morning!
>>
>> Thanks!
>>
>> > On Fri, Apr 7, 2017 at 4:12 PM, Matthew Kleiman <mkleiman@pivotal.io>
>> > wrote:
>> >>
>> >> Hi Dave
>> >>
>> >> The attached patch now includes a fix for the regression you found.
>> >> When
>> >> the query tool is in edit mode, the user can copy rows and paste them.
>> >> I've
>> >> also removed the two lines from copy_data.js that you mentioned so that
>> >> users can copy from the table even after they have copied once.
>> >>
>> >> I have noticed that the "Paste Rows" button remains enabled even if
>> >> only
>> >> columns are copied. Although pressing this button won't do anything
>> >> unless
>> >> rows have been copied to the clipboard, this might prove confusing to
>> >> users.
>> >> I am going to look at disabling this button until rows are in the
>> >> clipboard.
>> >> I will let you know if I have anything for you by the end of today.
>> >>
>> >> Best,
>> >> Matt
>> >>
>> >> On Fri, Apr 7, 2017 at 10:27 AM, Dave Page <dpage@pgadmin.org> wrote:
>> >>>
>> >>> On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.io>
>> >>> wrote:
>> >>> >> The one tweak I made to the patch was to remove the code that
>> >>> >> disabled
>> >>> >> the Copy button from the top of copy_data.js. I think the button
>> >>> >> should remain enabled to allow the user to copy again, in case they
>> >>> >> use the clipboard for something else and then need to refresh it
>> >>> >> with
>> >>> >> the data. Of course, it should still be disabled when there is
>> >>> >> nothing
>> >>> >> selected that can be copied.
>> >>> >
>> >>> > Yes, the copy button enablement behavior was a bit strange.  I'm
>> >>> > glad
>> >>> > you
>> >>> > found a fix for it. Do you mind sending us your updated patch?
>> >>>
>> >>> I literally just removed lines 5 & 6 of copy_data.js.
>> >>>
>> >>> >> Any chance this can be fixed before Monday? I would like to include
>> >>> >> it
>> >>> >> in the release if possible
>> >>> >
>> >>> > I'll drop a bug at the top of our backlog, and Matt will take a look
>> >>> > at
>> >>> > it
>> >>> > today. We'll let you know at the end of the day where we're at with
>> >>> > this
>> >>> > fix.
>> >>>
>> >>> Thanks - and sorry to hear your moving onto other things :-(
>> >>>
>> >>> --
>> >>> 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

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Вложения

Re: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Matthew Kleiman
Дата:
Hi Dave,

We have found the source of this bug and fixed it in the attached patch. 
If this revised patch addresses the copy issues you pointed out, it can be pulled into master.

Please note that we'll be working on a few more stories around column selection styling, e.g. removal of checkboxes. See email thread with subject "[pgadmin-hackers] [Design Update] Visual design of query editor and results table" for details.
We will change the styling of this patch as part of that work.

Best,
George and Matt






On Mon, Apr 10, 2017 at 11:13 AM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, Apr 10, 2017 at 4:01 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
> Hi Dave,
>
>> Unfortunately I found another regression; I have a test table called
>> موسيقى (Arabic for music) which if included in a selection of rows,
>
> Could you share the queries that:
> - create this table

CREATE TABLE public."موسيقى"
(
)

> - populate it with data

It doesn't have any - there are no columns in it.

> - access it leading to the regression

SELECT * FROM pg_tables

or

SELECT * FROM pg_class

in the Query Tool.

> What user interaction are you performing to trigger the behavior (is it the
> query tool or something else)?

Run the select query in the query tool, select the rows, then hit Copy.

Note that I think the issue with columns is the editor I was using. If
I paste into vim, I can't reproduce that. The row issue is
consistently reproducible though.

> It would also help if you could attach a screenshot of the issue.

Attached.

>
> Thanks,
> George & Matt
>
>
>
> On Mon, Apr 10, 2017 at 5:52 AM Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi Matt,
>>
>> On Fri, Apr 7, 2017 at 10:56 PM, Matthew Kleiman <mkleiman@pivotal.io>
>> wrote:
>> > Hi Dave,
>> >
>> > I've updated the attached patch to include a change to the "Paste Rows"
>> > button. It will now be enabled only if there are rows on the clipboard.
>>
>> Unfortunately I found another regression; I have a test table called
>> موسيقى (Arabic for music) which if included in a selection of rows,
>> seems to ensure that it and any preceding rows are excluded from the
>> copy. If I include it in a selection of columns, then things get weird
>> as the pasted output for that row will sometimes - but not always -
>> seem to have the column order moved around (maybe because it's an RTL
>> language).
>>
>> Obviously the second issue doesn't apply to the current code, but
>> copying rows does seem to work properly at present, so I'm afraid I
>> need to bump this back to you again.
>>
>> > Good luck with the release on Monday morning!
>>
>> Thanks!
>>
>> > On Fri, Apr 7, 2017 at 4:12 PM, Matthew Kleiman <mkleiman@pivotal.io>
>> > wrote:
>> >>
>> >> Hi Dave
>> >>
>> >> The attached patch now includes a fix for the regression you found.
>> >> When
>> >> the query tool is in edit mode, the user can copy rows and paste them.
>> >> I've
>> >> also removed the two lines from copy_data.js that you mentioned so that
>> >> users can copy from the table even after they have copied once.
>> >>
>> >> I have noticed that the "Paste Rows" button remains enabled even if
>> >> only
>> >> columns are copied. Although pressing this button won't do anything
>> >> unless
>> >> rows have been copied to the clipboard, this might prove confusing to
>> >> users.
>> >> I am going to look at disabling this button until rows are in the
>> >> clipboard.
>> >> I will let you know if I have anything for you by the end of today.
>> >>
>> >> Best,
>> >> Matt
>> >>
>> >> On Fri, Apr 7, 2017 at 10:27 AM, Dave Page <dpage@pgadmin.org> wrote:
>> >>>
>> >>> On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.io>
>> >>> wrote:
>> >>> >> The one tweak I made to the patch was to remove the code that
>> >>> >> disabled
>> >>> >> the Copy button from the top of copy_data.js. I think the button
>> >>> >> should remain enabled to allow the user to copy again, in case they
>> >>> >> use the clipboard for something else and then need to refresh it
>> >>> >> with
>> >>> >> the data. Of course, it should still be disabled when there is
>> >>> >> nothing
>> >>> >> selected that can be copied.
>> >>> >
>> >>> > Yes, the copy button enablement behavior was a bit strange.  I'm
>> >>> > glad
>> >>> > you
>> >>> > found a fix for it. Do you mind sending us your updated patch?
>> >>>
>> >>> I literally just removed lines 5 & 6 of copy_data.js.
>> >>>
>> >>> >> Any chance this can be fixed before Monday? I would like to include
>> >>> >> it
>> >>> >> in the release if possible
>> >>> >
>> >>> > I'll drop a bug at the top of our backlog, and Matt will take a look
>> >>> > at
>> >>> > it
>> >>> > today. We'll let you know at the end of the day where we're at with
>> >>> > this
>> >>> > fix.
>> >>>
>> >>> Thanks - and sorry to hear your moving onto other things :-(
>> >>>
>> >>> --
>> >>> 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: [pgadmin-hackers][patch] Column selection on SQLEditor

От
Dave Page
Дата:
Thanks - patch committed!

On Wed, Apr 12, 2017 at 10:00 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
> Hi Dave,
>
> We have found the source of this bug and fixed it in the attached patch.
> If this revised patch addresses the copy issues you pointed out, it can be
> pulled into master.
>
> Please note that we'll be working on a few more stories around column
> selection styling, e.g. removal of checkboxes. See email thread with subject
> "[pgadmin-hackers] [Design Update] Visual design of query editor and results
> table" for details.
> We will change the styling of this patch as part of that work.
>
> Best,
> George and Matt
>
>
>
>
>
>
> On Mon, Apr 10, 2017 at 11:13 AM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Mon, Apr 10, 2017 at 4:01 PM, Matthew Kleiman <mkleiman@pivotal.io>
>> wrote:
>> > Hi Dave,
>> >
>> >> Unfortunately I found another regression; I have a test table called
>> >> موسيقى (Arabic for music) which if included in a selection of rows,
>> >
>> > Could you share the queries that:
>> > - create this table
>>
>> CREATE TABLE public."موسيقى"
>> (
>> )
>>
>> > - populate it with data
>>
>> It doesn't have any - there are no columns in it.
>>
>> > - access it leading to the regression
>>
>> SELECT * FROM pg_tables
>>
>> or
>>
>> SELECT * FROM pg_class
>>
>> in the Query Tool.
>>
>> > What user interaction are you performing to trigger the behavior (is it
>> > the
>> > query tool or something else)?
>>
>> Run the select query in the query tool, select the rows, then hit Copy.
>>
>> Note that I think the issue with columns is the editor I was using. If
>> I paste into vim, I can't reproduce that. The row issue is
>> consistently reproducible though.
>>
>> > It would also help if you could attach a screenshot of the issue.
>>
>> Attached.
>>
>> >
>> > Thanks,
>> > George & Matt
>> >
>> >
>> >
>> > On Mon, Apr 10, 2017 at 5:52 AM Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Hi Matt,
>> >>
>> >> On Fri, Apr 7, 2017 at 10:56 PM, Matthew Kleiman <mkleiman@pivotal.io>
>> >> wrote:
>> >> > Hi Dave,
>> >> >
>> >> > I've updated the attached patch to include a change to the "Paste
>> >> > Rows"
>> >> > button. It will now be enabled only if there are rows on the
>> >> > clipboard.
>> >>
>> >> Unfortunately I found another regression; I have a test table called
>> >> موسيقى (Arabic for music) which if included in a selection of rows,
>> >> seems to ensure that it and any preceding rows are excluded from the
>> >> copy. If I include it in a selection of columns, then things get weird
>> >> as the pasted output for that row will sometimes - but not always -
>> >> seem to have the column order moved around (maybe because it's an RTL
>> >> language).
>> >>
>> >> Obviously the second issue doesn't apply to the current code, but
>> >> copying rows does seem to work properly at present, so I'm afraid I
>> >> need to bump this back to you again.
>> >>
>> >> > Good luck with the release on Monday morning!
>> >>
>> >> Thanks!
>> >>
>> >> > On Fri, Apr 7, 2017 at 4:12 PM, Matthew Kleiman <mkleiman@pivotal.io>
>> >> > wrote:
>> >> >>
>> >> >> Hi Dave
>> >> >>
>> >> >> The attached patch now includes a fix for the regression you found.
>> >> >> When
>> >> >> the query tool is in edit mode, the user can copy rows and paste
>> >> >> them.
>> >> >> I've
>> >> >> also removed the two lines from copy_data.js that you mentioned so
>> >> >> that
>> >> >> users can copy from the table even after they have copied once.
>> >> >>
>> >> >> I have noticed that the "Paste Rows" button remains enabled even if
>> >> >> only
>> >> >> columns are copied. Although pressing this button won't do anything
>> >> >> unless
>> >> >> rows have been copied to the clipboard, this might prove confusing
>> >> >> to
>> >> >> users.
>> >> >> I am going to look at disabling this button until rows are in the
>> >> >> clipboard.
>> >> >> I will let you know if I have anything for you by the end of today.
>> >> >>
>> >> >> Best,
>> >> >> Matt
>> >> >>
>> >> >> On Fri, Apr 7, 2017 at 10:27 AM, Dave Page <dpage@pgadmin.org>
>> >> >> wrote:
>> >> >>>
>> >> >>> On Fri, Apr 7, 2017 at 2:49 PM, Atira Odhner <aodhner@pivotal.io>
>> >> >>> wrote:
>> >> >>> >> The one tweak I made to the patch was to remove the code that
>> >> >>> >> disabled
>> >> >>> >> the Copy button from the top of copy_data.js. I think the button
>> >> >>> >> should remain enabled to allow the user to copy again, in case
>> >> >>> >> they
>> >> >>> >> use the clipboard for something else and then need to refresh it
>> >> >>> >> with
>> >> >>> >> the data. Of course, it should still be disabled when there is
>> >> >>> >> nothing
>> >> >>> >> selected that can be copied.
>> >> >>> >
>> >> >>> > Yes, the copy button enablement behavior was a bit strange.  I'm
>> >> >>> > glad
>> >> >>> > you
>> >> >>> > found a fix for it. Do you mind sending us your updated patch?
>> >> >>>
>> >> >>> I literally just removed lines 5 & 6 of copy_data.js.
>> >> >>>
>> >> >>> >> Any chance this can be fixed before Monday? I would like to
>> >> >>> >> include
>> >> >>> >> it
>> >> >>> >> in the release if possible
>> >> >>> >
>> >> >>> > I'll drop a bug at the top of our backlog, and Matt will take a
>> >> >>> > look
>> >> >>> > at
>> >> >>> > it
>> >> >>> > today. We'll let you know at the end of the day where we're at
>> >> >>> > with
>> >> >>> > this
>> >> >>> > fix.
>> >> >>>
>> >> >>> Thanks - and sorry to hear your moving onto other things :-(
>> >> >>>
>> >> >>> --
>> >> >>> 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
>
>



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

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