Skip to content

Commit 1be8d8e

Browse files
rrahirrrahir
committed
[FIX] viewport: remove snap-friendly margins
Since the introduction of the smooth viewports, the "comfort" margins at the end of the main viewport are no longer necessary. They were introduced to ensure that the viewport could be snapped properly. Task: 4633503 Part-of: #5988 Signed-off-by: Pierre Rousseau (pro) <[email protected]>
1 parent 2b3b8d9 commit 1be8d8e

File tree

10 files changed

+45
-68
lines changed

10 files changed

+45
-68
lines changed

‎src/components/spreadsheet/spreadsheet.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ css/* scss */ `
224224
> canvas {
225225
box-sizing: content-box;
226226
border-bottom: 1px solid #e2e3e3;
227+
border-right: 1px solid #e2e3e3;
227228
}
228229
229230
.o-grid-overlay {

‎src/helpers/internal_viewport.ts‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH, FOOTER_HEIGHT } from "../constants";
1+
import { FOOTER_HEIGHT } from "../constants";
22
import {
33
DOMCoordinates,
44
DOMDimension,
@@ -74,35 +74,17 @@ export class InternalViewport {
7474
first: this.boundaries.top,
7575
last: this.boundaries.bottom,
7676
});
77-
const { end: lastColEnd, size: lastColSize } = this.getters.getColDimensions(
78-
this.sheetId,
79-
lastCol
80-
);
81-
const { end: lastRowEnd, size: lastRowSize } = this.getters.getRowDimensions(
82-
this.sheetId,
83-
lastRow
84-
);
8577

86-
const leftColIndex = this.searchHeaderIndex("COL", lastColEnd - this.viewportWidth, 0);
87-
const leftColSize = this.getters.getColSize(this.sheetId, leftColIndex);
88-
const leftRowIndex = this.searchHeaderIndex("ROW", lastRowEnd - this.viewportHeight, 0);
89-
const topRowSize = this.getters.getRowSize(this.sheetId, leftRowIndex);
78+
const { end: lastColEnd } = this.getters.getColDimensions(this.sheetId, lastCol);
79+
const { end: lastRowEnd } = this.getters.getRowDimensions(this.sheetId, lastRow);
9080

9181
let width = lastColEnd - this.offsetCorrectionX;
9282
if (this.canScrollHorizontally) {
93-
width += Math.max(
94-
DEFAULT_CELL_WIDTH, // leave some minimal space to let the user know they scrolled all the way
95-
Math.min(leftColSize, this.viewportWidth - lastColSize) // Add pixels that allows the snapping at maximum horizontal scroll
96-
);
9783
width = Math.max(width, this.viewportWidth); // if the viewport grid size is smaller than its client width, return client width
9884
}
9985

10086
let height = lastRowEnd - this.offsetCorrectionY;
10187
if (this.canScrollVertically) {
102-
height += Math.max(
103-
DEFAULT_CELL_HEIGHT + 5, // leave some space to let the user know they scrolled all the way
104-
Math.min(topRowSize, this.viewportHeight - lastRowSize) // Add pixels that allows the snapping at maximum vertical scroll
105-
);
10688
height = Math.max(height, this.viewportHeight); // if the viewport grid size is smaller than its client height, return client height
10789

10890
if (lastRowEnd + FOOTER_HEIGHT > height && !this.getters.isReadonly()) {

‎src/plugins/ui_stateful/sheetview.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,8 +397,8 @@ export class SheetViewPlugin extends UIPlugin {
397397
const { width, height } = this.getMainViewportRect();
398398
const viewport = this.getMainInternalViewport(sheetId);
399399
return {
400-
maxOffsetX: Math.max(0, width - viewport.viewportWidth + 1),
401-
maxOffsetY: Math.max(0, height - viewport.viewportHeight + 1),
400+
maxOffsetX: Math.max(0, width - viewport.viewportWidth),
401+
maxOffsetY: Math.max(0, height - viewport.viewportHeight),
402402
};
403403
}
404404

‎tests/__snapshots__/renderer_store.test.ts.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ exports[`renderer snapshot for a simple grid rendering 1`] = `
77
"context.fillRect(0, 0, 952.5, 974.5)",
88
"context.save()",
99
"context.beginPath()",
10-
"context.rect(0, 0, 2592, 2374)",
10+
"context.rect(0, 0, 2496, 2346)",
1111
"context.clip()",
1212
"context.strokeStyle="#E2E3E3";",
1313
"context.lineWidth=0.4;",

‎tests/grid/__snapshots__/dashboard_grid_component.test.ts.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ exports[`Grid component in dashboard mode simple dashboard rendering snapshot 1`
4545
style="top:0px; right:0px; width:15px; bottom:0px; "
4646
>
4747
<div
48-
style="width:1px; height:2328px; "
48+
style="width:1px; height:2300px; "
4949
/>
5050
</div>
5151
@@ -54,7 +54,7 @@ exports[`Grid component in dashboard mode simple dashboard rendering snapshot 1`
5454
style="left:0px; bottom:0px; height:15px; right:0px; "
5555
>
5656
<div
57-
style="width:2592px; height:1px; "
57+
style="width:2496px; height:1px; "
5858
/>
5959
</div>
6060

‎tests/grid/__snapshots__/grid_component.test.ts.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ exports[`Grid component simple rendering snapshot 1`] = `
125125
style="top:26px; right:0px; width:15px; bottom:0px; "
126126
>
127127
<div
128-
style="width:1px; height:2374px; "
128+
style="width:1px; height:2346px; "
129129
/>
130130
</div>
131131
@@ -134,7 +134,7 @@ exports[`Grid component simple rendering snapshot 1`] = `
134134
style="left:48px; bottom:0px; height:15px; right:0px; "
135135
>
136136
<div
137-
style="width:2592px; height:1px; "
137+
style="width:2496px; height:1px; "
138138
/>
139139
</div>
140140

‎tests/sheet/sheet_manipulation_plugin.test.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -941,8 +941,8 @@ describe("Rows", () => {
941941
});
942942
const newDimensions = model.getters.getMainViewportRect();
943943
expect(newDimensions).toMatchObject({
944-
width: 192, // col size + 1 DEFAULT_CELL_WIDTH for spacing
945-
height: 170, // sum of row sizes + 1 DEFAULT_CELL_HEIGHT + 5px for spacing + 46px for adding rows footer
944+
width: DEFAULT_CELL_WIDTH, // sum of col sizes
945+
height: 142, // sum of row sizes + 46px for adding rows footer
946946
});
947947
});
948948
test("On addition after", () => {
@@ -963,8 +963,8 @@ describe("Rows", () => {
963963
});
964964
const newDimensions = model.getters.getMainViewportRect();
965965
expect(newDimensions).toMatchObject({
966-
width: 192, // col size + 1 DEFAULT_CELL_WIDTH for spacing
967-
height: 190, // sum of row sizes + 1 DEFAULT_CELL_HEIGHT and 5px for spacing + 46px for adding rows footer
966+
width: DEFAULT_CELL_WIDTH, // sum of col sizes
967+
height: 162, // sum of row sizes + 46px for adding rows footer
968968
});
969969
expect(model.getters.getNumberRows(sheetId)).toBe(6);
970970
});
@@ -983,16 +983,16 @@ describe("Rows", () => {
983983
});
984984
let dimensions = model.getters.getMainViewportRect();
985985
expect(dimensions).toMatchObject({
986-
width: 192, // col size + 1 DEFAULT_CELL_WIDTH for spacing
987-
height: 170, // sum of row sizes + 1 DEFAULT_CELL_HEIGHT and 5px for spacing + 46px for adding rows footer
986+
width: DEFAULT_CELL_WIDTH, // sum of col sizes
987+
height: 142, // sum of row sizes + 46px for adding rows footer
988988
});
989989
const to = model.getters.getActiveSheetId();
990990
createSheet(model, { activate: true, sheetId: "42" });
991991
activateSheet(model, to);
992992
dimensions = model.getters.getMainViewportRect();
993993
expect(dimensions).toMatchObject({
994-
width: 192, // col size + 1 DEFAULT_CELL_WIDTH for spacing
995-
height: 170, // sum of row sizes + 1 DEFAULT_CELL_HEIGHT and 5px for spacing + 46px for adding rows footer
994+
width: DEFAULT_CELL_WIDTH, // sum of col sizes
995+
height: 142, // sum of row sizes + 46px for adding rows footer
996996
});
997997
});
998998
});

‎tests/sheet/sheetview_plugin.test.ts‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ describe("Viewport of Simple sheet", () => {
158158
expect(model.getters.getActiveMainViewport()).toMatchObject({
159159
left: 0,
160160
right: 10,
161-
top: 59,
161+
top: 58,
162162
bottom: 99,
163163
});
164164
const { height } = model.getters.getMainViewportRect();
@@ -171,8 +171,8 @@ describe("Viewport of Simple sheet", () => {
171171
expect(model.getters.getActiveMainViewport()).toMatchObject({
172172
left: 0,
173173
right: 10,
174-
top: 59,
175-
bottom: 103,
174+
top: 58,
175+
bottom: 101,
176176
});
177177
expect(model.getters.getActiveSheetScrollInfo()).toMatchObject({
178178
scrollX: 0,
@@ -214,15 +214,15 @@ describe("Viewport of Simple sheet", () => {
214214
scrollY: 0,
215215
});
216216

217-
setViewportOffset(model, DEFAULT_CELL_WIDTH * 16, 0);
217+
setViewportOffset(model, DEFAULT_CELL_WIDTH * 14, 0);
218218
expect(model.getters.getActiveMainViewport()).toMatchObject({
219219
top: 0,
220220
bottom: 43,
221-
left: 16,
222-
right: 25,
221+
left: 14,
222+
right: 24,
223223
});
224224
expect(model.getters.getActiveSheetScrollInfo()).toMatchObject({
225-
scrollX: DEFAULT_CELL_WIDTH * 16,
225+
scrollX: DEFAULT_CELL_WIDTH * 14,
226226
scrollY: 0,
227227
});
228228

@@ -269,15 +269,15 @@ describe("Viewport of Simple sheet", () => {
269269
right: 25,
270270
});
271271

272-
setViewportOffset(model, DEFAULT_CELL_WIDTH * 16, 0);
272+
const { width: sheetViewWidth } = model.getters.getSheetViewDimension();
273273
expect(model.getters.getActiveMainViewport()).toMatchObject({
274274
top: 5,
275275
bottom: 43,
276-
left: 20,
276+
left: 19,
277277
right: 25,
278278
});
279279
expect(model.getters.getActiveSheetScrollInfo()).toMatchObject({
280-
scrollX: DEFAULT_CELL_WIDTH * 16,
280+
scrollX: 26 * DEFAULT_CELL_WIDTH - sheetViewWidth, // fully scrolled
281281
scrollY: 0,
282282
});
283283
});
@@ -429,11 +429,11 @@ describe("Viewport of Simple sheet", () => {
429429
const nRows = model.getters.getNumberRows(sheetId);
430430
setViewportOffset(model, nCols * DEFAULT_CELL_WIDTH + 10, nRows * DEFAULT_CELL_HEIGHT + 10);
431431

432-
const maxOffsetX = DEFAULT_CELL_WIDTH * (nCols - 10 + 1);
433-
const maxOffsetY = DEFAULT_CELL_HEIGHT * (nRows - 10 + 1) + 46;
432+
const maxOffsetX = DEFAULT_CELL_WIDTH * (nCols - 10);
433+
const maxOffsetY = DEFAULT_CELL_HEIGHT * (nRows - 10) + 46;
434434
expect(model.getters.getActiveSheetScrollInfo()).toEqual({
435-
scrollX: maxOffsetX + 1,
436-
scrollY: maxOffsetY + 1 + 5,
435+
scrollX: maxOffsetX,
436+
scrollY: maxOffsetY,
437437
});
438438
});
439439

@@ -471,13 +471,7 @@ describe("Viewport of Simple sheet", () => {
471471
expect(model.getters.getActiveMainViewport()).toMatchObject({
472472
top: 0,
473473
bottom: 43,
474-
left: 7,
475-
right: 25,
476-
});
477-
expect(model.getters.getActiveMainViewport()).toMatchObject({
478-
top: 0,
479-
bottom: 43,
480-
left: 7,
474+
left: 5,
481475
right: 25,
482476
});
483477
const { width: sheetViewWidth } = model.getters.getSheetViewDimension();
@@ -518,7 +512,7 @@ describe("Viewport of Simple sheet", () => {
518512
});
519513
resizeRows(model, [...Array(numberRows).keys()], DEFAULT_CELL_HEIGHT / 2);
520514
expect(model.getters.getActiveMainViewport()).toMatchObject({
521-
top: 22,
515+
top: 20,
522516
bottom: 99,
523517
left: 0,
524518
right: 10,
@@ -553,11 +547,11 @@ describe("Viewport of Simple sheet", () => {
553547
expect(model.getters.getActiveMainViewport()).toMatchObject({
554548
top: viewport.top,
555549
bottom: viewport.bottom,
556-
left: 3,
557-
right: viewport.right,
550+
left: 2,
551+
right: 12, // stops at the last visible column
558552
});
559553
expect(model.getters.getActiveSheetScrollInfo()).toMatchObject({
560-
scrollX: DEFAULT_CELL_WIDTH * 14 - model.getters.getSheetViewDimension().width,
554+
scrollX: DEFAULT_CELL_WIDTH * 13 - model.getters.getSheetViewDimension().width,
561555
scrollY: 0,
562556
});
563557
});
@@ -582,7 +576,7 @@ describe("Viewport of Simple sheet", () => {
582576
expect(model.getters.getActiveMainViewport()).toMatchObject(viewport);
583577
hideRows(model, range(60, 100));
584578
expect(model.getters.getActiveMainViewport()).toMatchObject({
585-
top: 19,
579+
top: 18,
586580
bottom: 99,
587581
left: viewport.left,
588582
right: viewport.right,
@@ -1633,8 +1627,8 @@ describe("shift viewport up/down", () => {
16331627
selectCell(model, selectedCell);
16341628
model.dispatch("SHIFT_VIEWPORT_DOWN");
16351629
expect(model.getters.getSelectedZone()).toEqual({
1636-
top: toZone(selectedCell).top + 5,
1637-
bottom: toZone(selectedCell).bottom + 5,
1630+
top: toZone(selectedCell).top + 4,
1631+
bottom: toZone(selectedCell).bottom + 4,
16381632
left: 0,
16391633
right: 0,
16401634
});

‎tests/spreadsheet/__snapshots__/spreadsheet_component.test.ts.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,7 @@ exports[`Simple Spreadsheet Component simple rendering snapshot 1`] = `
764764
style="top:26px; right:0px; width:15px; bottom:0px; "
765765
>
766766
<div
767-
style="width:1px; height:2374px; "
767+
style="width:1px; height:2346px; "
768768
/>
769769
</div>
770770
@@ -773,7 +773,7 @@ exports[`Simple Spreadsheet Component simple rendering snapshot 1`] = `
773773
style="left:48px; bottom:0px; height:15px; right:0px; "
774774
>
775775
<div
776-
style="width:2592px; height:1px; "
776+
style="width:2496px; height:1px; "
777777
/>
778778
</div>
779779

‎tests/xlsx/__snapshots__/xlsx_export.test.ts.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26076,7 +26076,7 @@ exports[`Test XLSX export Images image larger than the sheet 1`] = `
2607626076
24
2607726077
</xdr:col>
2607826078
<xdr:colOff>
26079-
954338325
26079+
953423925
2608026080
</xdr:colOff>
2608126081
<xdr:row>
2608226082
24
@@ -26098,7 +26098,7 @@ exports[`Test XLSX export Images image larger than the sheet 1`] = `
2609826098
</xdr:blipFill>
2609926099
<xdr:spPr>
2610026100
<a:xfrm>
26101-
<a:ext cx="976274400" cy="962025000"/>
26101+
<a:ext cx="975360000" cy="962025000"/>
2610226102
</a:xfrm>
2610326103
<a:prstGeom prst="rect">
2610426104
<a:avLst/>

0 commit comments

Comments
 (0)