Skip to content

Commit

Permalink
calc: update viewedRectangle on status message
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Change-Id: I48058b0ac5df70ca14a0a7c32e1f9b697fa37f81
  • Loading branch information
@eszkadev
Jaume Pujantell authored and eszkadev committed Jan 9, 2025
1 parent 0fff143 commit b5a51a5
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 17 deletions.
2 changes: 1 addition & 1 deletion browser/src/layer/tile/CanvasTileLayer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 6 additions & 2 deletions browser/src/map/Map.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -794,15 +794,19 @@ 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({
animate: false,
pan: false
}, options === true ? {animate: true} : options);

var oldSize = this.getSize();
if (!oldSize) {
oldSize = this.getSize();
}
this._sizeChanged = true;

var newSize = this.getSize(),
Expand Down
6 changes: 3 additions & 3 deletions cypress_test/integration_tests/desktop/calc/cell_cursor_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions cypress_test/integration_tests/desktop/calc/scrolling_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -73,6 +73,6 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Scroll through document',
}

// Document should scroll
desktopHelper.assertScrollbarPosition('horizontal', 160, 190);
desktopHelper.assertScrollbarPosition('horizontal', 80, 110);
});
});
6 changes: 3 additions & 3 deletions cypress_test/integration_tests/desktop/calc/sheet_switch_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}');
Expand Down
12 changes: 6 additions & 6 deletions cypress_test/integration_tests/multiuser/calc/cell_cursor_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}');

Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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);
});
});

0 comments on commit b5a51a5

Please sign in to comment.