Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(new-ui): add image prefetch in new ui #628

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
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
54 changes: 54 additions & 0 deletions lib/static/modules/utils/imageEntity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import {ImageEntity, ImageEntityCommitted, ImageEntityError, ImageEntityFail, ImageEntityStaged, ImageEntitySuccess, ImageEntityUpdated} from '../../new-ui/types/store';

function preloadImage(url: string): HTMLElement {
const link = document.createElement('link');

link.rel = 'preload';
link.as = 'image';
link.href = url;
link.onload;

document.head.appendChild(link);

return link;
}

function hasExpectedImage(image: ImageEntity): image is ImageEntityFail | ImageEntitySuccess | ImageEntityUpdated {
return Object.hasOwn(image, 'expectedImg');
}

function hasActualImage(image: ImageEntity): image is ImageEntityFail | ImageEntityCommitted | ImageEntityError | ImageEntityStaged {
return Object.hasOwn(image, 'actualImg');
}

function hasDiffImage(image: ImageEntity): image is ImageEntityFail {
return Object.hasOwn(image, 'diffImg');
}

function hasRefImage(image: ImageEntity): image is ImageEntityFail {
return Object.hasOwn(image, 'refImg');
}

export function preloadImageEntity(image: ImageEntity): () => void {
const elements: HTMLElement[] = [];

if (hasExpectedImage(image)) {
elements.push(preloadImage(image.expectedImg.path));
}

if (hasActualImage(image)) {
elements.push(preloadImage(image.actualImg.path));
}

if (hasDiffImage(image)) {
elements.push(preloadImage(image.diffImg.path));
}

if (hasRefImage(image)) {
elements.push(preloadImage(image.refImg.path));
}

return (): void => {
elements.forEach(element => element.remove());
};
}
4 changes: 4 additions & 0 deletions lib/static/modules/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ export function parseKeyToGroupTestsBy(key) {
return [groupSection, groupKey];
}

/** @deprecated - this is just not working.
Copy link
Member

Choose a reason for hiding this comment

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

Так давай вместо коммента просто удалим эту функцию и будем юзать везде новую, если она работает корректно и лучше?

* @link https://stackoverflow.com/questions/3646036/preloading-images-with-javascript
* @see preloadImage from lib/static/modules/utils/imageEntity.ts instead
*/
export function preloadImage(url) {
new Image().src = url;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/static/new-ui/components/SuiteTitle/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ export function SuiteTitle(props: SuiteTitlePropsInternal): ReactNode {
</div>
<div className={styles.paginationContainer}>
<span className={styles.counter}>{props.index === -1 ? '–' : props.index + 1}/{props.totalItems}</span>
<Button view={'flat'} disabled={props.index <= 0} onClick={props.onPrevious}><Icon
<Button qa='suite-prev' view={'flat'} disabled={props.index <= 0} onClick={props.onPrevious}><Icon
data={ChevronUp}/></Button>
<Button view={'flat'} disabled={props.index < 0 || props.index === props.totalItems - 1} onClick={props.onNext}><Icon
<Button qa='suite-next' view={'flat'} disabled={props.index < 0 || props.index === props.totalItems - 1} onClick={props.onNext}><Icon
data={ChevronDown}/></Button>
</div>
</div>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import {ArrowUturnCcwLeft, Check} from '@gravity-ui/icons';
import {Button, Divider, Icon, Select} from '@gravity-ui/uikit';
import classNames from 'classnames';
import React, {ReactNode} from 'react';
import React, {ReactNode, useEffect, useRef} from 'react';
import {useDispatch, useSelector} from 'react-redux';

import {SplitViewLayout} from '@/static/new-ui/components/SplitViewLayout';
import {UiCard} from '@/static/new-ui/components/Card/UiCard';
import {
getCurrentImage,
getCurrentNamedImage,
getImagesByNamedImageIds,
getVisibleNamedImageIds
} from '@/static/new-ui/features/visual-checks/selectors';
import {SuiteTitle} from '@/static/new-ui/components/SuiteTitle';
Expand All @@ -28,6 +29,36 @@ import {
} from '@/static/new-ui/features/visual-checks/components/VisualChecksPage/AssertViewResultSkeleton';
import {thunkAcceptImages, thunkRevertImages} from '@/static/modules/actions/screenshots';
import {useAnalytics} from '@/static/new-ui/hooks/useAnalytics';
import {preloadImageEntity} from '../../../../../modules/utils/imageEntity';

export const PRELOAD_IMAGES_COUNT = 3;

const usePreloadImages = (
currentNamedImageIndex: number,
visibleNamedImageIds: string[]): void => {
const preloaded = useRef<Record<string, () => void | undefined>>({});

const namedImageIdsToPreload: string[] = visibleNamedImageIds.slice(
Math.max(0, currentNamedImageIndex - 1 - PRELOAD_IMAGES_COUNT),
Math.min(visibleNamedImageIds.length, currentNamedImageIndex + 1 + PRELOAD_IMAGES_COUNT)
);

const imagesToPreload = useSelector((state) => getImagesByNamedImageIds(state, namedImageIdsToPreload));

useEffect(() => {
imagesToPreload.forEach(image => {
if (preloaded.current[image.id]) {
return;
}

preloaded.current[image.id] = preloadImageEntity(image);
});
}, [currentNamedImageIndex]);

useEffect(() => () => {
Object.values(preloaded.current).forEach(preload => preload?.());
Copy link
Member

Choose a reason for hiding this comment

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

Ну это же не preload() получается, а cleanup какой-нибудь? нейминг немного сбил с толку

Copy link
Member

Choose a reason for hiding this comment

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

Еще на всякий случай хочу спросить, мы получается для прелоада добавляем прям элементы в head и только на unmount их удаляем. Если туда напихать скажем так 5000 скриншотов, ничего плохого не будет, т.е. браузер будет понимать, что это лишь rel=preload и не будет реально их все держать в памяти, верно?

}, []);
};

export function VisualChecksPage(): ReactNode {
const dispatch = useDispatch();
Expand All @@ -41,6 +72,8 @@ export function VisualChecksPage(): ReactNode {
const onPreviousImageHandler = (): void => void dispatch(visualChecksPageSetCurrentNamedImage(visibleNamedImageIds[currentNamedImageIndex - 1]));
const onNextImageHandler = (): void => void dispatch(visualChecksPageSetCurrentNamedImage(visibleNamedImageIds[currentNamedImageIndex + 1]));

usePreloadImages(currentNamedImageIndex, visibleNamedImageIds);

const diffMode = useSelector(state => state.view.diffMode);
const onChangeHandler = (diffModeId: DiffModeId): void => {
dispatch(setDiffMode({diffModeId}));
Expand Down
19 changes: 19 additions & 0 deletions lib/static/new-ui/features/visual-checks/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,25 @@ export const getCurrentImage = (state: State): ImageEntity | null => {
return getImages(state)[currentImageId];
};

export const getImagesByNamedImageIds = (state: State, names: string[]): ImageEntity[] => {
const results: ImageEntity[] = [];

const images = getImages(state);
const namedImages = getNamedImages(state);

for (const name of names) {
const namedImage = namedImages[name];

if (!namedImage) {
continue;
}

results.push(...namedImage.imageIds.map(id => images[id]));
}

return results;
};

export const getVisibleNamedImageIds = createSelector([getNamedImages], (namedImages): string[] => {
return Object.values(namedImages).map(namedImage => namedImage.id);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import React from 'react';
import {addBrowserToTree, addImageToTree, addResultToTree, addSuiteToTree, mkBrowserEntity, mkEmptyTree, mkImageEntityFail, mkRealStore, mkResultEntity, mkSuiteEntityLeaf, renderWithStore} from '../../../../utils';
import proxyquire from 'proxyquire';

describe('<VisualChecksPage />', () => {
const sandbox = sinon.sandbox.create();

const prepareTestStore = () => {
const tree = mkEmptyTree();

const suite = mkSuiteEntityLeaf(`test-1`);
addSuiteToTree({tree, suite});

const browser = mkBrowserEntity(`bro-1`, {parentId: suite.id});
addBrowserToTree({tree, browser});

const result = mkResultEntity(`res-1`, {parentId: browser.id});
addResultToTree({tree, result});

for (const i of Array.from({length: 10}).map((_, i) => i + 1)) {
const image = mkImageEntityFail(`img-${i}`, {parentId: result.id});
addImageToTree({tree, image});
}

const store = mkRealStore({
initialState: {
app: {
isInitialized: true
},
tree
}
});

return store;
};

let store;
let component;

Check warning on line 38 in test/unit/lib/static/new-ui/features/visual-checks/components/VisualChecksPage.jsx

View workflow job for this annotation

GitHub Actions / build (18.x)

'component' is assigned a value but never used

Check warning on line 38 in test/unit/lib/static/new-ui/features/visual-checks/components/VisualChecksPage.jsx

View workflow job for this annotation

GitHub Actions / build (20.x)

'component' is assigned a value but never used

Check warning on line 38 in test/unit/lib/static/new-ui/features/visual-checks/components/VisualChecksPage.jsx

View workflow job for this annotation

GitHub Actions / build (22.6)

'component' is assigned a value but never used
let preloadImageEntityStub;

beforeEach(() => {
preloadImageEntityStub = sandbox.stub();

store = prepareTestStore();

const VisualChecksPage = proxyquire('lib/static/new-ui/features/visual-checks/components/VisualChecksPage', {
'../../../../../modules/utils/imageEntity': {preloadImageEntity: preloadImageEntityStub}
}).VisualChecksPage;

component = renderWithStore(<VisualChecksPage />, store);
});

afterEach(() => {
sandbox.restore();
});

it('should preload current and 3 adjacent images on mount', async () => {
const state = store.getState();
const orderedImages = Object.values(state.tree.images.byId);

for (let i = 0; i < 3; i++) {
assert.calledWith(
preloadImageEntityStub,
orderedImages[i]
);
}
});
Copy link
Member

Choose a reason for hiding this comment

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

Может еще докинуть тест с кликом на кнопку, что грузится еще 1 картинка? Но не критично

});
Loading