From b5a51a5b28ed30aa5b4c7555b036396f5ab07451 Mon Sep 17 00:00:00 2001 From: Jaume Pujantell Date: Tue, 3 Dec 2024 17:34:50 +0100 Subject: [PATCH] calc: update viewedRectangle on status message viewedRectangle was not properly being updated when new document sizes were received via status message. This sometimes caused the view to not scroll properly when using arrow keys to select cells just after opening a file. Tried to add a cypress test but couldn't get the error to trigger on cypress. Some test have been updated due to changed scrollbar sizes, since the bar size depends on the document size. This change also broke 'Jump on search with not visible cursor' test. Fixed by ensuring the cursor is followed on search. Signed-off-by: Jaume Pujantell Change-Id: I48058b0ac5df70ca14a0a7c32e1f9b697fa37f81 --- browser/src/layer/tile/CanvasTileLayer.js | 2 +- browser/src/map/Map.js | 8 ++++++-- .../desktop/calc/cell_cursor_spec.js | 6 +++--- .../integration_tests/desktop/calc/scrolling_spec.js | 4 ++-- .../desktop/calc/sheet_switch_spec.js | 6 +++--- .../multiuser/calc/cell_cursor_spec.js | 12 ++++++------ 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/browser/src/layer/tile/CanvasTileLayer.js b/browser/src/layer/tile/CanvasTileLayer.js index b4e796e5a2d9d..3b3eddacf5ecd 100644 --- a/browser/src/layer/tile/CanvasTileLayer.js +++ b/browser/src/layer/tile/CanvasTileLayer.js @@ -4160,7 +4160,7 @@ L.CanvasTileLayer = L.Layer.extend({ } if (oldSize.x !== newSize.x || oldSize.y !== newSize.y) { - this._map.invalidateSize(); + this._map.invalidateSize({}, oldSize); } var hasMobileWizardOpened = this._map.uiManager.mobileWizard ? this._map.uiManager.mobileWizard.isOpen() : false; diff --git a/browser/src/map/Map.js b/browser/src/map/Map.js index 4de117e2be1a8..32e361db6b748 100644 --- a/browser/src/map/Map.js +++ b/browser/src/map/Map.js @@ -794,7 +794,9 @@ L.Map = L.Evented.extend({ return this.panTo(newCenter, options); }, - invalidateSize: function (options) { + // If map size has already been updated, invalidateSize needs the oldSize to work properly + // (e.g. if getSize() has already been called whith _sizeChanged === true) + invalidateSize: function (options, oldSize) { if (!this._loaded) { return this; } options = L.extend({ @@ -802,7 +804,9 @@ L.Map = L.Evented.extend({ pan: false }, options === true ? {animate: true} : options); - var oldSize = this.getSize(); + if (!oldSize) { + oldSize = this.getSize(); + } this._sizeChanged = true; var newSize = this.getSize(), diff --git a/cypress_test/integration_tests/desktop/calc/cell_cursor_spec.js b/cypress_test/integration_tests/desktop/calc/cell_cursor_spec.js index 4cfc27d119a89..6605853373d22 100644 --- a/cypress_test/integration_tests/desktop/calc/cell_cursor_spec.js +++ b/cypress_test/integration_tests/desktop/calc/cell_cursor_spec.js @@ -14,11 +14,11 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Test jumping on large cell }); it('No jump on long merged cell', function() { - desktopHelper.assertScrollbarPosition('horizontal', 205, 315); + desktopHelper.assertScrollbarPosition('horizontal', 205, 320); calcHelper.clickOnFirstCell(true, false, false); cy.cGet(helper.addressInputSelector).should('have.value', 'A1:Z1'); - desktopHelper.assertScrollbarPosition('horizontal', 205, 315); + desktopHelper.assertScrollbarPosition('horizontal', 205, 320); }); it('Jump on address with not visible cursor', function() { @@ -30,7 +30,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Test jumping on large cell }); it('Jump on search with not visible cursor', function() { - desktopHelper.assertScrollbarPosition('horizontal', 205, 315); + desktopHelper.assertScrollbarPosition('horizontal', 205, 320); cy.cGet('input#search-input').clear().type('FIRST{enter}'); cy.cGet(helper.addressInputSelector).should('have.value', 'A10'); diff --git a/cypress_test/integration_tests/desktop/calc/scrolling_spec.js b/cypress_test/integration_tests/desktop/calc/scrolling_spec.js index 52ea3e696d6fa..bf474d9ef7a32 100644 --- a/cypress_test/integration_tests/desktop/calc/scrolling_spec.js +++ b/cypress_test/integration_tests/desktop/calc/scrolling_spec.js @@ -50,7 +50,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Scroll through document', } // Document should scroll - desktopHelper.assertScrollbarPosition('vertical', 250, 270); + desktopHelper.assertScrollbarPosition('vertical', 230, 250); // Document should not scroll horizontally desktopHelper.assertScrollbarPosition('horizontal', 48, 50); }); @@ -73,6 +73,6 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Scroll through document', } // Document should scroll - desktopHelper.assertScrollbarPosition('horizontal', 160, 190); + desktopHelper.assertScrollbarPosition('horizontal', 80, 110); }); }); diff --git a/cypress_test/integration_tests/desktop/calc/sheet_switch_spec.js b/cypress_test/integration_tests/desktop/calc/sheet_switch_spec.js index 3152291efc6c2..4ca78ba1d0481 100644 --- a/cypress_test/integration_tests/desktop/calc/sheet_switch_spec.js +++ b/cypress_test/integration_tests/desktop/calc/sheet_switch_spec.js @@ -27,15 +27,15 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Sheet switching tests', fu desktopHelper.assertScrollbarPosition('vertical', 15, 25); }); - it('Check view position on repeated selection of currently selected sheet', function() { + it.only('Check view position on repeated selection of currently selected sheet', function() { // initially we are on sheet 2 tab cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'F720'); - desktopHelper.assertScrollbarPosition('vertical', 330, 350); + desktopHelper.assertScrollbarPosition('vertical', 280, 350); // click on sheet 2 tab (yes, current one) cy.cGet('#spreadsheet-tab1').click(); cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'F720'); - desktopHelper.assertScrollbarPosition('vertical', 330, 350); + desktopHelper.assertScrollbarPosition('vertical', 280, 350); // go to different place in the spreadsheet cy.cGet(helper.addressInputSelector).type('{selectAll}A2{enter}'); diff --git a/cypress_test/integration_tests/multiuser/calc/cell_cursor_spec.js b/cypress_test/integration_tests/multiuser/calc/cell_cursor_spec.js index 769ed56d0a6f8..ec62d99c23707 100644 --- a/cypress_test/integration_tests/multiuser/calc/cell_cursor_spec.js +++ b/cypress_test/integration_tests/multiuser/calc/cell_cursor_spec.js @@ -22,7 +22,7 @@ describe(['tagmultiuser'], 'Check cell cursor and view behavior', function() { cy.cSetActiveFrame('#iframe1'); cy.cGet(helper.addressInputSelector).type('{selectAll}A400{enter}'); - desktopHelper.assertScrollbarPosition('vertical', 210, 240); + desktopHelper.assertScrollbarPosition('vertical', 185, 240); cy.cGet('#sc_input_window .ui-custom-textarea-text-layer').click(); cy.cGet('#sc_input_window .ui-custom-textarea-text-layer').type('some text{enter}'); @@ -31,7 +31,7 @@ describe(['tagmultiuser'], 'Check cell cursor and view behavior', function() { cy.cGet('#followingChip').click(); // verify that second view is scrolled to the: A400 - desktopHelper.assertScrollbarPosition('vertical', 210, 240); + desktopHelper.assertScrollbarPosition('vertical', 185, 240); // second view should still have cursor at the end: A588 cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'A588'); @@ -42,7 +42,7 @@ describe(['tagmultiuser'], 'Check cell cursor and view behavior', function() { // verify that second view is still at the: A400 cy.cSetActiveFrame('#iframe2'); - desktopHelper.assertScrollbarPosition('vertical', 210, 240); + desktopHelper.assertScrollbarPosition('vertical', 185, 240); // second view should still have cursor at the previous cell: A588+1 cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'A589'); @@ -59,13 +59,13 @@ describe(['tagmultiuser'], 'Check cell cursor and view behavior', function() { cy.cSetActiveFrame('#iframe1'); cy.cGet(helper.addressInputSelector).type('{selectAll}A400{enter}'); - desktopHelper.assertScrollbarPosition('vertical', 210, 240); + desktopHelper.assertScrollbarPosition('vertical', 185, 240); calcHelper.clickOnFirstCell(true, false, false); cy.cGet('#map').type('abc{enter}'); // second view should jump there cy.cSetActiveFrame('#iframe2'); - desktopHelper.assertScrollbarPosition('vertical', 210, 240); + desktopHelper.assertScrollbarPosition('vertical', 185, 240); // first view inserts sheet before current one cy.cSetActiveFrame('#iframe1'); @@ -83,6 +83,6 @@ describe(['tagmultiuser'], 'Check cell cursor and view behavior', function() { // first goes to second sheet and we should see A388 cy.cSetActiveFrame('#iframe1'); cy.cGet('#spreadsheet-tab1').click(); - desktopHelper.assertScrollbarPosition('vertical', 210, 240); + desktopHelper.assertScrollbarPosition('vertical', 185, 240); }); });