Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion web/libs/editor/src/components/App/Grid.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,8 @@ const VirtualizedGrid = observer(({ store, annotations, root }) => {
initialHydrationDone.current = true;

const visibleCount = Math.ceil(containerWidth / (panelWidth + PANEL_GAP)) + 1;
const initialVisibleCount = Math.min(visibleCount, visibleAnnotations.length);
// Hydrate visible count + 1 so the next panel (e.g. 3rd in Compare All) is ready when needed (FIT-720 / FIT-1494)
const initialVisibleCount = Math.min(visibleCount + 1, visibleAnnotations.length);

for (let i = 0; i < initialVisibleCount; i++) {
const annotation = visibleAnnotations[i];
Expand Down
37 changes: 35 additions & 2 deletions web/libs/editor/src/components/App/__tests__/Grid.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react";
import { render, screen } from "@testing-library/react";
import { act, render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { Provider } from "mobx-react";
import Grid, { VirtualizedGrid, VirtualizedAnnotationPanel, Item } from "../Grid";
Expand Down Expand Up @@ -42,7 +42,7 @@ jest.mock("../../../hooks/useAnnotationQuery", () => ({

jest.mock("react-virtualized-auto-sizer", () => ({
__esModule: true,
default: ({ children }) => children({ width: 800, height: 400 }),
default: jest.fn(({ children }) => children({ width: 800, height: 400 })),
}));

jest.mock("react-window", () => {
Expand Down Expand Up @@ -680,6 +680,39 @@ describe("Grid", () => {
jest.useRealTimers();
});

it("initial hydration hydrates visibleCount+1 so 3rd panel resolves (narrow viewport)", async () => {
jest.useFakeTimers();
mockFetchAnnotationCached.mockReset().mockResolvedValue({ result: [] });
mockIsFF.mockReturnValue(true);
// Narrow width (400) => visibleCount = 2; with visibleCount+1 we hydrate 3 on mount
const AutoSizer = require("react-virtualized-auto-sizer").default;
AutoSizer.mockImplementationOnce(({ children }) => children({ width: 400, height: 400 }));
const stubAnnotation = (id, pk) => ({
...createAnnotation({ id, pk }),
versions: { result: [] },
regions: [],
});
const annotations = [stubAnnotation("a1", 1), stubAnnotation("a2", 2), stubAnnotation("a3", 3)];
const store = createStore({ selected: { selected: annotations[0] } });
const root = {};

render(
<Provider store={store}>
<VirtualizedGrid store={store} annotations={annotations} root={root} />
</Provider>,
);

await act(async () => {});
jest.advanceTimersByTime(100);
await act(async () => {});

const calledPks = mockFetchAnnotationCached.mock.calls.map((c) => c[0]);
expect(calledPks).toContain(3);
expect(calledPks.length).toBeGreaterThanOrEqual(3);

jest.useRealTimers();
});

it("VirtualizedGrid renders with stub annotations (no result)", () => {
mockIsFF.mockImplementation((flag) => flag === FF_DEV_3391 || flag === FF_FIT_720_LAZY_LOAD_ANNOTATIONS);
const stubAnnotation = (id, pk) => ({
Expand Down
2 changes: 1 addition & 1 deletion web/libs/editor/src/mixins/AnnotationMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { FF_DEV_3391, FF_SIMPLE_INIT, isFF } from "../utils/feature-flags";
export const AnnotationMixin = types.model("AnnotationMixin", {}).views((self) => ({
get annotation() {
// annotation should not be accessed before store is initialized
if (isFF(FF_SIMPLE_INIT) && !window.STORE_INIT_OK) {
if (isFF(FF_SIMPLE_INIT) && !window.STORE_INIT_OK && !isFF(FF_DEV_3391)) {
console.error("LSF: annotation accessed before store is initialized", self);
}

Expand Down
100 changes: 100 additions & 0 deletions web/libs/editor/src/mixins/__tests__/AnnotationMixin.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* Unit tests for AnnotationMixin (FIT-1494 / Compare All init).
*
* When FF_SIMPLE_INIT and FF_DEV_3391 are on, the annotation getter must NOT log
* "annotation accessed before store is initialized" when called during init (STORE_INIT_OK false),
* because the getter resolves the annotation from the tree via getParentOfTypeString.
* These tests fail before the AnnotationMixin fix and pass after it.
*/
import { types } from "mobx-state-tree";
import { AnnotationMixin } from "../AnnotationMixin";
import * as featureFlags from "../../utils/feature-flags";

const { FF_DEV_3391, FF_SIMPLE_INIT } = featureFlags;

jest.mock("../../utils/feature-flags", () => {
const actual = jest.requireActual("../../utils/feature-flags");
return { ...actual, isFF: jest.fn((id) => actual.isFF(id)) };
});

const ChildWithMixin = types.compose(
types.model("ChildNode", { name: types.optional(types.string, "n1") }),
AnnotationMixin,
);

const Annotation = types.model("Annotation", {
id: types.identifier,
child: types.optional(ChildWithMixin, { name: "n1" }),
});

const AnnotationStore = types.model("AnnotationStore", {
annotations: types.array(Annotation),
});

const Store = types.model("Store", {
annotationStore: types.optional(AnnotationStore, { annotations: [] }),
});

describe("AnnotationMixin – Compare All init (FIT-1494)", () => {
let actualFeatureFlags;

beforeEach(() => {
jest.clearAllMocks();
actualFeatureFlags = jest.requireActual("../../utils/feature-flags");
featureFlags.isFF.mockImplementation((id) => actualFeatureFlags.isFF(id));
window.STORE_INIT_OK = undefined;
});

afterEach(() => {
window.STORE_INIT_OK = undefined;
});

it("does not log when STORE_INIT_OK is false and FF_SIMPLE_INIT and FF_DEV_3391 are on", () => {
featureFlags.isFF.mockImplementation(
(id) => id === FF_SIMPLE_INIT || id === FF_DEV_3391 || actualFeatureFlags.isFF(id),
);
window.STORE_INIT_OK = false;

const store = Store.create({
annotationStore: {
annotations: [{ id: "a1", child: { name: "n1" } }],
},
});
const child = store.annotationStore.annotations[0].child;
const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {});

const result = child.annotation;

const badCalls = consoleErrorSpy.mock.calls.filter(
(call) => call[0] && String(call[0]).includes("annotation accessed before store is initialized"),
);
expect(badCalls).toHaveLength(0);
expect(result).not.toBeNull();

consoleErrorSpy.mockRestore();
});

it("still logs when STORE_INIT_OK is false and FF_DEV_3391 is off", () => {
featureFlags.isFF.mockImplementation((id) =>
id === FF_SIMPLE_INIT ? true : id === FF_DEV_3391 ? false : actualFeatureFlags.isFF(id),
);
window.STORE_INIT_OK = false;

const store = Store.create({
annotationStore: {
annotations: [{ id: "a1", child: { name: "n1" } }],
},
});
const child = store.annotationStore.annotations[0].child;
const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {});

child.annotation;

const badCalls = consoleErrorSpy.mock.calls.filter(
(call) => call[0] && String(call[0]).includes("annotation accessed before store is initialized"),
);
expect(badCalls.length).toBeGreaterThan(0);

consoleErrorSpy.mockRestore();
});
});
Loading