Re: [pgAdmin4][PATCH] Improvements to Query Results Grid User Experience

Hi Matthew,

Couple of review comments:
1) Clicking on a new added row(after save) results in console error -
screenshot attached.
2) If i drag to select rows inside the edit grid, columns headers are also
highlighted which shouldn't i think as a user.

New design of edit grid looks good :)

Thanks
Surinder

On Fri, Jun 2, 2017 at 8:32 PM, Matthew Kleiman <mkleiman(at)pivotal(dot)io> wrote:

> Hi Dave!
>
> It looks like everything has been addressed with this current patch, aside
> from the feature test for for edit table tool. If we agree that Surinder
> can send that patch on a separate thread, is this patch good to be
> committed?
>
> Thanks,
> Matt
>
> On Thu, Jun 1, 2017 at 11:17 AM, Robert Eckhardt <reckhardt(at)pivotal(dot)io>
> wrote:
>
>> Awesome, thanks.
>>
>> -- Rob
>>
>> On Thu, Jun 1, 2017 at 11:13 AM, Surinder Kumar <
>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Robert,
>>>
>>> On Jun 1, 2017 8:22 PM, "Robert Eckhardt" <reckhardt(at)pivotal(dot)io> wrote:
>>> >
>>> > Surindar,
>>> >
>>> > Have you sent this patch and I'm missing it or is it still in flight
>>> for you?
>>> I have found the solution, will test that patch and hopefully by
>>> tomorrow I will send.
>>>
>>> >
>>> > -- Rob
>>> >
>>> > On Wed, May 31, 2017 at 1:02 AM, Surinder Kumar <
>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>> >>
>>> >> Hi Joao
>>> >>
>>> >> On Wed, May 31, 2017 at 1:19 AM, Joao Pedro De Almeida Pereira <
>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>> >>>
>>> >>> Hello
>>> >>>
>>> >>> We have rebased the patches against master again, which includes
>>> Surinder's fix for RM2400. These patches should now apply against the HEAD
>>> of master.
>>> >>>>
>>> >>>>
>>> >>>> Given these issues, I think it would be sensible to add a feature
>>> test
>>> >>>> to copy/paste a couple of existing rows in a table, blank out the
>>> pkey
>>> >>>> values, save then refresh, and check everything looks right.
>>> Thoughts?
>>> >>>
>>> >>> Currently, the feature test, CopySelectedQueryResultsFeatureTest,
>>> only covers copy functionality in the query tool. It sounds like we could
>>> use some additional coverage around the Edit Table tool, which could
>>> include paste rows functionality. Can we get this patch merged and create a
>>> RedMine issue that enumerates the additional functionality we want to cover
>>> via feature test?
>>> >>
>>> >> ​FeatureTest for edit Table tool and copy/paste rows are already
>>> written, but there is some flakiness when executing. I will be sending that
>>> patch again with fix.​
>>> >>>
>>> >>>
>>> >>> Thanks,
>>> >>> Joao & Matt
>>> >>>
>>> >>>
>>> >>> On Sat, May 27, 2017 at 2:19 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>> wrote:
>>> >>>>
>>> >>>> On Sat, May 27, 2017 at 9:02 AM, Surinder Kumar
>>> >>>> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>> >>>> > Hi Dave,
>>> >>>> > On Sat, May 27, 2017 at 3:07 AM, Dave Page <dpage(at)pgadmin(dot)org>
>>> wrote:
>>> >>>> >>
>>> >>>> >> Hi
>>> >>>> >>
>>> >>>> >> OK, so we're getting somewhere now :-). Here's what I found:
>>> >>>> >>
>>> >>>> >> - The grid looks and feels great now. Selection works nicely, and
>>> >>>> >> copying rows seems to work well.
>>> >>>> >>
>>> >>>> >> - Patch 5 doesn't apply - but only to slick.grid.js. I manually
>>> fixed
>>> >>>> >> it for testing, but the patch really doesn't look like it was
>>> created
>>> >>>> >> from the version of the file from GIT HEAD with patches 1 - 4
>>> applied.
>>> >>>> >>
>>> >>>> >> - There is a problem. I noticed that a) when copy/pasting rows,
>>> if I
>>> >>>> >> blank out a key column with a default it gets set to [null]
>>> instead of
>>> >>>> >> [default] and b) inserting rows seems to get the values of the
>>> new
>>> >>>> >> row(s) from the wrong place, resulting in duplicate key
>>> violations.
>>> >>>> >
>>> >>>> > This belongs to RM2400 - handle [default] and [null] while
>>> copy/pasting
>>> >>>> > rows. An updated patch and Feature tests are already sent.
>>> >>>> > You can review and commit if looks good.
>>> >>>>
>>> >>>> That part, yes - but not mixing up the values.
>>> >>>>
>>> >>>>
>>> >>>> >> Now, both of these issues sound very much like ones Surinder
>>> fixed
>>> >>>> >> recently following other improvements to the grid to more
>>> properly
>>> >>>> >> handle nulls and default values. I *think* these fixes were in
>>> commit:
>>> >>>> >> d7d4bf475bc5b131d9a76376ebfc87e004d92333. Perhaps there's been a
>>> >>>> >> merging error in your development branch at some point?
>>> >>>> >>
>>> >>>> >> Given these issues, I think it would be sensible to add a
>>> feature test
>>> >>>> >> to copy/paste a couple of existing rows in a table, blank out
>>> the pkey
>>> >>>> >> values, save then refresh, and check everything looks right.
>>> Thoughts?
>>> >>>> >>
>>> >>>> >> I'm looking forward to seeing this fully baked :-)
>>> >>>> >>
>>> >>>> >> Thanks!
>>> >>>> >>
>>> >>>> >> On Fri, May 26, 2017 at 11:23 AM, Joao Pedro De Almeida Pereira
>>> >>>> >> <jdealmeidapereira(at)pivotal(dot)io> wrote:
>>> >>>> >> > Hello Dave,
>>> >>>> >> >
>>> >>>> >> > We created the previous patches using the command recommended
>>> in the
>>> >>>> >> > pgAdmin
>>> >>>> >> > website, but apparently the diff doesn't work correctly when
>>> you have
>>> >>>> >> > binary
>>> >>>> >> > files in the commit, like images.
>>> >>>> >> >
>>> >>>> >> > We regenerated the patches using the command with the addition
>>> of
>>> >>>> >> > --binary
>>> >>>> >> >
>>> >>>> >> > git diff e0d5cf6b ac65f43b --binary >
>>> 6-adapt-slickgrid-to-pgadmin.patch
>>> >>>> >> >
>>> >>>> >> > After that, we applied them in master and this was the output:
>>> >>>> >> >
>>> >>>> >> > ± jp+si |master → origin {9} ✓| → git apply
>>> 1-upgrade-slickgrid.patch
>>> >>>> >> > 1-upgrade-slickgrid.patch:120: trailing whitespace.
>>> >>>> >> >
>>> >>>> >> > 1-upgrade-slickgrid.patch:129: trailing whitespace.
>>> >>>> >> >
>>> >>>> >> > 1-upgrade-slickgrid.patch:138: trailing whitespace.
>>> >>>> >> >
>>> >>>> >> > 1-upgrade-slickgrid.patch:147: trailing whitespace.
>>> >>>> >> >
>>> >>>> >> > 1-upgrade-slickgrid.patch:156: trailing whitespace.
>>> >>>> >> >
>>> >>>> >> > warning: squelched 3856 whitespace errors
>>> >>>> >> > warning: 3861 lines add whitespace errors.
>>> >>>> >> >
>>> >>>> >> > maujer in ~/workspace/pgadmin4
>>> >>>> >> > ± jp+si |master → origin {9} U:17 ?:7 ✗| → git apply
>>> >>>> >> > 2-select-cells-improvements.patch
>>> >>>> >> >
>>> >>>> >> > maujer in ~/workspace/pgadmin4
>>> >>>> >> > ± jp+si |master → origin {9} U:35 ?:12 ✗| → git apply
>>> >>>> >> > 3-drag-and-select.patch
>>> >>>> >> >
>>> >>>> >> > maujer in ~/workspace/pgadmin4
>>> >>>> >> > ± jp+si |master → origin {9} U:41 ?:13 ✗| → git apply
>>> >>>> >> > 4-remove-checkboxes.patch
>>> >>>> >> >
>>> >>>> >> > maujer in ~/workspace/pgadmin4
>>> >>>> >> > ± jp+si |master → origin {9} U:41 ?:14 ✗| → git apply
>>> >>>> >> > 5-improvements-to-range-selection.patch
>>> >>>> >> > 5-improvements-to-range-selection.patch:647: trailing
>>> whitespace.
>>> >>>> >> > function scrollColumnIntoView(columnIndex) {
>>> >>>> >> > 5-improvements-to-range-selection.patch:648: trailing
>>> whitespace.
>>> >>>> >> > var colspan = getColspan(row, columnIndex);
>>> >>>> >> > 5-improvements-to-range-selection.patch:649: trailing
>>> whitespace.
>>> >>>> >> >
>>> >>>> >> > 5-improvements-to-range-selection.patch:650: trailing
>>> whitespace.
>>> >>>> >> > var left = columnPosLeft[columnIndex],
>>> >>>> >> > 5-improvements-to-range-selection.patch:651: trailing
>>> whitespace.
>>> >>>> >> > right = columnPosRight[columnIndex + (colspan > 1 ?
>>> colspan - 1
>>> >>>> >> > :
>>> >>>> >> > 0)],
>>> >>>> >> > warning: squelched 15 whitespace errors
>>> >>>> >> > warning: 20 lines add whitespace errors.
>>> >>>> >> >
>>> >>>> >> > maujer in ~/workspace/pgadmin4
>>> >>>> >> > ± jp+si |master → origin {9} U:41 ?:16 ✗| → git apply
>>> >>>> >> > 6-adapt-slickgrid-to-pgadmin.patch
>>> >>>> >> >
>>> >>>> >> >
>>> >>>> >> > Maybe we should update the website with this information to
>>> help future
>>> >>>> >> > committers.
>>> >>>> >> >
>>> >>>> >> > Thanks
>>> >>>> >> > Joao & Shruti
>>> >>>> >> >
>>> >>>> >> >
>>> >>>> >> > On Fri, May 26, 2017 at 10:35 AM, Dave Page <dpage(at)pgadmin(dot)org>
>>> wrote:
>>> >>>> >> >>
>>> >>>> >> >> Hi,
>>> >>>> >> >>
>>> >>>> >> >> These patches really don't look like they were created in the
>>> normal
>>> >>>> >> >> way:
>>> >>>> >> >>
>>> >>>> >> >> (pgadmin4)snake:web dpage$ git apply
>>> >>>> >> >> ~/Downloads/1-upgrade-slickgrid.patch
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:126:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >>
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:129:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >> where the browser copies/pastes the serialized data.
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:130:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >>
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:154:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >>
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:165:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >>
>>> >>>> >> >> error: cannot apply binary patch to
>>> >>>> >> >> 'web/pgadmin/static/vendor/slickgrid/images/CheckboxN.png'
>>> without
>>> >>>> >> >> full index line
>>> >>>> >> >> error: web/pgadmin/static/vendor/slickgrid/images/CheckboxN.png:
>>> patch
>>> >>>> >> >> does not apply
>>> >>>> >> >> error: cannot apply binary patch to
>>> >>>> >> >> 'web/pgadmin/static/vendor/slickgrid/images/CheckboxY.png'
>>> without
>>> >>>> >> >> full index line
>>> >>>> >> >> error: web/pgadmin/static/vendor/slickgrid/images/CheckboxY.png:
>>> patch
>>> >>>> >> >> does not apply
>>> >>>> >> >> error: patch failed:
>>> >>>> >> >>
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselect
>>> ionmodel.js:94
>>> >>>> >> >> error:
>>> >>>> >> >>
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselect
>>> ionmodel.js:
>>> >>>> >> >> patch does not apply
>>> >>>> >> >> error: patch failed:
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.css:102
>>> >>>> >> >> error: web/pgadmin/static/vendor/slickgrid/slick.grid.css:
>>> patch does
>>> >>>> >> >> not
>>> >>>> >> >> apply
>>> >>>> >> >> error: patch failed:
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.js:1
>>> >>>> >> >> error: web/pgadmin/static/vendor/slickgrid/slick.grid.js:
>>> patch does
>>> >>>> >> >> not
>>> >>>> >> >> apply
>>> >>>> >> >> (pgadmin4)snake:web dpage$ cd ../
>>> >>>> >> >> (pgadmin4)snake:pgadmin4 dpage$ git apply
>>> >>>> >> >> ~/Downloads/1-upgrade-slickgrid.patch
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:126:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >>
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:129:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >> where the browser copies/pastes the serialized data.
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:130:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >>
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:154:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >>
>>> >>>> >> >> /Users/dpage/Downloads/1-upgrade-slickgrid.patch:165:
>>> trailing
>>> >>>> >> >> whitespace.
>>> >>>> >> >>
>>> >>>> >> >> error: cannot apply binary patch to
>>> >>>> >> >> 'web/pgadmin/static/vendor/slickgrid/images/CheckboxN.png'
>>> without
>>> >>>> >> >> full index line
>>> >>>> >> >> error: web/pgadmin/static/vendor/slickgrid/images/CheckboxN.png:
>>> patch
>>> >>>> >> >> does not apply
>>> >>>> >> >> error: cannot apply binary patch to
>>> >>>> >> >> 'web/pgadmin/static/vendor/slickgrid/images/CheckboxY.png'
>>> without
>>> >>>> >> >> full index line
>>> >>>> >> >> error: web/pgadmin/static/vendor/slickgrid/images/CheckboxY.png:
>>> patch
>>> >>>> >> >> does not apply
>>> >>>> >> >> error: patch failed:
>>> >>>> >> >>
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselect
>>> ionmodel.js:94
>>> >>>> >> >> error:
>>> >>>> >> >>
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselect
>>> ionmodel.js:
>>> >>>> >> >> patch does not apply
>>> >>>> >> >> error: patch failed:
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.css:102
>>> >>>> >> >> error: web/pgadmin/static/vendor/slickgrid/slick.grid.css:
>>> patch does
>>> >>>> >> >> not
>>> >>>> >> >> apply
>>> >>>> >> >> error: patch failed:
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.js:1
>>> >>>> >> >> error: web/pgadmin/static/vendor/slickgrid/slick.grid.js:
>>> patch does
>>> >>>> >> >> not
>>> >>>> >> >> apply
>>> >>>> >> >>
>>> >>>> >> >> I even tried with patch:
>>> >>>> >> >>
>>> >>>> >> >> (pgadmin4)snake:pgadmin4 dpage$ patch -p1 <
>>> >>>> >> >> ~/Downloads/1-upgrade-slickgrid.patch
>>> >>>> >> >> patching file libraries.txt
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/README
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slickgrid/README.md
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/controls/slick.columnpic
>>> ker.js
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellcopyma
>>> nager.js
>>> >>>> >> >> patching file
>>> >>>> >> >>
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellextern
>>> alcopymanager.js
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellranges
>>> elector.js
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.cellselect
>>> ionmodel.js
>>> >>>> >> >> Hunk #3 succeeded at 95 with fuzz 2.
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.headerbutt
>>> ons.js
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.headermenu
>>> .js
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/plugins/slick.rowselecti
>>> onmodel.js
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick-default-theme.css
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slic
>>> kgrid/slick.core.js
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slic
>>> kgrid/slick.dataview.js
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slic
>>> kgrid/slick.editors.js
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slic
>>> kgrid/slick.formatters.js
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slic
>>> kgrid/slick.grid.css
>>> >>>> >> >> Hunk #6 FAILED at 117.
>>> >>>> >> >> 1 out of 9 hunks FAILED -- saving rejects to file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.css.rej
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slic
>>> kgrid/slick.grid.js
>>> >>>> >> >> Hunk #1 FAILED at 1.
>>> >>>> >> >> Hunk #2 FAILED at 18.
>>> >>>> >> >> Hunk #3 FAILED at 75.
>>> >>>> >> >> Hunk #4 FAILED at 89.
>>> >>>> >> >> Hunk #5 FAILED at 129.
>>> >>>> >> >> Hunk #6 FAILED at 143.
>>> >>>> >> >> Hunk #7 FAILED at 174.
>>> >>>> >> >> Hunk #8 FAILED at 208.
>>> >>>> >> >> Hunk #9 FAILED at 289.
>>> >>>> >> >> Hunk #10 FAILED at 330.
>>> >>>> >> >> Hunk #11 FAILED at 343.
>>> >>>> >> >> Hunk #12 FAILED at 385.
>>> >>>> >> >> Hunk #13 FAILED at 457.
>>> >>>> >> >> Hunk #14 FAILED at 491.
>>> >>>> >> >> Hunk #15 FAILED at 510.
>>> >>>> >> >> Hunk #16 FAILED at 548.
>>> >>>> >> >> Hunk #17 FAILED at 557.
>>> >>>> >> >> Hunk #18 FAILED at 600.
>>> >>>> >> >> Hunk #19 FAILED at 652.
>>> >>>> >> >> Hunk #20 FAILED at 686.
>>> >>>> >> >> Hunk #21 FAILED at 706.
>>> >>>> >> >> Hunk #22 FAILED at 749.
>>> >>>> >> >> Hunk #23 FAILED at 858.
>>> >>>> >> >> Hunk #24 FAILED at 870.
>>> >>>> >> >> Hunk #25 FAILED at 886.
>>> >>>> >> >> Hunk #26 FAILED at 937.
>>> >>>> >> >> Hunk #27 FAILED at 957.
>>> >>>> >> >> Hunk #28 FAILED at 987.
>>> >>>> >> >> Hunk #29 FAILED at 1007.
>>> >>>> >> >> Hunk #30 FAILED at 1039.
>>> >>>> >> >> Hunk #31 FAILED at 1057.
>>> >>>> >> >> Hunk #32 FAILED at 1080.
>>> >>>> >> >> Hunk #33 FAILED at 1098.
>>> >>>> >> >> Hunk #34 FAILED at 1117.
>>> >>>> >> >> Hunk #35 FAILED at 1155.
>>> >>>> >> >> Hunk #36 FAILED at 1267.
>>> >>>> >> >> Hunk #37 FAILED at 1303.
>>> >>>> >> >> Hunk #38 FAILED at 1317.
>>> >>>> >> >> Hunk #39 FAILED at 1400.
>>> >>>> >> >> Hunk #40 FAILED at 1415.
>>> >>>> >> >> Hunk #41 FAILED at 1446.
>>> >>>> >> >> Hunk #42 FAILED at 1485.
>>> >>>> >> >> Hunk #43 FAILED at 1583.
>>> >>>> >> >> Hunk #44 FAILED at 1600.
>>> >>>> >> >> Hunk #45 FAILED at 1637.
>>> >>>> >> >> Hunk #46 FAILED at 1650.
>>> >>>> >> >> Hunk #47 FAILED at 1763.
>>> >>>> >> >> Hunk #48 FAILED at 1780.
>>> >>>> >> >> Hunk #49 FAILED at 1804.
>>> >>>> >> >> Hunk #50 FAILED at 1818.
>>> >>>> >> >> Hunk #51 FAILED at 1832.
>>> >>>> >> >> Hunk #52 FAILED at 1848.
>>> >>>> >> >> Hunk #53 FAILED at 1877.
>>> >>>> >> >> Hunk #54 FAILED at 1893.
>>> >>>> >> >> Hunk #55 FAILED at 2256.
>>> >>>> >> >> Hunk #56 FAILED at 2274.
>>> >>>> >> >> Hunk #57 FAILED at 2415.
>>> >>>> >> >> Hunk #58 FAILED at 2474.
>>> >>>> >> >> Hunk #59 FAILED at 2538.
>>> >>>> >> >> Hunk #60 FAILED at 2606.
>>> >>>> >> >> Hunk #61 FAILED at 2621.
>>> >>>> >> >> Hunk #62 FAILED at 2724.
>>> >>>> >> >> Hunk #63 FAILED at 2740.
>>> >>>> >> >> Hunk #64 FAILED at 2798.
>>> >>>> >> >> Hunk #65 FAILED at 2815.
>>> >>>> >> >> Hunk #66 FAILED at 2839.
>>> >>>> >> >> Hunk #67 FAILED at 2913.
>>> >>>> >> >> Hunk #68 FAILED at 2928.
>>> >>>> >> >> Hunk #69 FAILED at 2954.
>>> >>>> >> >> Hunk #70 FAILED at 2974.
>>> >>>> >> >> Hunk #71 FAILED at 3018.
>>> >>>> >> >> Hunk #72 FAILED at 3307.
>>> >>>> >> >> Hunk #73 FAILED at 3443.
>>> >>>> >> >> Hunk #74 FAILED at 3454.
>>> >>>> >> >> Hunk #75 FAILED at 3621.
>>> >>>> >> >> Hunk #76 FAILED at 3662.
>>> >>>> >> >> Hunk #77 FAILED at 3674.
>>> >>>> >> >> Hunk #78 FAILED at 3684.
>>> >>>> >> >> Hunk #79 FAILED at 3724.
>>> >>>> >> >> Hunk #80 FAILED at 3770.
>>> >>>> >> >> 80 out of 80 hunks FAILED -- saving rejects to file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.grid.js.rej
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.groupitemmetadatap
>>> rovider.js
>>> >>>> >> >> patching file
>>> >>>> >> >> web/pgadmin/static/vendor/slickgrid/slick.remotemodel-yahoo.
>>> js
>>> >>>> >> >> patching file web/pgadmin/static/vendor/slic
>>> kgrid/slick.remotemodel.js
>>> >>>> >> >>
>>> >>>> >> >> On Thu, May 25, 2017 at 2:35 PM, Shruti B Iyer <
>>> siyer(at)pivotal(dot)io>
>>> >>>> >> >> wrote:
>>> >>>> >> >> > Hi
>>> >>>> >> >> >
>>> >>>> >> >> > Attached are the patches generated using diff and the
>>> correction of
>>> >>>> >> >> > the
>>> >>>> >> >> > bugs
>>> >>>> >> >> > you mentioned in the previous email.
>>> >>>> >> >> >
>>> >>>> >> >> > We also split the initial first patch
>>> >>>> >> >> > 0001-Improves-user-s-ability-t
>>> o-select-cells-in-query-res.patch into
>>> >>>> >> >> > two
>>> >>>> >> >> > different patches because it included the upgrade of
>>> slickgrid and
>>> >>>> >> >> > other
>>> >>>> >> >> > changes making it hard to read. Now, the code review should
>>> be much
>>> >>>> >> >> > easier
>>> >>>> >> >> > and the upgrade of a vendor library becomes more clear in
>>> the commit
>>> >>>> >> >> > history.
>>> >>>> >> >> >
>>> >>>> >> >> > The 6th patch 6-adapt-slickgrid-to-pgadmin.patch contains
>>> the edits
>>> >>>> >> >> > to
>>> >>>> >> >> > the
>>> >>>> >> >> > slickgrid library that makes drag-and-select bounding box
>>> align with
>>> >>>> >> >> > the
>>> >>>> >> >> > gridlines. We created a PR to slickgrid that we are still
>>> discussing
>>> >>>> >> >> > the
>>> >>>> >> >> > implementation. Meanwhile. we created a temporary fix to
>>> solve the
>>> >>>> >> >> > problem.
>>> >>>> >> >> > We suggest that this 6th patch be committed separately.
>>> >>>> >> >> >
>>> >>>> >> >> > Thanks
>>> >>>> >> >> > Joao & Shruti
>>> >>>> >> >> >
>>> >>>> >> >> > On Wed, May 24, 2017 at 8:18 AM, Dave Page <
>>> dpage(at)pgadmin(dot)org> wrote:
>>> >>>> >> >> >>
>>> >>>> >> >> >> 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(at)pivotal(dot)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(at)pgadmin(dot)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(at)pivotal(dot)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(at)pgadmin(dot)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-In
>>> troduces-XCellSelectionModel.patch:640:
>>> >>>> >> >> >> >>>> trailing whitespace.
>>> >>>> >> >> >> >>>> function scrollColumnIntoView(columnIndex) {
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>> /Users/dpage/Downloads/0004-In
>>> troduces-XCellSelectionModel.patch:641:
>>> >>>> >> >> >> >>>> trailing whitespace.
>>> >>>> >> >> >> >>>> var colspan = getColspan(row, columnIndex);
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>> /Users/dpage/Downloads/0004-In
>>> troduces-XCellSelectionModel.patch:642:
>>> >>>> >> >> >> >>>> trailing whitespace.
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>> /Users/dpage/Downloads/0004-In
>>> troduces-XCellSelectionModel.patch:643:
>>> >>>> >> >> >> >>>> trailing whitespace.
>>> >>>> >> >> >> >>>> var left = columnPosLeft[columnIndex],
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>>
>>> >>>> >> >> >> >>>> /Users/dpage/Downloads/0004-In
>>> troduces-XCellSelectionModel.patch:644:
>>> >>>> >> >> >> >>>> trailing whitespace.
>>> >>>> >> >> >> >>>> right = columnPosRight[columnIndex + (colspan
>>> > 1 ?
>>> >>>> >> >> >> >>>> colspan -
>>> >>>> >> >> >> >>>> 1
>>> >>>> >> >> >> >>>> : 0)],
>>> >>>> >> >> >> >>>> error: patch failed:
>>> >>>> >> >> >> >>>> web/pgadmin/static/vendor/slic
>>> kgrid/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(at)pivotal(dot)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-t
>>> o-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(at)postgresql(dot)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
>>> >>>> >> >> >
>>> >>>> >> >> >
>>> >>>> >> >>
>>> >>>> >> >>
>>> >>>> >> >>
>>> >>>> >> >> --
>>> >>>> >> >> 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(at)postgresql(dot)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
>>> >>>> >>
>>> >>>> >>
>>> >>>> >> --
>>> >>>> >> Sent via pgadmin-hackers mailing list (
>>> pgadmin-hackers(at)postgresql(dot)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
>>> >>>
>>> >>>
>>> >>
>>> >
>>>
>>>
>>
>