fix: fix review issues

gemini-testing · Aug 21, 2024 · a424b5e · a424b5e
1 parent ea44585
commit a424b5e
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 53 deletions.
diff --git a/.gitignore b/.gitignore
@@ -30,3 +30,4 @@ test/func/**/report-backup
 test/func/**/reports
 test/func/packages/*/plugin.js
 test/func/fixtures/playwright/test-results
+@
diff --git a/lib/static/new-ui/app/report.tsx b/lib/static/new-ui/app/report.tsx
@@ -7,7 +7,7 @@ import {initStaticReport, finStaticReport} from '../../modules/actions';
 const rootEl = document.getElementById('app') as HTMLDivElement;
 const root = createRoot(rootEl);
 
-function Gui(): ReactNode {
+function Report(): ReactNode {
     useEffect(() => {
         store.dispatch(initStaticReport());
 
@@ -19,4 +19,4 @@ function Gui(): ReactNode {
     return <App/>;
 }
 
-root.render(<Gui />);
+root.render(<Report />);
diff --git a/lib/static/new-ui/components/ImageWithMagnifier/index.module.css b/lib/static/new-ui/components/ImageWithMagnifier/index.module.css
@@ -0,0 +1,10 @@
+.magnifier {
+    background-color: white;
+    background-repeat: no-repeat;
+    border: 1px solid lightgrey;
+    border-radius: 5px;
+    opacity: 1;
+    pointer-events: none;
+    position: fixed;
+    z-index: 1000
+}
diff --git a/.../new-ui/components/ImageWithMagnifier.tsx → ...i/components/ImageWithMagnifier/index.tsx b/.../new-ui/components/ImageWithMagnifier.tsx → ...i/components/ImageWithMagnifier/index.tsx
@@ -1,5 +1,9 @@
-import React, {ReactNode, useEffect, useState} from 'react';
+import classnames from 'classnames';
+import React, {ReactNode, useEffect, useRef, useState} from 'react';
 import {createPortal} from 'react-dom';
+import styles from './index.module.css';
+
+const DEFAULT_ZOOM_LEVEL = 3;
 
 interface ImageWithMagnifierProps {
     src: string;
@@ -9,6 +13,8 @@ interface ImageWithMagnifierProps {
     magnifierHeight?: number;
     magnifierWidth?: number;
     zoomLevel?: number;
+    // Used to detect parent container scrolling and update the magnifier state
+    scrollContainerRef?: React.RefObject<HTMLElement>;
 }
 
 export function ImageWithMagnifier({
@@ -18,74 +24,98 @@ export function ImageWithMagnifier({
     style,
     magnifierHeight = 150,
     magnifierWidth = 150,
-    zoomLevel = 3
+    zoomLevel = DEFAULT_ZOOM_LEVEL,
+    scrollContainerRef
 }: ImageWithMagnifierProps): ReactNode {
     const [showMagnifier, setShowMagnifier] = useState(false);
     const [[imgWidth, imgHeight], setSize] = useState([0, 0]);
     const [[x, y], setXY] = useState([0, 0]);
-    const [[mouseX, mouseY], setMouseXY] = useState([0, 0]);
+    const mousePositionRef = useRef([0, 0]);
     const [magnifierStyle, setMagnifierStyle] = useState({});
+    const imgRef = useRef<HTMLImageElement>(null);
 
     const mouseEnter = (e: React.MouseEvent<HTMLImageElement>): void => {
         const el = e.currentTarget;
 
         const {width, height} = el.getBoundingClientRect();
         setSize([width, height]);
         setShowMagnifier(true);
-        setMouseXY([e.clientX, e.clientY]);
+        mousePositionRef.current = [e.clientX, e.clientY];
     };
 
     const mouseLeave = (e: React.MouseEvent<HTMLImageElement>): void => {
         e.preventDefault();
         setShowMagnifier(false);
-        setMouseXY([e.clientX, e.clientY]);
+        mousePositionRef.current = [e.clientX, e.clientY];
     };
 
     const mouseMove = (e: React.MouseEvent<HTMLImageElement>): void => {
         const el = e.currentTarget;
         const {top, left} = el.getBoundingClientRect();
 
-        const x = e.pageX - left - window.scrollX;
-        const y = e.pageY - top - window.scrollY;
+        const x = e.clientX - left - window.scrollX;
+        const y = e.clientY - top - window.scrollY;
 
         setXY([x, y]);
-        setMouseXY([e.clientX, e.clientY]);
+        mousePositionRef.current = [e.clientX, e.clientY];
     };
 
     useEffect(() => {
+        if (showMagnifier && scrollContainerRef?.current && imgRef?.current) {
+            const handleScroll = (): void => {
+                if (!imgRef.current) {
+                    return;
+                }
+                const [mouseX, mouseY] = mousePositionRef.current;
+
+                const el = imgRef.current;
+                const {top, left} = el.getBoundingClientRect();
+
+                const x = mouseX - left - window.scrollX;
+                const y = mouseY - top - window.scrollY;
+
+                setXY([x, y]);
+            };
+
+            scrollContainerRef.current.addEventListener('scroll', handleScroll);
+
+            return () => {
+                scrollContainerRef.current?.removeEventListener('scroll', handleScroll);
+            };
+        }
+
+        return undefined;
+    }, [showMagnifier, scrollContainerRef]);
+
+    useEffect(() => {
+        const [mouseX, mouseY] = mousePositionRef.current;
+
         setMagnifierStyle({
             display: showMagnifier ? '' : 'none',
-            position: 'fixed',
-            pointerEvents: 'none',
             height: `${magnifierHeight}px`,
             width: `${magnifierWidth}px`,
-            opacity: '1',
-            border: '1px solid lightgrey',
-            backgroundColor: 'white',
-            borderRadius: '5px',
             backgroundImage: `url('${src}')`,
-            backgroundRepeat: 'no-repeat',
             top: `${mouseY - magnifierHeight / 2}px`,
             left: `${mouseX - magnifierWidth / 2}px`,
             backgroundSize: `${imgWidth * zoomLevel}px ${imgHeight * zoomLevel}px`,
             backgroundPositionX: `${-x * zoomLevel + magnifierWidth / 2}px`,
-            backgroundPositionY: `${-y * zoomLevel + magnifierHeight / 2}px`,
-            zIndex: 1000
+            backgroundPositionY: `${-y * zoomLevel + magnifierHeight / 2}px`
         });
     }, [showMagnifier, imgWidth, imgHeight, x, y]);
 
-    return <div className="relative inline-block">
+    return <div>
         <img
             src={src}
-            className={className}
+            className={classnames(className)}
             style={style}
             alt={alt}
             onMouseEnter={(e): void => mouseEnter(e)}
             onMouseLeave={(e): void => mouseLeave(e)}
             onMouseMove={(e): void => mouseMove(e)}
+            ref={imgRef}
         />
-        <div></div>
         {createPortal(<div
+            className={styles.magnifier}
             style={magnifierStyle}
         />, document.body)}
     </div>;

diff --git a/lib/static/new-ui/features/suites/components/SuitesTreeView/index.module.css b/lib/static/new-ui/features/suites/components/SuitesTreeView/index.module.css
@@ -50,15 +50,15 @@
 
         .tree-view__item--current {
             background: #a28aff !important;
-            color: #fff;
+            color: #fff !important;
         }
 
-        .tree-view__item--current:hover {
+        .tree-view__item.tree-view__item--current:hover {
             background: #af9aff !important;
         }
 
         .tree-view__item--current svg {
-            color: #fff;
+            color: #fff !important;
         }
 
         .tree-view__item--browser {

diff --git a/lib/static/new-ui/features/suites/components/SuitesTreeView/index.tsx b/lib/static/new-ui/features/suites/components/SuitesTreeView/index.tsx
@@ -1,7 +1,9 @@
 import {Box} from '@gravity-ui/uikit';
 import {
-    unstable_getItemRenderState as getItemRenderState, unstable_ListContainerView as ListContainerView,
-    unstable_ListItemView as ListItemView, unstable_useList as useList
+    unstable_getItemRenderState as getItemRenderState,
+    unstable_ListContainerView as ListContainerView,
+    unstable_ListItemView as ListItemView,
+    unstable_useList as useList
 } from '@gravity-ui/uikit/unstable';
 import {useVirtualizer} from '@tanstack/react-virtual';
 import classNames from 'classnames';
@@ -26,6 +28,7 @@ import {
     getTreeViewItems
 } from '@/static/new-ui/features/suites/components/SuitesTreeView/selectors';
 import styles from './index.module.css';
+import {TestStatus} from '@/constants';
 
 interface SuitesTreeViewProps {
     treeViewItems: TreeViewItem<TreeViewSuiteData | TreeViewBrowserData>[];
@@ -63,12 +66,6 @@ function SuitesTreeViewInternal(props: SuitesTreeViewProps): ReactNode {
     const virtualizedItems = virtualizer.getVirtualItems();
 
     // Effects
-    useEffect(() => {
-        if (!suiteId && props.currentSuiteId && suiteId !== props.currentSuiteId) {
-            navigate(encodeURIComponent(props.currentSuiteId));
-        }
-    }, []);
-
     useEffect(() => {
         if (!props.isInitialized) {
             return;
@@ -86,7 +83,7 @@ function SuitesTreeViewInternal(props: SuitesTreeViewProps): ReactNode {
     const onItemClick = useCallback(({id}: {id: string}): void => {
         const item = list.structure.itemsById[id];
 
-        if (item.type === TreeViewItemType.Suite && list.state.expandedById && id in list.state.expandedById && list.state.setExpanded) {
+        if (item.type === TreeViewItemType.Suite) {
             props.actions.toggleSuiteSection({suiteId: item.fullTitle, shouldBeOpened: !props.treeViewExpandedById[item.fullTitle]});
         } else if (item.type === TreeViewItemType.Browser) {
             props.actions.suitesPageSetCurrentSuite(id);
@@ -107,15 +104,14 @@ function SuitesTreeViewInternal(props: SuitesTreeViewProps): ReactNode {
                 >
                     {virtualizedItems.map((virtualRow) => {
                         const item = list.structure.itemsById[virtualRow.key];
-                        const classes = [styles['tree-view__item']];
-                        if (item.fullTitle === props.currentSuiteId) {
-                            classes.push(styles['tree-view__item--current']);
-                        } else if ((item.status === 'fail' || item.status === 'error') && item.type === TreeViewItemType.Browser) {
-                            classes.push(styles['tree-view__item--error']);
-                        }
-                        if (item.type === TreeViewItemType.Browser) {
-                            classes.push(styles['tree-view__item--browser']);
-                        }
+                        const classes = [
+                            styles['tree-view__item'],
+                            {
+                                [styles['tree-view__item--current']]: item.fullTitle === props.currentSuiteId,
+                                [styles['tree-view__item--browser']]: item.type === TreeViewItemType.Browser,
+                                [styles['tree-view__item--error']]: item.type === TreeViewItemType.Browser && (item.status === TestStatus.FAIL || item.status === TestStatus.ERROR)
+                            }
+                        ];
 
                         return <Box
                             key={virtualRow.key}
@@ -124,7 +120,7 @@ function SuitesTreeViewInternal(props: SuitesTreeViewProps): ReactNode {
                             spacing={{pt: 1}}
                         >
                             <ListItemView
-                                height={0}
+                                height={0} // To prevent GravityUI from setting incorrect min-height
                                 className={classNames(classes)}
                                 {...getItemRenderState({
                                     id: virtualRow.key.toString(),
@@ -134,7 +130,7 @@ function SuitesTreeViewInternal(props: SuitesTreeViewProps): ReactNode {
                                         return {
                                             startSlot: getIconByStatus(x.status),
                                             title: <TreeViewItemTitle item={x}/>,
-                                            subtitle: <TreeViewItemSubtitle item={x}/>
+                                            subtitle: <TreeViewItemSubtitle item={x} scrollContainerRef={parentRef}/>
                                         };
                                     }
                                 }).props}/>

diff --git a/lib/static/new-ui/features/suites/components/SuitesTreeView/selectors.ts b/lib/static/new-ui/features/suites/components/SuitesTreeView/selectors.ts
@@ -24,6 +24,7 @@ import {
 } from '@/static/new-ui/store/selectors';
 import {trimArray} from '@/common-utils';
 import {ImageFile} from '@/types';
+import {getFullTitleByTitleParts} from '@/static/new-ui/utils';
 
 // Converts the existing store structure to the one that can be consumed by GravityUI
 export const getTreeViewItems = createSelector(
@@ -54,7 +55,7 @@ export const getTreeViewItems = createSelector(
             const data: TreeViewBrowserData = {
                 type: TreeViewItemType.Browser,
                 title: browserData.name,
-                fullTitle: (parentSuite.fullTitle + ' ' + browserData.name).trim(),
+                fullTitle: getFullTitleByTitleParts([parentSuite.fullTitle, browserData.name]),
                 status: lastResult.status,
                 errorTitle,
                 errorStack,
@@ -68,7 +69,7 @@ export const getTreeViewItems = createSelector(
             const data: TreeViewSuiteData = {
                 type: TreeViewItemType.Suite,
                 title: suiteData.name,
-                fullTitle: (parentSuite.fullTitle + ' ' + suiteData.name).trim(),
+                fullTitle: getFullTitleByTitleParts([parentSuite.fullTitle, suiteData.name]),
                 status: suiteData.status
             };
 

diff --git a/lib/static/new-ui/features/suites/components/TreeViewItemSubtitle/index.tsx b/lib/static/new-ui/features/suites/components/TreeViewItemSubtitle/index.tsx
@@ -3,9 +3,15 @@ import {TreeViewData, TreeViewItemType} from '@/static/new-ui/features/suites/co
 import styles from './index.module.css';
 import {ImageWithMagnifier} from '@/static/new-ui/components/ImageWithMagnifier';
 
-export function TreeViewItemSubtitle(props: {item: TreeViewData}): ReactNode {
+interface TreeViewItemSubtitleProps {
+    item: TreeViewData;
+    // Passed to image with magnifier to detect parent container scrolling and update magnifier position
+    scrollContainerRef: React.RefObject<HTMLElement>;
+}
+
+export function TreeViewItemSubtitle(props: TreeViewItemSubtitleProps): ReactNode {
     if (props.item.type === TreeViewItemType.Browser && props.item.diffImg) {
-        return <ImageWithMagnifier src={props.item.diffImg.path} alt={'diff-image'} style={{maxWidth: '99%', marginTop: '4px', maxHeight: '40vh'}} />;
+        return <ImageWithMagnifier src={props.item.diffImg.path} alt={'diff-image'} style={{maxWidth: '99%', marginTop: '4px', maxHeight: '40vh'}} scrollContainerRef={props.scrollContainerRef} />;
     } else if (props.item.type === TreeViewItemType.Browser && props.item.errorStack) {
         return <div className={styles['tree-view-item-subtitle__error-stack']}>
             {props.item.errorStack}

diff --git a/lib/static/new-ui/utils/index.tsx b/lib/static/new-ui/utils/index.tsx
@@ -13,3 +13,9 @@ export const getIconByStatus = (status: TestStatus): React.JSX.Element => {
 
     return <CircleDashed />;
 };
+
+export const getFullTitleByTitleParts = (titleParts: string[]): string => {
+    const DELIMITER = ' ';
+
+    return titleParts.join(DELIMITER).trim();
+};
diff --git a/test/setup/globals.js b/test/setup/globals.js
@@ -19,4 +19,17 @@ chai.use(require('chai-as-promised'));
 chai.use(require('chai-dom'));
 sinon.assert.expose(chai.assert, {prefix: ''});
 
-require('app-module-path').addPath(path.resolve(__dirname, '..', '..'));
+const projectRoot = path.resolve(__dirname, '..', '..');
+
+// Resolving imports like lib/.../
+require('app-module-path').addPath(projectRoot);
+
+// Resolving webpack alias imports like @/.../
+try {
+    const fs = require('fs');
+    fs.symlinkSync(path.join(projectRoot, 'lib'), path.join(projectRoot, '@'));
+} catch (e) {
+    if (e.code !== 'EEXIST') {
+        throw e;
+    }
+}
diff --git a/tsconfig.spec.json b/tsconfig.spec.json
@@ -6,7 +6,8 @@
         "jsx": "react",
         "noEmit": true,
         "paths": {
-            "lib/*": ["./lib/*"]
+            "lib/*": ["./lib/*"],
+            "@/*": ["./lib/*"]
         }
     },
 }
diff --git a/webpack.common.js b/webpack.common.js
@@ -12,8 +12,6 @@ const {ProgressPlugin} = require('webpack');
 
 const staticPath = path.resolve(__dirname, 'build', 'lib', 'static');
 
-console.log('here is resolved path; ' + path.resolve(__dirname, 'src'));
-
 module.exports = {
     entry: {
         report: ['./index.jsx', './variables.css', './styles.css'],