Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

feat(IM-5878): consider scroll position when taking snapshot #520

Closed
wants to merge 3 commits into from
Closed
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
2 changes: 1 addition & 1 deletion src/ext/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default function ext(env: Galaxy): Record<string, unknown> {
// TODO Most of the code is in place to enable snapshot, however WYSIWYG is not always true. Depending on the scroll position, some cell that where visible when the screenshot was taken, is not fully visiable in the screenshot.
// This should be fixed, possibly be figuring out how much of a cell is in view and then in the snapshot scroll that cell into view by the same amount.
cssScaling: false,
snapshot: false,
snapshot: env.flags.isEnabled("CLIENT_IM5878_PIVOT_SNAPSHOT"),
export: false,
sharing: false,
exportData: true,
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/use-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const useRender = (env: Galaxy) => {
const themeName = theme.name();
const { translator, language } = useTranslations();
const { pageInfo, updatePageInfo } = usePagination(layoutService);
const viewService = useViewService(pageInfo);
const viewService = useViewService(layout, pageInfo);
const rect = useSnapshot({ rect: useRect(), layoutService, viewService, model });
const [qPivotDataPages, isLoading] = useLoadDataPages({ model, layoutService, viewService, pageInfo, rect });
// It needs to be theme.name() because the reference to the theme object does not change when a theme is changed
Expand Down
11 changes: 10 additions & 1 deletion src/hooks/use-snapshot.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable no-param-reassign */
import { onTakeSnapshot, type stardust } from "@nebula.js/stardust";
import { onTakeSnapshot, useImperativeHandle, type stardust } from "@nebula.js/stardust";
import { Q_PATH } from "../constants";
import { getViewState } from "../pivot-table/components/cells/utils/get-view-state";
import type { Model, SnapshotLayout } from "../types/QIX";
import type { LayoutService, ViewService } from "../types/types";

Expand All @@ -17,6 +18,7 @@ const useSnapshot = ({ layoutService, viewService, rect, model }: UseSnapshotPro
return snapshotLayout;
}

const { rowPartialHeight, visibleTopIndex, visibleRows, page, rowsPerPage } = getViewState(viewService);
if ((model as EngineAPI.IGenericObject)?.getHyperCubePivotData) {
snapshotLayout.snapshotData.content = {
qPivotDataPages: await (model as EngineAPI.IGenericObject).getHyperCubePivotData(Q_PATH, [
Expand All @@ -27,6 +29,11 @@ const useSnapshot = ({ layoutService, viewService, rect, model }: UseSnapshotPro
qHeight: viewService.gridHeight,
},
]),
rowPartialHeight,
visibleTopIndex,
visibleRows,
page,
rowsPerPage,
};

snapshotLayout.snapshotData.object.size.w = rect.width;
Expand All @@ -35,6 +42,8 @@ const useSnapshot = ({ layoutService, viewService, rect, model }: UseSnapshotPro
return snapshotLayout;
});

useImperativeHandle(() => ({ getViewState: () => getViewState(viewService) }), [viewService]);

if (layoutService.layout.snapshotData?.content) {
return {
left: rect.left,
Expand Down
9 changes: 7 additions & 2 deletions src/hooks/use-view-service.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { useMemo } from "@nebula.js/stardust";
import createViewService from "../services/view-service";
import type { PivotLayout } from "../types/QIX";
import type { PageInfo, ViewService } from "../types/types";

// eslint-disable-next-line react-hooks/exhaustive-deps
const useViewService = (pageInfo: PageInfo): ViewService => useMemo(() => createViewService(), [pageInfo.page]);
const useViewService = (layout: PivotLayout, pageInfo: PageInfo): ViewService =>
useMemo(
() => createViewService(layout.snapshotData),
// eslint-disable-next-line react-hooks/exhaustive-deps
[layout.snapshotData, pageInfo.page],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure pageInfo.page should be in the dependencies. It was there before, so I kept it as it is.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pageInfo.page in deps means that the view service is "reset" or re-created when page changes. Which is what we want since all states in viewService are based on the current page.

);

export default useViewService;
14 changes: 13 additions & 1 deletion src/pivot-table/components/PivotTable.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { stardust } from "@nebula.js/stardust";
import React, { useCallback } from "react";
import React, { useCallback, useEffect } from "react";
import type { Model } from "../../types/QIX";
import {
ScrollableContainerOrigin,
Expand Down Expand Up @@ -50,6 +50,10 @@ export const StickyPivotTable = ({
const { visibleLeftDimensionInfo, visibleTopDimensionInfo } = useVisibleDimensions(layoutService, qPivotDataPages);
const { changeSortOrder, changeActivelySortedHeader } = useSorting(model, layoutService.layout.qHyperCube);

const isPrintingMode = !!layoutService.layout.snapshotData;
// const scrollLeft = isPrintingMode ? viewService.scrollLeft ?? 0 : 0;
const scrollTopPartially = isPrintingMode ? viewService.rowPartialHeight ?? 0 : 0;

const { headersData, measureData, topDimensionData, leftDimensionData, attrExprInfoIndexes, nextPageHandler } =
useData(qPivotDataPages, layoutService, pageInfo, visibleLeftDimensionInfo, visibleTopDimensionInfo);

Expand Down Expand Up @@ -123,6 +127,14 @@ export const StickyPivotTable = ({
const headerCellRowHightCallback = useCallback(() => headerCellHeight, [headerCellHeight]);
const contentCellRowHightCallback = useCallback(() => contentCellHeight, [contentCellHeight]);

useEffect(() => {
const element = verticalScrollableContainerRef.current;
if (element) {
// element.scrollLeft = scrollLeft;
element.scrollTop = scrollTopPartially;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely need to add a similar property for scrolLeft and other conditions if there are more, but maybe we can focus on vertical scroll for this PR

}
}, [scrollTopPartially, verticalScrollableContainerRef]);

return (
<ScrollableContainer
ref={verticalScrollableContainerRef}
Expand Down
9 changes: 9 additions & 0 deletions src/pivot-table/components/cells/utils/get-view-state.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { ViewService } from "../../../../types/types";

export const getViewState = (viewService: ViewService) => ({

Check failure on line 3 in src/pivot-table/components/cells/utils/get-view-state.ts

View workflow job for this annotation

GitHub Actions / build / build_release

'viewService' is defined but never used
rowPartialHeight: 125,
visibleTopIndex: 5,
visibleRows: 16,
page: 0,
rowsPerPage: 0,
});
9 changes: 7 additions & 2 deletions src/services/view-service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import type { SnapshotData } from "../types/QIX";
import type { ViewService } from "../types/types";

const createViewService = (): ViewService => ({
const createViewService = (snapshotData: SnapshotData | undefined): ViewService => ({
gridColumnStartIndex: 0,
gridRowStartIndex: 0,
gridWidth: 0,
gridHeight: 0,
rowPartialHeight: snapshotData?.content?.rowPartialHeight ?? 0,
visibleTopIndex: snapshotData?.content?.visibleTopIndex,
visibleRows: snapshotData?.content?.visibleRows,
rowsPerPage: snapshotData?.content?.rowsPerPage,
page: snapshotData?.content?.page,
});

export default createViewService;
5 changes: 5 additions & 0 deletions src/types/QIX.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ type Size = {
export interface SnapshotData {
content?: {
qPivotDataPages?: EngineAPI.INxPivotPage[];
rowPartialHeight?: number;
visibleTopIndex?: number;
visibleRows?: number;
page?: number;
rowsPerPage?: number;
};
object: {
size: Size;
Expand Down
5 changes: 5 additions & 0 deletions src/types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ export interface ViewService {
gridRowStartIndex: number;
gridWidth: number;
gridHeight: number;
rowPartialHeight?: number;
visibleTopIndex?: number;
visibleRows?: number;
page?: number;
rowsPerPage?: number;
}

export interface LayoutService {
Expand Down
Loading