Brijesh619 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3796115416
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -210,30 +322,32 @@ const SideBarBody = (props: {
<Drawer
sx={{
- width: position,
+ width: open ? defaultDrawerWidth : "60px",
flexShrink: 0,
minHeight: "calc(100vh - 64px)",
- minWidth: "30px",
- ...(open == false && {
- transform: `translateX(calc(-${position} + 30px)) !important`,
+ minWidth: "60px",
+ transition: "width 0.2s",
+ ...(!open && {
+ transform: "none !important",
+ visibility: "visible !important",
}),
- ...(open == false && { visibility: "visible !important" }),
-
"& .MuiDrawer-paper": {
background: "#034858",
boxSizing: "border-box",
overflow: "hidden",
position: "fixed",
top: "0",
- transition: "none !important",
- ...(open == false && {
- transform: `translateX(30px) !important`,
+ left: "0",
+ width: open ? defaultDrawerWidth : "60px",
+ transition: "width 0.2s",
+ ...(!open && {
+ transform: "none !important",
+ visibility: "visible !important",
}),
- ...(open == false && { visibility: "visible !important" }),
},
}}
PaperProps={{
- style: { width: position, minWidth: "30px" },
+ style: { width: open ? defaultDrawerWidth : "60px", minWidth: "60px"
},
}}
variant="persistent"
anchor="left"
Review Comment:
Resolved. The search expand button already has aria-label="Expand sidebar
search". For good measure, I also added an aria-label to the main drawer
expand/collapse toggle at the bottom of the sidebar!
##########
dashboard/src/views/Layout/About.tsx:
##########
@@ -24,31 +24,9 @@ import {
Stack,
Typography
} from "@mui/material";
-import { serverError } from "@utils/Utils";
-import { useEffect, useRef, useState } from "react";
const About = () => {
- const [versionData, setVersionData] = useState<any>({});
- const [loader, setLoader] = useState(false);
- const toastId = useRef(null);
-
- useEffect(() => {
- fetchVersionDetails();
- }, []);
-
- const fetchVersionDetails = async () => {
- setLoader(true);
- try {
- const versionResp = await getVersion();
- const { data = {} } = versionResp || {};
- setVersionData(data);
- setLoader(false);
- } catch (error) {
- setLoader(false);
- console.error(`Error occur while fetching version details`, error);
- serverError(error, toastId);
- }
- };
+ const { data: versionData, loading: loader } = useAppSelector((state: any)
=> state.session.versionData);
Review Comment:
Resolved. Added a fallback error message "Unknown (failed to fetch version)"
in the UI when state.session.versionData.error is present.
##########
dashboard/src/components/__tests__/EntityDisplayImage.test.tsx:
##########
@@ -15,271 +15,119 @@
* limitations under the License.
*/
-
-/**
- * Unit tests for EntityDisplayImage component
- *
- * Coverage Target: 100%
- * - Statements: 100%
- * - Branches: 100%
- * - Functions: 100%
- * - Lines: 100%
- */
-
import React from 'react'
-import { render, waitFor, act } from '@testing-library/react'
+import { render, fireEvent } from '@testing-library/react'
import DisplayImage from '../EntityDisplayImage'
-
-// Import Utils to spy on it
import * as Utils from '../../utils/Utils'
const mockGetEntityIconPath = jest.fn()
-// Mock the Utils module
jest.mock('../../utils/Utils', () => ({
- getEntityIconPath: jest.fn()
+ getEntityIconPath: jest.fn()
}))
-const mockFetch = (contentType: string | null, shouldReject?: boolean) => {
- if (shouldReject) {
- (global as any).fetch = jest.fn().mockRejectedValue(new
Error('fetch failed'))
- return
- }
- (global as any).fetch = jest.fn().mockResolvedValue({
- ok: true,
- headers: {
- get: jest.fn((header: string) => {
- return header === 'Content-Type' ? (contentType
|| '') : null
- })
- }
- })
-}
-
describe('EntityDisplayImage', () => {
- const entity = { guid: 'entity-1' }
-
- beforeEach(() => {
- jest.clearAllMocks()
-
- // Set up the mock implementation for getEntityIconPath
- ;(Utils.getEntityIconPath as jest.Mock).mockImplementation(({
entityData, errorUrl }: { entityData: any, errorUrl?: string }) => {
- const result = errorUrl ? `${errorUrl}-fallback` :
`/icons/${entityData.guid}.png`
- mockGetEntityIconPath({ entityData, errorUrl })
- return result
- })
- })
-
- it('renders cached image when content-type is image', async () => {
- mockFetch('image/png')
-
- const { container } = render(
- <DisplayImage entity={entity} width={20} height={20} />
- )
-
- // Wait for Skeleton to disappear and image to appear
- await waitFor(() => {
- const skeleton =
container.querySelector('.MuiSkeleton-root')
- expect(skeleton).not.toBeInTheDocument()
- }, { timeout: 10000, interval: 100 })
-
- await waitFor(() => {
- const img = container.querySelector('img')
- expect(img).toBeInTheDocument()
-
expect(img?.getAttribute('src')).toBe('/icons/entity-1.png')
- expect(img?.getAttribute('alt')).toBe('Entity Icon')
- expect(img?.getAttribute('id')).toBe('entity-1')
- expect(img?.getAttribute('data-cy')).toBe('entity-1')
- }, { timeout: 10000 })
- }, 20000)
-
- it('renders fallback image when content-type is not image', async () =>
{
- mockFetch('text/plain')
-
- const { container } = render(
- <DisplayImage entity={entity} width={20} height={20} />
- )
-
- await waitFor(() => {
- const img = container.querySelector('img')
- expect(img).toBeInTheDocument()
-
expect(img?.getAttribute('src')).toBe('/icons/entity-1.png-fallback')
- }, { timeout: 10000 })
- }, 20000)
-
- it('renders fallback image when content-type is null', async () => {
- mockFetch(null)
-
- const { container} = render(
- <DisplayImage entity={entity} width={20} height={20} />
- )
-
- await waitFor(() => {
- const img = container.querySelector('img')
- expect(img).toBeInTheDocument()
-
expect(img?.getAttribute('src')).toBe('/icons/entity-1.png-fallback')
- }, { timeout: 10000 })
- }, 20000)
-
- it('renders fallback image when fetch throws', async () => {
- mockFetch('image/png', true)
-
- const { container } = render(
- <DisplayImage entity={entity} width={20} height={20} />
- )
-
- await waitFor(() => {
- const img = container.querySelector('img')
- expect(img).toBeInTheDocument()
-
expect(img?.getAttribute('src')).toBe('/icons/entity-1.png-fallback')
- }, { timeout: 10000 })
- }, 20000)
-
- it('renders Avatar when avatarDisplay is provided and image is cached',
async () => {
- mockFetch('image/png')
-
- const { container } = render(
- <DisplayImage
- entity={entity}
- width={30}
- height={30}
- avatarDisplay={true}
- />
- )
-
- await waitFor(() => {
- const avatar =
container.querySelector('img[alt="entityImg"]')
- expect(avatar).toBeTruthy()
-
expect(avatar?.getAttribute('src')).toBe('/icons/entity-1.png')
- }, { timeout: 10000 })
- }, 20000)
-
- it('renders Avatar when avatarDisplay is provided and image is not
cached', async () => {
- mockFetch('text/plain')
-
- const { container } = render(
- <DisplayImage
- entity={entity}
- width={30}
- height={30}
- avatarDisplay={true}
- />
- )
-
- await waitFor(() => {
- const avatar =
container.querySelector('img[alt="entityImg"]')
- expect(avatar).toBeTruthy()
-
expect(avatar?.getAttribute('src')).toBe('/icons/entity-1.png-fallback')
- }, { timeout: 10000 })
- }, 20000)
-
- it('renders Skeleton when imageUrl is undefined', () => {
- mockGetEntityIconPath.mockReturnValue(undefined)
-
- const { container } = render(
- <DisplayImage entity={entity} width={20} height={20} />
- )
-
- const skeleton = container.querySelector('div')
- expect(skeleton).toBeTruthy()
- })
-
- it('handles isProcess prop', async () => {
- mockFetch('image/png')
-
- const entityWithProcess = { guid: 'entity-2', isProcess: true }
- render(
- <DisplayImage entity={entityWithProcess} width={20}
height={20} isProcess={true} />
- )
-
- await waitFor(() => {
- expect(mockGetEntityIconPath).toHaveBeenCalledWith(
- expect.objectContaining({
- entityData: expect.objectContaining({
isProcess: true })
- })
- )
- })
- }, 20000)
-
- it('handles entity without isProcess prop but with isProcess passed',
async () => {
- mockFetch('image/png')
-
- render(
- <DisplayImage entity={entity} width={20} height={20}
isProcess={false} />
- )
-
- await waitFor(() => {
- expect(mockGetEntityIconPath).toHaveBeenCalledWith(
- expect.objectContaining({
- entityData: expect.objectContaining({
isProcess: false })
- })
- )
- })
- }, 20000)
-
- it('sets checkEntityImage cache when image is valid', async () => {
- mockFetch('image/jpeg')
-
- const { container } = render(
- <DisplayImage entity={entity} width={20} height={20} />
- )
-
- await waitFor(() => {
- const img = container.querySelector('img')
- expect(img).toBeInTheDocument()
-
expect(img?.getAttribute('src')).toBe('/icons/entity-1.png')
- }, { timeout: 10000 })
- }, 20000)
-
- it('handles different image content types', async () => {
- const contentTypes = ['image/gif', 'image/webp',
'image/svg+xml']
-
- for (const contentType of contentTypes) {
- mockFetch(contentType)
- const { container, unmount } = render(
- <DisplayImage entity={{ guid:
`entity-${contentType}` }} width={20} height={20} />
- )
-
- await waitFor(() => {
- const img = container.querySelector('img')
- expect(img).toBeTruthy()
- }, { timeout: 10000 })
- unmount()
- }
- }, 30000)
-
- it('handles errorUrl in getEntityIconPath when fetch fails', async ()
=> {
- mockFetch('image/png', true)
- ;(Utils.getEntityIconPath as jest.Mock).mockImplementation(({
entityData, errorUrl }: { entityData: any, errorUrl?: string }) => {
- if (errorUrl) return `${errorUrl}-error`
- return `/icons/${entityData.guid}.png`
- })
-
- const { container } = render(
- <DisplayImage entity={entity} width={20} height={20} />
- )
-
- await waitFor(() => {
- const img = container.querySelector('img')
- expect(img).toBeInTheDocument()
- expect(img?.getAttribute('src')).toContain('-error')
- }, { timeout: 10000 })
- }, 20000)
-
- it('handles errorUrl in getEntityIconPath when content-type is not
image', async () => {
- mockFetch('application/json')
- ;(Utils.getEntityIconPath as jest.Mock).mockImplementation(({
entityData, errorUrl }: { entityData: any, errorUrl?: string }) => {
- if (errorUrl) return `${errorUrl}-error`
- return `/icons/${entityData.guid}.png`
- })
-
- const { container } = render(
- <DisplayImage entity={entity} width={20} height={20} />
- )
-
- await waitFor(() => {
- const img = container.querySelector('img')
- expect(img).toBeInTheDocument()
- expect(img?.getAttribute('src')).toContain('-error')
- }, { timeout: 10000 })
- }, 20000)
+ const entity = { guid: 'entity-1' }
+
+ beforeEach(() => {
+ jest.clearAllMocks()
+
+ ;(Utils.getEntityIconPath as jest.Mock).mockImplementation(({ entityData,
errorUrl }: { entityData: any, errorUrl?: string }) => {
+ const result = errorUrl ? `${errorUrl}-fallback` :
`/icons/${entityData.guid}.png`
+ mockGetEntityIconPath({ entityData, errorUrl })
+ return result
+ })
+ })
+
+ it('renders primary image instantly', () => {
+ const { container } = render(
+ <DisplayImage entity={entity} width={20} height={20} />
+ )
+
+ const img = container.querySelector('img')
+ expect(img).toBeInTheDocument()
+ expect(img?.getAttribute('src')).toBe('/icons/entity-1.png')
+ expect(img?.getAttribute('alt')).toBe('Entity Icon')
+ expect(img?.getAttribute('id')).toBe('entity-1')
+ expect(img?.getAttribute('data-cy')).toBe('entity-1')
+ })
+
+ it('switches to fallback image when native onError is triggered', () => {
Review Comment:
Resolved. Added two new tests! One specifically verifies that img.onerror is
explicitly set to null to prevent infinite loops, and another test covers the
edge case where primaryUrl and fallbackUrl are empty strings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
