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.

Some test have been updated due to changed scrollbar sizes, since the
bar size depends on the document size.

Tried to add a cypress test but couldn't get the error to trigger on
cypress.

Signed-off-by: Jaume Pujantell <jaume.pujantell@collabora.com>
Change-Id: I48058b0ac5df70ca14a0a7c32e1f9b697fa37f81
  • Loading branch information
Jaume Pujantell committed Dec 3, 2024
1 parent 7168663 commit ab860f8
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 12 deletions.
2 changes: 1 addition & 1 deletion browser/src/layer/tile/CanvasTileLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4147,7 +4147,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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Scroll through document',
}

// Document should scroll
desktopHelper.assertScrollbarPosition('vertical', 50, 60);
desktopHelper.assertScrollbarPosition('vertical', 45, 55);
// Document should not scroll horizontally
desktopHelper.assertScrollbarPosition('horizontal', 48, 50);
});
Expand All @@ -71,6 +71,6 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Scroll through document',
}

// Document should scroll
desktopHelper.assertScrollbarPosition('horizontal', 160, 170);
desktopHelper.assertScrollbarPosition('horizontal', 80, 100);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,21 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Sheet switching tests', fu
// go to sheet 2
cy.cGet('#spreadsheet-tab1').click();
cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'F720');
desktopHelper.assertScrollbarPosition('vertical', 320, 335);
desktopHelper.assertScrollbarPosition('vertical', 320, 340);

cy.cGet(helper.addressInputSelector).type('{selectAll}A2{enter}');
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', 320, 335);
desktopHelper.assertScrollbarPosition('vertical', 280, 300);

// 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', 320, 335);
desktopHelper.assertScrollbarPosition('vertical', 280, 300);

// go to different place in the spreadsheet
cy.cGet(helper.addressInputSelector).type('{selectAll}A2{enter}');
Expand Down

0 comments on commit ab860f8

Please sign in to comment.