Re: [patch] Column selection on SQLEditor

From: Atira Odhner <aodhner(at)pivotal(dot)io>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Sarah McAlear <smcalear(at)pivotal(dot)io>
Subject: Re: [patch] Column selection on SQLEditor
Date: 2017-04-04 18:16:13
Message-ID: CA+Vc24rzkZ8nn2vSXnTpobv=A0Hgpe7Eo9dByextGpTU5VW7Lw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

Attachment Content-Type Size
0001-Allow-selecting-columns-in-query-output.patch application/octet-stream 188.9 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Matthew Kleiman 2017-04-04 19:00:12 Re: [pgadmin4][patch] Remove (...) from links
Previous Message Matthew Kleiman 2017-04-04 16:37:34 Re: [pgAdmin4][Patch][RM2257]: Query tool - Insert row doesn't use default values