From 295c81c8ffdaa90439096e4b770659117e766395 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Thu, 23 Feb 2023 17:21:02 -0500 Subject: [PATCH] fix: Edit cell mouseout should save & excel copy should work, fix #1103 - fixes #1103 caused by a regression introduced in Slickgrid-Universal PR [901](https://github.com/ghiscoding/slickgrid-universal/pull/901) - requires Slickgrid-Universal PR [917](https://github.com/ghiscoding/slickgrid-universal/pull/917) to be merged and released - the regression came after I wanted to fix another bug which was that making a cell range and Copy+Paste wasn't working, when fixing that bug it caused a new bug (this regression). This PR should fix both of these bugs and remove a very old hack that was introduced with `suppressActiveCellChangeOnEdit` which is no longer required - added Cypress E2E tests to cover the bug identified in #1103 --- src/app/examples/grid-editor.component.html | 2 +- src/app/examples/grid-editor.component.ts | 1 + .../components/angular-slickgrid.component.ts | 10 +++---- .../angular-slickgrid/global-grid-options.ts | 2 +- test/cypress/e2e/example03.cy.ts | 28 ++++++++++++++++--- test/cypress/e2e/example16.cy.ts | 12 ++++---- 6 files changed, 38 insertions(+), 17 deletions(-) diff --git a/src/app/examples/grid-editor.component.html b/src/app/examples/grid-editor.component.html index 90481ea60..452cdf6ef 100644 --- a/src/app/examples/grid-editor.component.html +++ b/src/app/examples/grid-editor.component.html @@ -25,7 +25,7 @@

- diff --git a/src/app/examples/grid-editor.component.ts b/src/app/examples/grid-editor.component.ts index 9eaff54b4..ff1ec133e 100644 --- a/src/app/examples/grid-editor.component.ts +++ b/src/app/examples/grid-editor.component.ts @@ -186,6 +186,7 @@ export class GridEditorComponent implements OnInit { type: FieldType.string, editor: { model: CustomInputEditor, + // model: Editors.text, placeholder: 'custom', validator: myCustomTitleValidator, // use a custom validator }, diff --git a/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts b/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts index 8291b8dd4..b5980c390 100644 --- a/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts +++ b/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts @@ -512,7 +512,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy { // before certain extentions/plugins potentially adds extra columns not created by the user itself (RowMove, RowDetail, RowSelections) // we'll subscribe to the event and push back the change to the user so they always use full column defs array including extra cols this.subscriptions.push( - this._eventPubSubService.subscribe<{ columns: Column[]; grid: SlickGrid }>('onPluginColumnsChanged', data => { + this._eventPubSubService.subscribe<{ columns: Column[]; grid: SlickGrid; }>('onPluginColumnsChanged', data => { this._columnDefinitions = data.columns; this.columnDefinitionsChange.emit(this._columnDefinitions); }) @@ -654,7 +654,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy { resizerService: this.resizerService, sortService: this.sortService, treeDataService: this.treeDataService, - } + }; // all instances (SlickGrid, DataView & all Services) this._eventPubSubService.publish('onAngularGridCreated', this._angularGridInstances); @@ -1074,7 +1074,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy { this._eventPubSubService.subscribe('onPaginationChanged', (paginationChanges: ServicePagination) => { this.paginationChanged(paginationChanges); }), - this._eventPubSubService.subscribe('onPaginationVisibilityChanged', (visibility: { visible: boolean }) => { + this._eventPubSubService.subscribe('onPaginationVisibilityChanged', (visibility: { visible: boolean; }) => { this.showPagination = visibility?.visible ?? false; if (this.gridOptions?.backendServiceApi) { this.backendUtilityService?.refreshBackendDataset(this.gridOptions); @@ -1364,10 +1364,10 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy { return columnDefinitions.map((column: Column | any) => { // on every Editor that have a "collectionAsync", resolve the data and assign it to the "collection" property - if (column && column.editor && column.editor.collectionAsync) { + if (column?.editor?.collectionAsync) { this.loadEditorCollectionAsync(column); } - return { ...column, editor: column.editor && column.editor.model, internalColumnEditor: { ...column.editor } }; + return { ...column, editor: column.editor?.model, internalColumnEditor: { ...column.editor } }; }); } diff --git a/src/app/modules/angular-slickgrid/global-grid-options.ts b/src/app/modules/angular-slickgrid/global-grid-options.ts index 50d52bfdd..3f238a3d4 100644 --- a/src/app/modules/angular-slickgrid/global-grid-options.ts +++ b/src/app/modules/angular-slickgrid/global-grid-options.ts @@ -198,7 +198,7 @@ export const GlobalGridOptions: Partial = { numberedMultiColumnSort: true, tristateMultiColumnSort: false, sortColNumberInSeparateSpan: true, - suppressActiveCellChangeOnEdit: true, + suppressActiveCellChangeOnEdit: false, pagination: { pageSizes: [10, 15, 20, 25, 30, 40, 50, 75, 100], pageSize: 25, diff --git a/test/cypress/e2e/example03.cy.ts b/test/cypress/e2e/example03.cy.ts index b4536a34c..05d287c68 100644 --- a/test/cypress/e2e/example03.cy.ts +++ b/test/cypress/e2e/example03.cy.ts @@ -1,4 +1,4 @@ -describe('Example 3 - Grid with Editors', { retries: 1 }, () => { +describe('Example 3 - Grid with Editors', { retries: 0 }, () => { const GRID_ROW_HEIGHT = 35; const fullTitles = [ '', '', 'Title', 'Title, Custom Editor', 'Duration (days)', '% Complete', @@ -18,6 +18,26 @@ describe('Example 3 - Grid with Editors', { retries: 1 }, () => { .each(($child, index) => expect($child.text()).to.eq(fullTitles[index])); }); + it('should be able to change Title with Custom Editor and expect to save when changing the value and then mouse clicking on a different cell', () => { + cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(3)`).should('contain', 'Task 1').click(); + cy.get('input[type=text].editor-text') + .type('Task 8888'); + + // mouse click on next cell on the right & expect a save + cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(4)`).click(); + cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(3)`).should('contain', 'Task 8888'); + }); + + it('should be able to undo the editor and expect it to be opened, then clicking on Escape should reveal the cell to have rolled back text of "Task 1"', () => { + cy.get('[data-test="undo-btn"]').click(); + + cy.get('input[type=text].editor-text') + .should('exist') + .type('{esc}'); + + cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(3)`).should('contain', 'Task 1'); + }); + it('should enable "Auto Commit Edit"', () => { cy.get('[data-test=auto-commit]') .click(); @@ -50,7 +70,7 @@ describe('Example 3 - Grid with Editors', { retries: 1 }, () => { cy.get('.flatpickr-monthDropdown-months:visible') .select('January', { force: true }); cy.get('.numInput.cur-year:visible').type('2009'); - cy.get('.flatpickr-day:visible:nth(25)').click('bottom', { force: true }) + cy.get('.flatpickr-day:visible:nth(25)').click('bottom', { force: true }); cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(6)`).should('contain', '2009-01-22'); // change City of Origin @@ -123,7 +143,7 @@ describe('Example 3 - Grid with Editors', { retries: 1 }, () => { cy.get('.flatpickr-monthDropdown-months:visible') .select('January', { force: true }); cy.get('.numInput.cur-year:visible').type('2009'); - cy.get('.flatpickr-day:visible:nth(25)').click('bottom', { force: true }) + cy.get('.flatpickr-day:visible:nth(25)').click('bottom', { force: true }); cy.get(`[style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(6)`).should('contain', '2009-01-22'); // change Effort Driven @@ -197,7 +217,7 @@ describe('Example 3 - Grid with Editors', { retries: 1 }, () => { // change Finish date cy.get(`[style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(6)`).click(); - cy.get('.flatpickr-day:visible:nth(24)').click('bottom', { force: true }) + cy.get('.flatpickr-day:visible:nth(24)').click('bottom', { force: true }); cy.get(`[style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(6)`).should('contain', '2009-01-21'); // // change Effort Driven diff --git a/test/cypress/e2e/example16.cy.ts b/test/cypress/e2e/example16.cy.ts index f819850ba..a2a1fd803 100644 --- a/test/cypress/e2e/example16.cy.ts +++ b/test/cypress/e2e/example16.cy.ts @@ -1,4 +1,4 @@ -describe('Example 16: Grid State & Presets using Local Storage', { retries: 0 }, () => { +describe('Example 16: Grid State & Presets using Local Storage', { retries: 1 }, () => { const fullEnglishTitles = ['', 'Title', 'Description', 'Duration', '% Complete', 'Start', 'Completed']; const fullFrenchTitles = ['', 'Titre', 'Description', 'Durée', '% Achevée', 'Début', 'Terminé']; @@ -135,7 +135,7 @@ describe('Example 16: Grid State & Presets using Local Storage', { retries: 0 }, .should('be.visible'); cy.get('.filter-title input') - .type('Task 1') + .type('Task 1'); }); it('should click on "Title" column to sort it Ascending', () => { @@ -212,7 +212,7 @@ describe('Example 16: Grid State & Presets using Local Storage', { retries: 0 }, cy.get('@grid16') .find('#items-per-page-label').select('20'); - cy.get('@grid16') + cy.get('@grid16'); cy.get('.icon-seek-next').click().click(); cy.wait(100); @@ -319,7 +319,7 @@ describe('Example 16: Grid State & Presets using Local Storage', { retries: 0 }, .parent() .children() .each($child => { - console.log($child) + console.log($child); expect($child.attr('class')).to.contain('selected'); }); }); @@ -467,7 +467,7 @@ describe('Example 16: Grid State & Presets using Local Storage', { retries: 0 }, .parent() .children() .each($child => { - console.log($child) + console.log($child); expect($child.attr('class')).to.contain('selected'); }); }); @@ -485,7 +485,7 @@ describe('Example 16: Grid State & Presets using Local Storage', { retries: 0 }, .parent() .children() .each($child => { - console.log($child) + console.log($child); expect($child.attr('class')).to.contain('selected'); }); });