public inbox for [email protected]help / color / mirror / Atom feed
[pgAdmin4][PATCH] SlickGrid column resize triggers column select 3+ messages / 2 participants [nested] [flat]
* [pgAdmin4][PATCH] SlickGrid column resize triggers column select @ 2017-05-05 09:54 Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Murtuza Zabuawala @ 2017-05-05 09:54 UTC (permalink / raw) To: pgadmin-hackers 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 diff --git a/web/pgadmin/static/js/selection/column_selector.js b/web/pgadmin/static/js/selection/column_selector.js index c89b3fa..5260fc3 100644 --- a/web/pgadmin/static/js/selection/column_selector.js +++ b/web/pgadmin/static/js/selection/column_selector.js @@ -6,12 +6,10 @@ define(['jquery', 'sources/selection/range_selection_helper', 'slickgrid'], func if (column.selectable !== false) { - if (!clickedCheckbox(event)) { + if (clickedCheckbox(event)) { var $checkbox = $("[data-id='checkbox-" + column.id + "']"); - toggleCheckbox($checkbox); + updateRanges(grid, column.id); } - - updateRanges(grid, column.id); } } ); @@ -55,14 +53,6 @@ define(['jquery', 'sources/selection/range_selection_helper', 'slickgrid'], func return e.target.type == "checkbox" }; - var toggleCheckbox = function (checkbox) { - if (checkbox.prop("checked")) { - checkbox.prop("checked", false) - } else { - checkbox.prop("checked", true) - } - }; - var getColumnDefinitionsWithCheckboxes = function (columnDefinitions) { return _.map(columnDefinitions, function (columnDefinition) { if (columnDefinition.selectable !== false) { -- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers Attachments: [text/plain] RM_2348.diff (1.3K, 3-RM_2348.diff) download | inline diff: diff --git a/web/pgadmin/static/js/selection/column_selector.js b/web/pgadmin/static/js/selection/column_selector.js index c89b3fa..5260fc3 100644 --- a/web/pgadmin/static/js/selection/column_selector.js +++ b/web/pgadmin/static/js/selection/column_selector.js @@ -6,12 +6,10 @@ define(['jquery', 'sources/selection/range_selection_helper', 'slickgrid'], func if (column.selectable !== false) { - if (!clickedCheckbox(event)) { + if (clickedCheckbox(event)) { var $checkbox = $("[data-id='checkbox-" + column.id + "']"); - toggleCheckbox($checkbox); + updateRanges(grid, column.id); } - - updateRanges(grid, column.id); } } ); @@ -55,14 +53,6 @@ define(['jquery', 'sources/selection/range_selection_helper', 'slickgrid'], func return e.target.type == "checkbox" }; - var toggleCheckbox = function (checkbox) { - if (checkbox.prop("checked")) { - checkbox.prop("checked", false) - } else { - checkbox.prop("checked", true) - } - }; - var getColumnDefinitionsWithCheckboxes = function (columnDefinitions) { return _.map(columnDefinitions, function (columnDefinition) { if (columnDefinition.selectable !== false) { ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [pgAdmin4][PATCH] SlickGrid column resize triggers column select @ 2017-05-05 15:25 Sarah McAlear <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Sarah McAlear @ 2017-05-05 15:25 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; Ashesh Vashi <[email protected]>; +Cc: pgadmin-hackers; Matthew Kleiman <[email protected]> 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 < [email protected]> 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 ([email protected]) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > > ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [pgAdmin4][PATCH] SlickGrid column resize triggers column select @ 2017-05-05 16:36 Murtuza Zabuawala <[email protected]> parent: Sarah McAlear <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Murtuza Zabuawala @ 2017-05-05 16:36 UTC (permalink / raw) To: Sarah McAlear <[email protected]>; +Cc: Ashesh Vashi <[email protected]>; pgadmin-hackers; Matthew Kleiman <[email protected]> Sure. --Murtuza On Fri, May 5, 2017 at 8:55 PM, Sarah McAlear <[email protected]> wrote: > 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 ([email protected]) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgadmin-hackers >> >> > ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2017-05-05 16:36 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2017-05-05 09:54 [pgAdmin4][PATCH] SlickGrid column resize triggers column select Murtuza Zabuawala <[email protected]> 2017-05-05 15:25 ` Sarah McAlear <[email protected]> 2017-05-05 16:36 ` Murtuza Zabuawala <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox