Re: [patch] Column selection on SQLEditor

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(at)pgadmin(dot)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(at)pivotal(dot)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(at)pivotal(dot)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(at)pgadmin(dot)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(at)pivotal(dot)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(at)pivotal(dot)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
>