-
Notifications
You must be signed in to change notification settings - Fork 40
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
base: master
Are you sure you want to change the base?
Changes from all commits
83bc8f6
714f54f
947a530
9ccd0a0
b58c8d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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()); | ||
}; | ||
} |
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'; | ||
|
@@ -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?.()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ну это же не There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Еще на всякий случай хочу спросить, мы получается для прелоада добавляем прям элементы в head и только на unmount их удаляем. Если туда напихать скажем так 5000 скриншотов, ничего плохого не будет, т.е. браузер будет понимать, что это лишь rel=preload и не будет реально их все держать в памяти, верно? |
||
}, []); | ||
}; | ||
|
||
export function VisualChecksPage(): ReactNode { | ||
const dispatch = useDispatch(); | ||
|
@@ -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})); | ||
|
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 GitHub Actions / build (18.x)
Check warning on line 38 in test/unit/lib/static/new-ui/features/visual-checks/components/VisualChecksPage.jsx GitHub Actions / build (20.x)
|
||
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] | ||
); | ||
} | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Может еще докинуть тест с кликом на кнопку, что грузится еще 1 картинка? Но не критично |
||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Так давай вместо коммента просто удалим эту функцию и будем юзать везде новую, если она работает корректно и лучше?