[FIX] viewport: remove snap-friendly margins

src/components/helpers/dom_helpers.ts

@@ -185,3 +185,10 @@ export function isMacOS(): boolean {
 export function isCtrlKey(ev: KeyboardEvent | MouseEvent): boolean {
   return isMacOS() ? ev.metaKey : ev.ctrlKey;
 }
+
+/**
+ * Detects if the current browser is Firefox
+ */
+export function isBrowserFirefox() {
+  return /Firefox/i.test(navigator.userAgent);
+}

src/components/scrollbar/scrollbar_horizontal.ts

@@ -1,6 +1,7 @@
 import { Component, xml } from "@odoo/owl";
 import { SCROLLBAR_WIDTH } from "../../constants";
 import { SpreadsheetChildEnv } from "../../types";
+import { isBrowserFirefox } from "../helpers/dom_helpers";
 import { ScrollBar } from "./scrollbar";
 
 interface Props {
@@ -46,7 +47,7 @@ export class HorizontalScrollBar extends Component<Props, SpreadsheetChildEnv> {
       left: `${this.props.leftOffset + x}px`,
       bottom: "0px",
       height: `${SCROLLBAR_WIDTH}px`,
-      right: `0px`,
+      right: isBrowserFirefox() ? `${SCROLLBAR_WIDTH}px` : "0",
     };
   }
 

src/components/scrollbar/scrollbar_vertical.ts

@@ -1,6 +1,7 @@
 import { Component, xml } from "@odoo/owl";
 import { SCROLLBAR_WIDTH } from "../../constants";
 import { SpreadsheetChildEnv } from "../../types";
+import { isBrowserFirefox } from "../helpers/dom_helpers";
 import { ScrollBar } from "./scrollbar";
 
 interface Props {
@@ -46,7 +47,7 @@ export class VerticalScrollBar extends Component<Props, SpreadsheetChildEnv> {
       top: `${this.props.topOffset + y}px`,
       right: "0px",
       width: `${SCROLLBAR_WIDTH}px`,
-      bottom: `0px`,
+      bottom: isBrowserFirefox() ? `${SCROLLBAR_WIDTH}px` : "0",
     };
   }
 

src/components/spreadsheet/spreadsheet.ts

@@ -224,6 +224,7 @@ css/* scss */ `
     > canvas {
       box-sizing: content-box;
       border-bottom: 1px solid #e2e3e3;
+      border-right: 1px solid #e2e3e3;
     }
 
     .o-grid-overlay {

src/helpers/internal_viewport.ts

@@ -1,4 +1,4 @@
-import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH, FOOTER_HEIGHT } from "../constants";
+import { FOOTER_HEIGHT } from "../constants";
 import {
   DOMCoordinates,
   DOMDimension,
@@ -74,35 +74,17 @@ export class InternalViewport {
       first: this.boundaries.top,
       last: this.boundaries.bottom,
     });
-    const { end: lastColEnd, size: lastColSize } = this.getters.getColDimensions(
-      this.sheetId,
-      lastCol
-    );
-    const { end: lastRowEnd, size: lastRowSize } = this.getters.getRowDimensions(
-      this.sheetId,
-      lastRow
-    );
 
-    const leftColIndex = this.searchHeaderIndex("COL", lastColEnd - this.viewportWidth, 0);
-    const leftColSize = this.getters.getColSize(this.sheetId, leftColIndex);
-    const leftRowIndex = this.searchHeaderIndex("ROW", lastRowEnd - this.viewportHeight, 0);
-    const topRowSize = this.getters.getRowSize(this.sheetId, leftRowIndex);
+    const { end: lastColEnd } = this.getters.getColDimensions(this.sheetId, lastCol);
+    const { end: lastRowEnd } = this.getters.getRowDimensions(this.sheetId, lastRow);
 
     let width = lastColEnd - this.offsetCorrectionX;
     if (this.canScrollHorizontally) {
-      width += Math.max(
-        DEFAULT_CELL_WIDTH, // leave some minimal space to let the user know they scrolled all the way
-        Math.min(leftColSize, this.viewportWidth - lastColSize) // Add pixels that allows the snapping at maximum horizontal scroll
-      );
       width = Math.max(width, this.viewportWidth); // if the viewport grid size is smaller than its client width, return client width
     }
 
     let height = lastRowEnd - this.offsetCorrectionY;
     if (this.canScrollVertically) {
-      height += Math.max(
-        DEFAULT_CELL_HEIGHT + 5, // leave some space to let the user know they scrolled all the way
-        Math.min(topRowSize, this.viewportHeight - lastRowSize) // Add pixels that allows the snapping at maximum vertical scroll
-      );
       height = Math.max(height, this.viewportHeight); // if the viewport grid size is smaller than its client height, return client height
 
       if (lastRowEnd + FOOTER_HEIGHT > height && !this.getters.isReadonly()) {

src/plugins/ui_stateful/sheetview.ts

@@ -397,8 +397,8 @@ export class SheetViewPlugin extends UIPlugin {
     const { width, height } = this.getMainViewportRect();
     const viewport = this.getMainInternalViewport(sheetId);
     return {
-      maxOffsetX: Math.max(0, width - viewport.viewportWidth + 1),
-      maxOffsetY: Math.max(0, height - viewport.viewportHeight + 1),
+      maxOffsetX: Math.max(0, width - viewport.viewportWidth),
+      maxOffsetY: Math.max(0, height - viewport.viewportHeight),
     };
   }
 

tests/__snapshots__/renderer_store.test.ts.snap

@@ -7,7 +7,7 @@ exports[`renderer snapshot for a simple grid rendering 1`] = `
   "context.fillRect(0, 0, 952.5, 974.5)",
   "context.save()",
   "context.beginPath()",
-  "context.rect(0, 0, 2592, 2374)",
+  "context.rect(0, 0, 2496, 2346)",
   "context.clip()",
   "context.strokeStyle="#E2E3E3";",
   "context.lineWidth=0.4;",

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

@@ -42,19 +42,19 @@ exports[`Grid component in dashboard mode simple dashboard rendering snapshot 1`
   </div>
   <div
     class="o-scrollbar vertical"
-    style="top:0px; right:0px; width:15px; bottom:0px; "
+    style="top:0px; right:0px; width:15px; bottom:0; "
   >
     <div
-      style="width:1px; height:2328px; "
+      style="width:1px; height:2300px; "
     />
   </div>
 
   <div
     class="o-scrollbar horizontal"
-    style="left:0px; bottom:0px; height:15px; right:0px; "
+    style="left:0px; bottom:0px; height:15px; right:0; "
   >
     <div
-      style="width:2592px; height:1px; "
+      style="width:2496px; height:1px; "
     />
   </div>
 

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

@@ -122,19 +122,19 @@ exports[`Grid component simple rendering snapshot 1`] = `
 
   <div
     class="o-scrollbar vertical"
-    style="top:26px; right:0px; width:15px; bottom:0px; "
+    style="top:26px; right:0px; width:15px; bottom:0; "
   >
     <div
-      style="width:1px; height:2374px; "
+      style="width:1px; height:2346px; "
     />
   </div>
 
   <div
     class="o-scrollbar horizontal"
-    style="left:48px; bottom:0px; height:15px; right:0px; "
+    style="left:48px; bottom:0px; height:15px; right:0; "
   >
     <div
-      style="width:2592px; height:1px; "
+      style="width:2496px; height:1px; "
     />
   </div>
 

tests/sheet/sheet_manipulation_plugin.test.ts

@@ -941,8 +941,8 @@ describe("Rows", () => {
       });
       const newDimensions = model.getters.getMainViewportRect();
       expect(newDimensions).toMatchObject({
-        width: 192, // col size + 1 DEFAULT_CELL_WIDTH for spacing
-        height: 170, // sum of row sizes + 1 DEFAULT_CELL_HEIGHT + 5px for spacing + 46px for adding rows footer
+        width: DEFAULT_CELL_WIDTH, // sum of col sizes
+        height: 142, // sum of row sizes  + 46px for adding rows footer
       });
     });
     test("On addition after", () => {
@@ -963,8 +963,8 @@ describe("Rows", () => {
       });
       const newDimensions = model.getters.getMainViewportRect();
       expect(newDimensions).toMatchObject({
-        width: 192, // col size + 1 DEFAULT_CELL_WIDTH for spacing
-        height: 190, // sum of row sizes + 1 DEFAULT_CELL_HEIGHT  and 5px for spacing + 46px for adding rows footer
+        width: DEFAULT_CELL_WIDTH, // sum of col sizes
+        height: 162, // sum of row sizes + 46px for adding rows footer
       });
       expect(model.getters.getNumberRows(sheetId)).toBe(6);
     });
@@ -983,16 +983,16 @@ describe("Rows", () => {
       });
       let dimensions = model.getters.getMainViewportRect();
       expect(dimensions).toMatchObject({
-        width: 192, // col size + 1 DEFAULT_CELL_WIDTH for spacing
-        height: 170, // sum of row sizes + 1 DEFAULT_CELL_HEIGHT  and 5px for spacing + 46px for adding rows footer
+        width: DEFAULT_CELL_WIDTH, // sum of col sizes
+        height: 142, // sum of row sizes + 46px for adding rows footer
       });
       const to = model.getters.getActiveSheetId();
       createSheet(model, { activate: true, sheetId: "42" });
       activateSheet(model, to);
       dimensions = model.getters.getMainViewportRect();
       expect(dimensions).toMatchObject({
-        width: 192, // col size + 1 DEFAULT_CELL_WIDTH for spacing
-        height: 170, // sum of row sizes + 1 DEFAULT_CELL_HEIGHT  and 5px for spacing + 46px for adding rows footer
+        width: DEFAULT_CELL_WIDTH, // sum of col sizes
+        height: 142, // sum of row sizes + 46px for adding rows footer
       });
     });
   });

tests/sheet/sheetview_plugin.test.ts

@@ -158,7 +158,7 @@ describe("Viewport of Simple sheet", () => {
     expect(model.getters.getActiveMainViewport()).toMatchObject({
       left: 0,
       right: 10,
-      top: 59,
+      top: 58,
       bottom: 99,
     });
     const { height } = model.getters.getMainViewportRect();
@@ -171,8 +171,8 @@ describe("Viewport of Simple sheet", () => {
     expect(model.getters.getActiveMainViewport()).toMatchObject({
       left: 0,
       right: 10,
-      top: 59,
-      bottom: 103,
+      top: 58,
+      bottom: 101,
     });
     expect(model.getters.getActiveSheetScrollInfo()).toMatchObject({
       scrollX: 0,
@@ -214,15 +214,15 @@ describe("Viewport of Simple sheet", () => {
       scrollY: 0,
     });
 
-    setViewportOffset(model, DEFAULT_CELL_WIDTH * 16, 0);
+    setViewportOffset(model, DEFAULT_CELL_WIDTH * 14, 0);
     expect(model.getters.getActiveMainViewport()).toMatchObject({
       top: 0,
       bottom: 43,
-      left: 16,
-      right: 25,
+      left: 14,
+      right: 24,
     });
     expect(model.getters.getActiveSheetScrollInfo()).toMatchObject({
-      scrollX: DEFAULT_CELL_WIDTH * 16,
+      scrollX: DEFAULT_CELL_WIDTH * 14,
       scrollY: 0,
     });
 
@@ -269,15 +269,15 @@ describe("Viewport of Simple sheet", () => {
       right: 25,
     });
 
-    setViewportOffset(model, DEFAULT_CELL_WIDTH * 16, 0);
+    const { width: sheetViewWidth } = model.getters.getSheetViewDimension();
     expect(model.getters.getActiveMainViewport()).toMatchObject({
       top: 5,
       bottom: 43,
-      left: 20,
+      left: 19,
       right: 25,
     });
     expect(model.getters.getActiveSheetScrollInfo()).toMatchObject({
-      scrollX: DEFAULT_CELL_WIDTH * 16,
+      scrollX: 26 * DEFAULT_CELL_WIDTH - sheetViewWidth, // fully scrolled
       scrollY: 0,
     });
   });
@@ -429,11 +429,11 @@ describe("Viewport of Simple sheet", () => {
     const nRows = model.getters.getNumberRows(sheetId);
     setViewportOffset(model, nCols * DEFAULT_CELL_WIDTH + 10, nRows * DEFAULT_CELL_HEIGHT + 10);
 
-    const maxOffsetX = DEFAULT_CELL_WIDTH * (nCols - 10 + 1);
-    const maxOffsetY = DEFAULT_CELL_HEIGHT * (nRows - 10 + 1) + 46;
+    const maxOffsetX = DEFAULT_CELL_WIDTH * (nCols - 10);
+    const maxOffsetY = DEFAULT_CELL_HEIGHT * (nRows - 10) + 46;
     expect(model.getters.getActiveSheetScrollInfo()).toEqual({
-      scrollX: maxOffsetX + 1,
-      scrollY: maxOffsetY + 1 + 5,
+      scrollX: maxOffsetX,
+      scrollY: maxOffsetY,
     });
   });
 
@@ -471,13 +471,7 @@ describe("Viewport of Simple sheet", () => {
     expect(model.getters.getActiveMainViewport()).toMatchObject({
       top: 0,
       bottom: 43,
-      left: 7,
-      right: 25,
-    });
-    expect(model.getters.getActiveMainViewport()).toMatchObject({
-      top: 0,
-      bottom: 43,
-      left: 7,
+      left: 5,
       right: 25,
     });
     const { width: sheetViewWidth } = model.getters.getSheetViewDimension();
@@ -518,7 +512,7 @@ describe("Viewport of Simple sheet", () => {
     });
     resizeRows(model, [...Array(numberRows).keys()], DEFAULT_CELL_HEIGHT / 2);
     expect(model.getters.getActiveMainViewport()).toMatchObject({
-      top: 22,
+      top: 20,
       bottom: 99,
       left: 0,
       right: 10,
@@ -553,11 +547,11 @@ describe("Viewport of Simple sheet", () => {
     expect(model.getters.getActiveMainViewport()).toMatchObject({
       top: viewport.top,
       bottom: viewport.bottom,
-      left: 3,
-      right: viewport.right,
+      left: 2,
+      right: 12, // stops at the last visible column
     });
     expect(model.getters.getActiveSheetScrollInfo()).toMatchObject({
-      scrollX: DEFAULT_CELL_WIDTH * 14 - model.getters.getSheetViewDimension().width,
+      scrollX: DEFAULT_CELL_WIDTH * 13 - model.getters.getSheetViewDimension().width,
       scrollY: 0,
     });
   });
@@ -582,7 +576,7 @@ describe("Viewport of Simple sheet", () => {
     expect(model.getters.getActiveMainViewport()).toMatchObject(viewport);
     hideRows(model, range(60, 100));
     expect(model.getters.getActiveMainViewport()).toMatchObject({
-      top: 19,
+      top: 18,
       bottom: 99,
       left: viewport.left,
       right: viewport.right,
@@ -1633,8 +1627,8 @@ describe("shift viewport up/down", () => {
       selectCell(model, selectedCell);
       model.dispatch("SHIFT_VIEWPORT_DOWN");
       expect(model.getters.getSelectedZone()).toEqual({
-        top: toZone(selectedCell).top + 5,
-        bottom: toZone(selectedCell).bottom + 5,
+        top: toZone(selectedCell).top + 4,
+        bottom: toZone(selectedCell).bottom + 4,
         left: 0,
         right: 0,
       });

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

@@ -761,19 +761,19 @@ exports[`Simple Spreadsheet Component simple rendering snapshot 1`] = `
 
         <div
           class="o-scrollbar vertical"
-          style="top:26px; right:0px; width:15px; bottom:0px; "
+          style="top:26px; right:0px; width:15px; bottom:0; "
         >
           <div
-            style="width:1px; height:2374px; "
+            style="width:1px; height:2346px; "
           />
         </div>
 
         <div
           class="o-scrollbar horizontal"
-          style="left:48px; bottom:0px; height:15px; right:0px; "
+          style="left:48px; bottom:0px; height:15px; right:0; "
         >
           <div
-            style="width:2592px; height:1px; "
+            style="width:2496px; height:1px; "
           />
         </div>
 

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

@@ -26076,7 +26076,7 @@ exports[`Test XLSX export Images image larger than the sheet 1`] = `
                 24
             </xdr:col>
             <xdr:colOff>
-                954338325
+                953423925
             </xdr:colOff>
             <xdr:row>
                 24
@@ -26098,7 +26098,7 @@ exports[`Test XLSX export Images image larger than the sheet 1`] = `
             </xdr:blipFill>
             <xdr:spPr>
                 <a:xfrm>
-                    <a:ext cx="976274400" cy="962025000"/>
+                    <a:ext cx="975360000" cy="962025000"/>
                 </a:xfrm>
                 <a:prstGeom prst="rect">
                     <a:avLst/>