Skip to content

Commit

Permalink
[IMP] Dashboard: Cache clickable cells
Browse files Browse the repository at this point in the history
The computation of the clickable cells visibility condition can
sometimes be costy (per cell). At the moment, we recompute those
conditions at each rendering request, which can become quickly costy
if we simply scroll the viewport.

This revision introduces a caching of this computations to smooth out
the user experience when scrolling a dashboard.

closes #4090

Task: 3869752
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
  • Loading branch information
rrahir committed May 10, 2024
1 parent 5ee2cad commit 7645461
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 58 deletions.
88 changes: 88 additions & 0 deletions src/components/dashboard/clickable_cell_store.ts
@@ -0,0 +1,88 @@
import { markRaw } from "@odoo/owl";
import { positionToZone, toXC } from "../../helpers";
import { CellClickableItem, clickableCellRegistry } from "../../registries/cell_clickable_registry";
import { SpreadsheetStore } from "../../stores/spreadsheet_store";
import {
CellPosition,
Command,
Rect,
SpreadsheetChildEnv,
UID,
invalidateEvaluationCommands,
} from "../../types";

type Garbage = ((position: CellPosition, env: SpreadsheetChildEnv) => void) | false;

export interface ClickableCell {
coordinates: Rect;
position: CellPosition;
action: (position: CellPosition, env: SpreadsheetChildEnv) => void;
}

export class ClickableCellsStore extends SpreadsheetStore {
private _clickableCells: Record<UID, Record<string, Garbage>> = markRaw({});
private _registryItems: CellClickableItem[] = markRaw(
clickableCellRegistry.getAll().sort((a, b) => a.sequence - b.sequence)
);

handle(cmd: Command) {
if (
invalidateEvaluationCommands.has(cmd.type) ||
cmd.type === "EVALUATE_CELLS" ||
(cmd.type === "UPDATE_CELL" && ("content" in cmd || "format" in cmd))
) {
this._clickableCells = markRaw({});
this._registryItems = markRaw(
clickableCellRegistry.getAll().sort((a, b) => a.sequence - b.sequence)
);
}
}

private getClickableAction(position: CellPosition): Garbage {
const { sheetId, col, row } = position;
const clickableCells = this._clickableCells;
const xc = toXC(col, row);
if (!clickableCells[sheetId]) {
clickableCells[sheetId] = {};
}
if (!(xc in clickableCells[sheetId]!)) {
clickableCells[sheetId][xc] = this.findClickableAction(position);
}
return clickableCells[sheetId][xc];
}

private findClickableAction(position: CellPosition) {
const getters = this.getters;
for (const item of this._registryItems) {
if (item.condition(position, getters)) {
return item.execute;
}
}
return false;
}

get clickableCells(): ClickableCell[] {
const cells: ClickableCell[] = [];
const getters = this.getters;
const sheetId = getters.getActiveSheetId();
for (const col of getters.getSheetViewVisibleCols()) {
for (const row of getters.getSheetViewVisibleRows()) {
const position = { sheetId, col, row };
if (!getters.isMainCellPosition(position)) {
continue;
}
const action = this.getClickableAction(position);
if (!action) {
continue;
}
const zone = getters.expandZone(sheetId, positionToZone(position));
cells.push({
coordinates: getters.getVisibleRect(zone),
position,
action,
});
}
}
return cells;
}
}
60 changes: 7 additions & 53 deletions src/components/dashboard/dashboard.ts
@@ -1,17 +1,6 @@
import { Component, useChildSubEnv, useRef } from "@odoo/owl";
import { positionToZone } from "../../helpers/zones";
import { clickableCellRegistry } from "../../registries/cell_clickable_registry";
import { Component, toRaw, useChildSubEnv, useRef } from "@odoo/owl";
import { Store, useStore } from "../../store_engine";
import {
CellPosition,
DOMCoordinates,
DOMDimension,
Pixel,
Position,
Rect,
SpreadsheetChildEnv,
Zone,
} from "../../types/index";
import { DOMCoordinates, DOMDimension, Pixel, Rect, SpreadsheetChildEnv } from "../../types/index";
import { HoveredCellStore } from "../grid/hovered_cell_store";
import { GridOverlay } from "../grid_overlay/grid_overlay";
import { GridPopover } from "../grid_popover/grid_popover";
Expand All @@ -22,15 +11,10 @@ import { useWheelHandler } from "../helpers/wheel_hook";
import { CellPopoverStore } from "../popover";
import { Popover } from "../popover/popover";
import { HorizontalScrollBar, VerticalScrollBar } from "../scrollbar/";
import { ClickableCell, ClickableCellsStore } from "./clickable_cell_store";

interface Props {}

interface ClickableCell {
coordinates: Rect;
position: Position;
action: (position: CellPosition, env: SpreadsheetChildEnv) => void;
}

css/* scss */ `
.o-dashboard-clickable-cell {
position: absolute;
Expand All @@ -54,11 +38,13 @@ export class SpreadsheetDashboard extends Component<Props, SpreadsheetChildEnv>
onMouseWheel!: (ev: WheelEvent) => void;
canvasPosition!: DOMCoordinates;
hoveredCell!: Store<HoveredCellStore>;
clickableCellsStore!: Store<ClickableCellsStore>;

setup() {
const gridRef = useRef("grid");
this.canvasPosition = useAbsoluteBoundingRect(gridRef);
this.hoveredCell = useStore(HoveredCellStore);
this.clickableCellsStore = useStore(ClickableCellsStore);

useChildSubEnv({ getPopoverContainerRect: () => this.getGridRect() });
useGridDrawing("canvas", this.env.model, () => this.env.model.getters.getSheetViewDimension());
Expand Down Expand Up @@ -103,44 +89,12 @@ export class SpreadsheetDashboard extends Component<Props, SpreadsheetChildEnv>
*
*/
getClickableCells(): ClickableCell[] {
const cells: ClickableCell[] = [];
const sheetId = this.env.model.getters.getActiveSheetId();
for (const col of this.env.model.getters.getSheetViewVisibleCols()) {
for (const row of this.env.model.getters.getSheetViewVisibleRows()) {
const position = { sheetId, col, row };
const action = this.getClickableAction(position);
if (!action) {
continue;
}
let zone: Zone;
if (this.env.model.getters.isInMerge(position)) {
zone = this.env.model.getters.getMerge(position)!;
} else {
zone = positionToZone({ col, row });
}
const rect = this.env.model.getters.getVisibleRect(zone);
cells.push({
coordinates: rect,
position: { col, row },
action,
});
}
}
return cells;
}

getClickableAction(position: CellPosition) {
for (const items of clickableCellRegistry.getAll().sort((a, b) => a.sequence - b.sequence)) {
if (items.condition(position, this.env)) {
return items.execute;
}
}
return false;
return toRaw(this.clickableCellsStore.clickableCells);
}

selectClickableCell(clickableCell: ClickableCell) {
const { position, action } = clickableCell;
action({ ...position, sheetId: this.env.model.getters.getActiveSheetId() }, this.env);
action(position, this.env);
}

onClosePopover() {
Expand Down
9 changes: 5 additions & 4 deletions src/registries/cell_clickable_registry.ts
@@ -1,18 +1,19 @@
import { openLink } from "../helpers/links";
import { CellPosition, SpreadsheetChildEnv } from "../types";
import { CellPosition, Getters, SpreadsheetChildEnv } from "../types";
import { Registry } from "./registry";

export interface CellClickableItem {
condition: (position: CellPosition, env: SpreadsheetChildEnv) => boolean;
condition: (position: CellPosition, getters: Getters) => boolean;
execute: (position: CellPosition, env: SpreadsheetChildEnv) => void;
sequence: number;
}

export const clickableCellRegistry = new Registry<CellClickableItem>();

clickableCellRegistry.add("link", {
condition: (position: CellPosition, env: SpreadsheetChildEnv) =>
!!env.model.getters.getEvaluatedCell(position).link,
condition: (position: CellPosition, getters: Getters) => {
return !!getters.getEvaluatedCell(position).link;
},
execute: (position: CellPosition, env: SpreadsheetChildEnv) =>
openLink(env.model.getters.getEvaluatedCell(position).link!, env),
sequence: 5,
Expand Down
4 changes: 3 additions & 1 deletion tests/grid/dashboard_grid_component.test.ts
Expand Up @@ -124,7 +124,9 @@ describe("Grid component in dashboard mode", () => {
test("Clickable cells actions are properly udpated on viewport scroll", async () => {
const fn = jest.fn();
clickableCellRegistry.add("fake", {
condition: (position, env) => !!env.model.getters.getCell(position)?.content.startsWith("__"),
condition: (position, getters) => {
return !!getters.getCell(position)?.content.startsWith("__");
},
execute: (position) => fn(position.col, position.row),
sequence: 5,
});
Expand Down

0 comments on commit 7645461

Please sign in to comment.