Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d6f7E-0006jq-Vn for pgadmin-hackers@arkaria.postgresql.org; Fri, 05 May 2017 15:25:57 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d6f7D-0002Kr-Rn for pgadmin-hackers@arkaria.postgresql.org; Fri, 05 May 2017 15:25:55 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1d6f7D-0002Kh-AX for pgadmin-hackers@postgresql.org; Fri, 05 May 2017 15:25:55 +0000 Received: from mail-io0-x22a.google.com ([2607:f8b0:4001:c06::22a]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d6f74-00074e-Du for pgadmin-hackers@postgresql.org; Fri, 05 May 2017 15:25:54 +0000 Received: by mail-io0-x22a.google.com with SMTP id f102so12117918ioi.2 for ; Fri, 05 May 2017 08:25:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pivotal-io.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=FMqyL2+pGpsCQXOOt9cFwlSUxtZxV+zjx7HIlXUTG70=; b=k2atFRO9fFggcGIG7DLHbCyKA8ESoFrLNPxU8bwfMc8cyKilJxRJpstVRMO6TTpiUa kPR7nhCf82I/cGT4fx5IW5WYc5V2PNU4I2IFGe0wSbincXz1j8Jv+/q/jWWWhl4OP3gH 9hbL86SG+FAtfEZ1ob3RhC1RdXCGNX7hBuoTrGOJSnGhQ3YGlNOJKO5WsTG7etTLVk6S FJI8Klz27ArYUamYKO8hf/BSW0c4diGCRU3ynDSXABdCqznGciETAeWLqIeWLj9sBplv 5SvuyAJRwxpvS4NQ1xIxzUXZdsP0BUI2Vm8NTDPPsYRWQt3TLlSebNdghA9SLpplKZ08 Z17Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=FMqyL2+pGpsCQXOOt9cFwlSUxtZxV+zjx7HIlXUTG70=; b=ee0qbobKnM7UAxvxvk/coxb8p3ZhJ7dL76iB4aDMpFXYO0VAdnIWBB2xm4SaC/JoOR QH+d+lPnRRP6LDy7f9OHx816fhSINmFRsEfLXHVeJxJso83anS8bQrRLtiJIkSel/hyX C3W2zFLcbYr8UyqKmB0noxerUZLUTnC3dXvGYmeNKR/Rny7FgMj4f+gDAkMVx+fJmUSt /InmkgmvI6Tv1TImUONH0vA0SkjPv5NuKeDPwB9xHG3HZn/RoIkwwwnG8xekf+5cMoLV HzQ4OtNAQKMyY9UwQJOTDYTE55oRTdzPqyCa6begxXAUUyBkRTwmUVhwfJyhKVUcTouO HJzg== X-Gm-Message-State: AODbwcBiq7luRuYIKdfNbLbEEyuSOs2C6ipe61slqLBpu3T7MjP5jK/U VsAxKP709882nXubA8khQqlQzRJqLpHw X-Received: by 10.107.17.88 with SMTP id z85mr5018384ioi.151.1493997942978; Fri, 05 May 2017 08:25:42 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.169.216 with HTTP; Fri, 5 May 2017 08:25:22 -0700 (PDT) In-Reply-To: References: From: Sarah McAlear Date: Fri, 5 May 2017 11:25:22 -0400 Message-ID: Subject: Re: [pgAdmin4][PATCH] SlickGrid column resize triggers column select To: Murtuza Zabuawala , Ashesh Vashi Cc: pgadmin-hackers , Matthew Kleiman Content-Type: multipart/alternative; boundary=001a113f3a7caff8fc054ec8802c X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a113f3a7caff8fc054ec8802c Content-Type: text/plain; charset=UTF-8 Hi Murtuza and Ashesh! We are currently working on a number of features related to the query results grid. Some of the changes we are working on is that the checkboxes are going away. The implementation of this patch will have to change based on this. We noticed that 5 Javascript Tests are failing, in part because this patch introduces a new bug with the selection of rows. Replication steps: - select a column - select a row Now notice that the checkbox for the column is still checked, even though the column's cells are no longer selected. One of the tests that was failing was covering this behavior. Unfortunately there was a typo in the naming of the test (it originally was called "ColumnSelector selecting columns when a row is selected deselects the row" but should have been called "ColumnSelector selecting columns when a row is selected deselects the *column*"), but we have copied it with the correct naming below: describe("when a row is selected", function () { beforeEach(function () { var selectedRanges = [new Slick.Range(0, 0, 0, 1)]; rowSelectionModel.setSelectedRanges(selectedRanges); }); it("deselects the column", function () { container.find('.slick-header-column')[1].click(); var selectedRanges = rowSelectionModel.getSelectedRanges(); expect(selectedRanges.length).toBe(1); var column = selectedRanges[0]; expect(column.fromCell).toBe(1); expect(column.toCell).toBe(1); expect(column.fromRow).toBe(0); expect(column.toRow).toBe(9); }) }); Since we are finishing up this feature anyway, if this bug can wait another week, we can take it into our backlog and address it and submit the fix with our next patch. Thanks! Matt & Sarah On Fri, May 5, 2017 at 5:54 AM, Murtuza Zabuawala < murtuza.zabuawala@enterprisedb.com> wrote: > Hi, > > PFA patch to fix the issue where in SlickGrid column resize also triggers > 'onHeaderClick' event and triggers column selection, In this > scenario before drag event which column is under mouse gets selected, So to > fix the issue we will check if header checkbox is clicked. > RM#2348 > > Please review. > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: 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 > > --001a113f3a7caff8fc054ec8802c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Murtuza and Ashesh!

We are currently= working on a number of features related to the query results grid. Some of= the changes we are working on is that the checkboxes are going away. The i= mplementation of this patch will have to change based on this. We noticed t= hat 5 Javascript Tests are failing, in part because this patch introduces a= new bug with the selection of rows.

Replication s= teps:
- select a column
- select a row

Now notice that the checkbox for the column is still checked, even= though the column's cells are no longer selected.

=
One of the tests that was failing was covering this behavior. Unfortun= ately there was a typo in the naming of the test (it originally was called = "ColumnSelector selecting columns when a row is selected deselects the= row" but should have been called "ColumnSelector selecting colum= ns when a row is selected deselects the column"), but we have c= opied it with the correct naming below:
describe("when a row is selected", =
function () {
beforeEach= (function () {
var = selectedRanges =3D [new Slick.Range= (0, 0, 0, 1)];
=
rowSelectionModel.<= span style=3D"color:rgb(255,198,109)">setSelectedRanges
(selectedRang= es);
});

it("deselects the column"
<= span style=3D"color:rgb(204,120,50)">, function () {
container.find('.slick-header-column')[1].click();
va= r selectedRanges =3D rowSelectionModel.getSelectedRanges();=

ex= pect(selectedRanges.length).toBe(1);

var column =3D selectedRanges[0];

expect(column= .fromCell).toBe(1);
expect= (column.toCell).toBe(1);
expect(column.fromRow).toBe(0);
<= /span> expect(column.toRow).toBe(9);
})
});


Since we are finishing up this feature anyway, if this bug can wait ano= ther week, we can take it into our backlog and address it and submit the fi= x with our next patch.=C2=A0


Thanks= !
Matt & Sarah

On Fri, May 5, 2017 at 5:54 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA pat= ch to fix the issue where in SlickGrid column resize also triggers 'onH= eaderClick' event and triggers column selection, In this scenario=C2=A0= before drag event which column is under mouse gets selected, So to fix the = issue we will check if header checkbox is clicked.
RM#2348
<= div>
Please review.

--=
Regards,
Murtuza ZabuawalaE= nterpriseDB:=C2=A0http://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-ha= ckers


--001a113f3a7caff8fc054ec8802c--