Skip to content

Commit

Permalink
[FIX] sheetview: pageUp/Down with frozen rows
Browse files Browse the repository at this point in the history
There was two issues with the commands `SHIFT_VIEWPORT_UP/DOWN` when
some rows were frozen:

- the viewport was never scrolled all the way down to the last row
with pageDown
- pageDown/Up scrolled more than a "page" (the size of the viewport)

This commit fixes both issues.

Also replaced calls from `getActiveMainViewport` to `getMainInternalViewport`
in the plugin `SheetView` as it needs slightly less computations.

closes #4027

Task: 3847414
X-original-commit: 51860d7
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
Signed-off-by: Adrien Minne (adrm) <adrm@odoo.com>
  • Loading branch information
hokolomopo committed Apr 10, 2024
1 parent 0fab128 commit 162a8f3
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 27 deletions.
41 changes: 14 additions & 27 deletions src/plugins/ui_stateful/sheetview.ts
Expand Up @@ -133,21 +133,21 @@ export class SheetViewPlugin extends UIPlugin {
}

private handleEvent(event: SelectionEvent) {
const sheetId = this.getters.getActiveSheetId();
if (event.options.scrollIntoView) {
let { col, row } = findCellInNewZone(event.previousAnchor.zone, event.anchor.zone);
if (event.mode === "updateAnchor") {
const oldZone = event.previousAnchor.zone;
const newZone = event.anchor.zone;
// altering a zone should not move the viewport in a dimension that wasn't changed
const { top, bottom, left, right } = this.getters.getActiveMainViewport();
const { top, bottom, left, right } = this.getMainInternalViewport(sheetId);
if (oldZone.left === newZone.left && oldZone.right === newZone.right) {
col = left > col || col > right ? left : col;
}
if (oldZone.top === newZone.top && oldZone.bottom === newZone.bottom) {
row = top > row || row > bottom ? top : row;
}
}
const sheetId = this.getters.getActiveSheetId();
col = Math.min(col, this.getters.getNumberCols(sheetId) - 1);
row = Math.min(row, this.getters.getNumberRows(sheetId) - 1);
this.refreshViewport(this.getters.getActiveSheetId(), { col, row });
Expand Down Expand Up @@ -175,20 +175,16 @@ export class SheetViewPlugin extends UIPlugin {
this.setSheetViewOffset(cmd.offsetX, cmd.offsetY);
break;
case "SHIFT_VIEWPORT_DOWN":
const { top } = this.getActiveMainViewport();
const sheetId = this.getters.getActiveSheetId();
const shiftedOffsetY = this.clipOffsetY(
this.getters.getRowDimensions(sheetId, top).start + this.sheetViewHeight
);
this.shiftVertically(shiftedOffsetY);
const { top, viewportHeight, offsetCorrectionY } = this.getMainInternalViewport(sheetId);
const topRowDims = this.getters.getRowDimensions(sheetId, top);
this.shiftVertically(topRowDims.start + viewportHeight - offsetCorrectionY);
break;
case "SHIFT_VIEWPORT_UP": {
const { top } = this.getActiveMainViewport();
const sheetId = this.getters.getActiveSheetId();
const shiftedOffsetY = this.clipOffsetY(
this.getters.getRowDimensions(sheetId, top).end - this.sheetViewHeight
);
this.shiftVertically(shiftedOffsetY);
const { top, viewportHeight, offsetCorrectionY } = this.getMainInternalViewport(sheetId);
const topRowDims = this.getters.getRowDimensions(sheetId, top);
this.shiftVertically(topRowDims.end - offsetCorrectionY - viewportHeight);
break;
}
case "REMOVE_COLUMNS_ROWS":
Expand Down Expand Up @@ -609,18 +605,6 @@ export class SheetViewPlugin extends UIPlugin {
);
}

/**
* Clip the vertical offset within the allowed range.
* Not above the sheet, nor below the sheet.
*/
private clipOffsetY(offsetY: Pixel): Pixel {
const { height } = this.getMainViewportRect();
const maxOffset = height - this.sheetViewHeight;
offsetY = Math.min(offsetY, maxOffset);
offsetY = Math.max(offsetY, 0);
return offsetY;
}

private getViewportOffset(sheetId: UID) {
return {
x: this.viewports[sheetId]?.bottomRight.offsetScrollbarX || 0,
Expand Down Expand Up @@ -711,12 +695,15 @@ export class SheetViewPlugin extends UIPlugin {
* viewport top.
*/
private shiftVertically(offset: Pixel) {
const { top } = this.getActiveMainViewport();
const sheetId = this.getters.getActiveSheetId();
const { top } = this.getMainInternalViewport(sheetId);
const { scrollX } = this.getActiveSheetScrollInfo();
this.setSheetViewOffset(scrollX, offset);
const { anchor } = this.getters.getSelection();
const deltaRow = this.getActiveMainViewport().top - top;
this.selection.selectCell(anchor.cell.col, anchor.cell.row + deltaRow);
if (anchor.cell.row >= this.getters.getPaneDivisions(sheetId).ySplit) {
const deltaRow = this.getMainInternalViewport(sheetId).top - top;
this.selection.selectCell(anchor.cell.col, anchor.cell.row + deltaRow);
}
}

getVisibleFigures(): Figure[] {
Expand Down
41 changes: 41 additions & 0 deletions tests/plugins/sheetview.test.ts
Expand Up @@ -1372,6 +1372,47 @@ describe("shift viewport up/down", () => {
});
});

describe("shift down/up with frozen panes", () => {
test("shift down/up with frozen rows and with selection in frozen rows", () => {
freezeRows(model, 5);
const { bottom, top } = model.getters.getActiveMainViewport();
model.dispatch("SHIFT_VIEWPORT_DOWN");
expect(model.getters.getActiveMainViewport().top).toBe(bottom);
expect(zoneToXc(model.getters.getSelectedZone())).toEqual("A1");
model.dispatch("SHIFT_VIEWPORT_UP");
expect(model.getters.getActiveMainViewport().top).toBe(top);
expect(zoneToXc(model.getters.getSelectedZone())).toEqual("A1");
});

test("shift down/up with frozen rows and with selection not in frozen rows", () => {
freezeRows(model, 5);
selectCell(model, "A6");
const { bottom, top } = model.getters.getActiveMainViewport();
model.dispatch("SHIFT_VIEWPORT_DOWN");
expect(model.getters.getActiveMainViewport().top).toBe(bottom);
expect(zoneToXc(model.getters.getSelectedZone())).toEqual(toXC(0, bottom));
model.dispatch("SHIFT_VIEWPORT_UP");
expect(model.getters.getActiveMainViewport().top).toBe(top);
expect(zoneToXc(model.getters.getSelectedZone())).toEqual("A6");
});

test("shift down scrolls until the last row if there are frozen rows", () => {
const sheetId = model.getters.getActiveSheetId();
const numberOfRows = model.getters.getNumberRows(sheetId);
freezeRows(model, 10);
let { bottom } = model.getters.getActiveMainViewport();
while (true) {
model.dispatch("SHIFT_VIEWPORT_DOWN");
const newBottom = model.getters.getActiveMainViewport().bottom;
if (newBottom === bottom) {
break;
}
bottom = newBottom;
}
expect(model.getters.getActiveMainViewport().bottom).toBe(numberOfRows - 1);
});
});

test.each(["A1", "A2"])(
"viewport and selection %s do not move when its already the end of the sheet",
(selectedCell) => {
Expand Down

0 comments on commit 162a8f3

Please sign in to comment.