Skip to content

Commit 286a193

Browse files
committed
[IMP] renderer: draw cell background over grid lines
With this commit, the cell background are now drawn totally covering the grid lines. It's way prettier for table/pivots to have an uniform background color without grid lines breaking it. The downside is we cannot draw the hover overlay with a color alpha, because it would overlap with the next cell, making a darker color at the border. We can workaround by merging the colors programmatically and avoid drawing with alpha. Task: 4552232
1 parent 98ec926 commit 286a193

File tree

2 files changed

+57
-20
lines changed

2 files changed

+57
-20
lines changed

src/stores/grid_renderer_store.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { ModelStore, SpreadsheetStore } from ".";
2020
import { HoveredIconStore } from "../components/grid_overlay/hovered_icon_store";
2121
import { HoveredTableStore } from "../components/tables/hovered_table_store";
2222
import {
23+
blendColors,
2324
computeRotationPosition,
2425
computeTextFont,
2526
computeTextFontSizeInPixels,
@@ -211,31 +212,31 @@ export class GridRenderer extends SpreadsheetStore {
211212
const style = box.style;
212213
if (style.fillColor && style.fillColor !== "#ffffff") {
213214
ctx.fillStyle = style.fillColor || "#ffffff";
214-
ctx.fillRect(box.x, box.y, box.width, box.height);
215+
ctx.fillRect(box.x - 0.5, box.y - 0.5, box.width + 1, box.height + 1);
215216
}
216217
if (box.dataBarFill) {
217218
ctx.fillStyle = box.dataBarFill.color;
218219
const percentage = box.dataBarFill.percentage;
219220
const width = box.width * (percentage / 100);
220221
ctx.fillRect(box.x, box.y, width, box.height);
221222
}
223+
if (box.overlayColor) {
224+
ctx.fillStyle = blendColors(style.fillColor || "#ffffff", box.overlayColor);
225+
ctx.fillRect(box.x - 0.5, box.y - 0.5, box.width + 1, box.height + 1);
226+
}
222227
if (box?.chip) {
223228
ctx.save();
224229
ctx.beginPath();
225230
ctx.rect(box.x, box.y, box.width, box.height);
226231
ctx.clip();
227232
const chip = box.chip;
228-
ctx.fillStyle = chip.color;
233+
ctx.fillStyle = box.overlayColor ? blendColors(chip.color, box.overlayColor) : chip.color;
229234
const radius = 10;
230235
ctx.beginPath();
231236
ctx.roundRect(chip.x, chip.y, chip.width, chip.height, radius);
232237
ctx.fill();
233238
ctx.restore();
234239
}
235-
if (box.overlayColor) {
236-
ctx.fillStyle = box.overlayColor;
237-
ctx.fillRect(box.x, box.y, box.width, box.height);
238-
}
239240
if (box.isError) {
240241
ctx.fillStyle = "red";
241242
ctx.beginPath();

tests/renderer/renderer_store.test.ts

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,13 @@ import {
1515
import { Mode } from "@odoo/o-spreadsheet-engine/types/model";
1616
import { Model } from "../../src";
1717
import { HoveredTableStore } from "../../src/components/tables/hovered_table_store";
18-
import { fontSizeInPixels, getContextFontSize, toHex, toZone } from "../../src/helpers";
18+
import {
19+
blendColors,
20+
fontSizeInPixels,
21+
getContextFontSize,
22+
toHex,
23+
toZone,
24+
} from "../../src/helpers";
1925
import { FormulaFingerprintStore } from "../../src/stores/formula_fingerprints_store";
2026
import { GridRenderer } from "../../src/stores/grid_renderer_store";
2127
import { RendererStore } from "../../src/stores/renderer_store";
@@ -75,6 +81,20 @@ function getBoxFromText(gridRenderer: GridRenderer, text: string): Box {
7581
)!;
7682
}
7783

84+
/**
85+
* Cell fills are drawn with a 0.5 offset from the cell rect so they fill all of the cell and are not
86+
* affected with the 0.5 offset we use to make the canvas lines sharp.
87+
*/
88+
function removeOffsetOfFillStyles(fillStyles: any[]): any[] {
89+
return fillStyles.map((fs) => ({
90+
...fs,
91+
x: fs.x + 0.5,
92+
y: fs.y + 0.5,
93+
w: fs.w - 1,
94+
h: fs.h - 1,
95+
}));
96+
}
97+
7898
interface ContextObserver {
7999
onSet?(key, val): void;
80100
onGet?(key): void;
@@ -384,13 +404,17 @@ describe("renderer", () => {
384404

385405
drawGridRenderer(ctx);
386406

387-
expect(fillStyle).toEqual([{ color: "#DC6CDF", h: 23, w: 96, x: 0, y: 0 }]);
407+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([
408+
{ color: "#DC6CDF", h: 23, w: 96, x: 0, y: 0 },
409+
]);
388410

389411
fillStyle = [];
390412
setStyle(model, "A1", { fillColor: "#DC6CDE" });
391413
drawGridRenderer(ctx);
392414

393-
expect(fillStyle).toEqual([{ color: "#DC6CDE", h: 23, w: 96, x: 0, y: 0 }]);
415+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([
416+
{ color: "#DC6CDE", h: 23, w: 96, x: 0, y: 0 },
417+
]);
394418
});
395419

396420
test("fillstyle of merge will be rendered for all cells in merge", () => {
@@ -430,13 +454,17 @@ describe("renderer", () => {
430454

431455
drawGridRenderer(ctx);
432456

433-
expect(fillStyle).toEqual([{ color: "#DC6CDF", h: 3 * 23, w: 96, x: 0, y: 0 }]);
457+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([
458+
{ color: "#DC6CDF", h: 3 * 23, w: 96, x: 0, y: 0 },
459+
]);
434460

435461
fillStyle = [];
436462
setStyle(model, "A1", { fillColor: "#DC6CDE" });
437463
drawGridRenderer(ctx);
438464

439-
expect(fillStyle).toEqual([{ color: "#DC6CDE", h: 3 * 23, w: 96, x: 0, y: 0 }]);
465+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([
466+
{ color: "#DC6CDE", h: 3 * 23, w: 96, x: 0, y: 0 },
467+
]);
440468
});
441469

442470
test("fillstyle of cell works with CF", () => {
@@ -468,21 +496,23 @@ describe("renderer", () => {
468496

469497
drawGridRenderer(ctx);
470498

471-
expect(fillStyle).toEqual([]);
499+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([]);
472500

473501
fillStyle = [];
474502
setCellContent(model, "A1", "1");
475503
drawGridRenderer(ctx);
476504

477-
expect(fillStyle).toEqual([{ color: "#DC6CDF", h: 23, w: 96, x: 0, y: 0 }]);
505+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([
506+
{ color: "#DC6CDF", h: 23, w: 96, x: 0, y: 0 },
507+
]);
478508
});
479509

480510
test("fill style of hovered clickable cells goes over regular fill style", () => {
481511
const { drawGridRenderer, model, container } = setRenderer(
482512
new Model({ sheets: [{ colNumber: 1, rowNumber: 3 }] })
483513
);
484514
const background = "#DC6CDF";
485-
const hoverColor = TABLE_HOVER_BACKGROUND_COLOR;
515+
const hoverColor = blendColors(background, TABLE_HOVER_BACKGROUND_COLOR);
486516
createTable(model, "A1", { numberOfHeaders: 0 });
487517
setStyle(model, "A1", { fillColor: background });
488518
setCellContent(model, "A1", "Data");
@@ -507,13 +537,15 @@ describe("renderer", () => {
507537
});
508538

509539
drawGridRenderer(ctx);
510-
expect(fillStyles).toEqual([{ color: background, h: 23, w: 96, x: 0, y: 0 }]);
540+
expect(removeOffsetOfFillStyles(fillStyles)).toEqual([
541+
{ color: background, h: 23, w: 96, x: 0, y: 0 },
542+
]);
511543

512544
fillStyles = [];
513545
container.get(HoveredTableStore).hover({ col: 0, row: 0 });
514546
drawGridRenderer(ctx);
515547

516-
expect(fillStyles).toEqual([
548+
expect(removeOffsetOfFillStyles(fillStyles)).toEqual([
517549
{ color: background, h: 23, w: 96, x: 0, y: 0 },
518550
{ color: hoverColor, h: 23, w: 96, x: 0, y: 0 },
519551
]);
@@ -548,13 +580,15 @@ describe("renderer", () => {
548580

549581
drawGridRenderer(ctx);
550582

551-
expect(fillStyle).toEqual([]);
583+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([]);
552584

553585
fillStyle = [];
554586
setCellContent(model, "A1", "1");
555587
drawGridRenderer(ctx);
556588

557-
expect(fillStyle).toEqual([{ color: "#DC6CDF", h: 23 * 3, w: 96, x: 0, y: 0 }]);
589+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([
590+
{ color: "#DC6CDF", h: 23 * 3, w: 96, x: 0, y: 0 },
591+
]);
558592
});
559593

560594
test("formula fingerprints", () => {
@@ -848,7 +882,7 @@ describe("renderer", () => {
848882

849883
drawGridRenderer(ctx);
850884

851-
expect(fillStyle).toEqual([]);
885+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([]);
852886
fillStyle = [];
853887
const sheetId = model.getters.getActiveSheetId();
854888
const result = model.dispatch("ADD_CONDITIONAL_FORMAT", {
@@ -867,7 +901,9 @@ describe("renderer", () => {
867901
expect(result).toBeSuccessfullyDispatched();
868902
drawGridRenderer(ctx);
869903

870-
expect(fillStyle).toEqual([{ color: "#DC6CDF", h: 23, w: 96, x: 0, y: 0 }]);
904+
expect(removeOffsetOfFillStyles(fillStyle)).toEqual([
905+
{ color: "#DC6CDF", h: 23, w: 96, x: 0, y: 0 },
906+
]);
871907
});
872908

873909
test.each(["I am a very long text", "100000000000000"])(

0 commit comments

Comments
 (0)