Re: [pgadmin-hackers] [pgAdmin4][PATCH] Improvements to Query ResultsGrid User Experience

Hi

We don't want this to be committed, even to a local tree (as there's a
risk it may inadvertently get pushed). These patches are for review
only at this stage. Once they are ready, they may or may not be
committed individually, and even then it's very unlikely that we'll
want to use the supplied commit message (at minimum we'll want to add
a "Fixes #xxxx" so Redmine updates the ticket and links the commit to
it).

Please follow the project's normal process and submit patches that can
be applied with git apply.

Thanks.


On Tue, May 23, 2017 at 5:46 PM, Shruti B Iyer <siyer@pivotal.io> wrote:
> Hi Dave,
>
> git am is also helpful to apply a patch that was created using git
> format-patch, which is how we created these four patches. git am will apply
> the diff in the patch and also make a commit using the commit message stored
> in the patch. Could you try it again using git am? If it still doesn't work
> for you, we can try creating the diff files.
>
> Thanks,
> Shruti & Matt
>
> On Tue, May 23, 2017 at 5:28 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> git am is for applying patches from mailbox files:
>>
>> Splits mail messages in a mailbox into commit log message, authorship
>> information and patches, and applies them to the current branch.
>>
>> That doesn't seem like it'll help me as a gmail user. Can you fix the
>> patches to apply with git apply please?
>>
>> On Tuesday, May 23, 2017, Shruti B Iyer <siyer@pivotal.io> wrote:
>>>
>>> Hi Dave,
>>>
>>> We see the same errors when doing git apply for each patch. However, if
>>> you do git am for each patch, it should proceed.
>>>
>>> Thanks,
>>> Shruti & Matt
>>>
>>> On Tue, May 23, 2017 at 4:55 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>>
>>>> Hi!
>>>>
>>>> Looks great! I found a few issues which I think should be addressed
>>>> before we continue too far. Note that I haven't reviewed the code at
>>>> this stage:
>>>>
>>>> - When dragging a selection, the bounding box doesn't line up with the
>>>> bottom of the grid rows. Note that I couldn't screen shot this
>>>> unfortunately. It's not broken as such - just looks wrong.
>>>>
>>>> - If I copy one or more rows, I'm unable to paste them in as new rows
>>>> when editing table data.
>>>>
>>>> - The 0004 patch doesn't apply:
>>>>
>>>> (pgadmin4)snake:web dpage$ git apply
>>>> ~/Downloads/0004-Introduces-XCellSelectionModel.patch
>>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:640:
>>>> trailing whitespace.
>>>>     function scrollColumnIntoView(columnIndex) {
>>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:641:
>>>> trailing whitespace.
>>>>       var colspan = getColspan(row, columnIndex);
>>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:642:
>>>> trailing whitespace.
>>>>
>>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:643:
>>>> trailing whitespace.
>>>>       var left = columnPosLeft[columnIndex],
>>>> /Users/dpage/Downloads/0004-Introduces-XCellSelectionModel.patch:644:
>>>> trailing whitespace.
>>>>         right = columnPosRight[columnIndex + (colspan > 1 ? colspan - 1
>>>> : 0)],
>>>> error: patch failed:
>>>> web/pgadmin/static/vendor/slickgrid/slick.grid.js:2794
>>>> error: web/pgadmin/static/vendor/slickgrid/slick.grid.js: patch does not
>>>> apply
>>>>
>>>> Thanks, Dave.
>>>>
>>>>
>>>> On Tue, May 23, 2017 at 12:11 PM, Matthew Kleiman <mkleiman@pivotal.io>
>>>> wrote:
>>>> > Hi Hackers!
>>>> >
>>>> > Attached are the updates to the query results grid, broken up into
>>>> > four
>>>> > patches.
>>>> >
>>>> >
>>>> > Description of the Completed Functionality After Applying All Four
>>>> > Patches
>>>> > Currently the designed behavior is somewhere between excel like
>>>> > behavior and
>>>> > not. As such we can describe the behavior as follows:
>>>> >
>>>> > Select columns by clicking on the header
>>>> > Select rows by clicking on the row header (column 0)
>>>> > You can drag and select with the mouse
>>>> > You can select all with ctrl+a or by clicking the upper left cell
>>>> > You can copy with ctrl+c or with the copy icon
>>>> > you can increase or decrease the size of the selected area with
>>>> > shift+arrow
>>>> > shift+arrow understands directionality, e.g. drag select from left to
>>>> > right
>>>> > differs from drag select from right to left
>>>> > Clicking anywhere outside of the selected area deselects the area and
>>>> > reselects the new cell(s) clicked on
>>>> >
>>>> > Current potentially awkward but intentional functionality
>>>> >
>>>> > When you select multiple columns/rows by clicking on the header, then
>>>> > press
>>>> > shift+arrow all but the last selected columns/rows are deselected
>>>> >
>>>> > Includes fixes for:
>>>> > RM Bug #2348 - On resize of first/any column in "Query Tool/View Data"
>>>> > will
>>>> > select/deselect all the rows/columns.
>>>> > RM Bug #2344 - ctrl+v and ctrl+c need to work
>>>> >
>>>> >
>>>> > Detailed Description of Each Patch
>>>> >
>>>> > 0001-Improves-user-s-ability-to-select-cells-in-query-res.patch -
>>>> >
>>>> >       - user can select columns
>>>> >
>>>> >       - user can modify column or row selection with shift+arrow
>>>> >
>>>> >       - user can select entire grid with ctrl+A or cmd+A
>>>> >
>>>> >       - user can copy from grid using keyboard shortcuts
>>>> >
>>>> > 0002-Drag-and-select-from-data-grid.patch -
>>>> >
>>>> > 0003-Removes-checkboxes-from-the-grid.patch -
>>>> >
>>>> >     -  Changes header color to grey
>>>> >
>>>> > 0004-Introduces-XCellSelectionModel.patch -
>>>> >
>>>> >     - header styles depend on the selection
>>>> >
>>>> >     - show the correct row/column when scrolling up or left
>>>> >
>>>> >     - fixes drag and drop when drop is done outside the grid
>>>> >
>>>> >
>>>> > Thanks,
>>>> >
>>>> > Matt & Shruti
>>>> >
>>>> >
>>>> >
>>>> >
>>>> >
>>>> > --
>>>> > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>>> > To make changes to your subscription:
>>>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>>>> >
>>>>
>>>>
>>>>
>>>> --
>>>> 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