Brijesh619 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3782760553
##########
dashboard/src/components/EntityDisplayImage.tsx:
##########
@@ -15,86 +15,58 @@
* limitations under the License.
*/
-import { useEffect, useState } from "react";
-import { Avatar, Skeleton } from "@mui/material";
+import { Avatar } from "@mui/material";
import { getEntityIconPath } from "../utils/Utils";
+interface DisplayImageProps {
+ entity: Record<string, unknown>;
+ width?: string | number;
+ height?: string | number;
+ avatarDisplay?: boolean;
+ isProcess?: boolean;
+}
+
const DisplayImage = ({
entity,
width,
height,
avatarDisplay,
isProcess
-}: any) => {
- const [imageUrl, setImageUrl] = useState<any>(null);
- const [checkEntityImage, setCheckEntityImage] = useState<any>({
- [entity.guid]: false
- });
-
- useEffect(() => {
- const fetchImagePath = async () => {
- let entityData = { ...entity, ...{ isProcess: isProcess } };
- let imagePath: any = getEntityIconPath({ entityData: entityData });
- try {
- const response = await fetch(imagePath);
- const contentType: any = response.headers.get("Content-Type");
-
- if (contentType.startsWith("image/")) {
- let cache = { [entityData.guid]: imagePath };
- setCheckEntityImage(cache);
- setImageUrl(getEntityIconPath({ entityData: entityData }));
- } else {
- setImageUrl(
- getEntityIconPath({ entityData: entityData, errorUrl: imagePath })
- );
- }
- } catch (error) {
- setImageUrl(
- getEntityIconPath({ entityData: entityData, errorUrl: imagePath })
- );
- }
- };
+}: DisplayImageProps) => {
+ const entityData = { ...entity, isProcess: isProcess };
+
+ const primaryUrl = getEntityIconPath({ entityData }) || "";
+ const fallbackUrl = getEntityIconPath({ entityData, errorUrl: primaryUrl })
|| "";
- fetchImagePath();
- }, []);
+ const handleError = (e: React.SyntheticEvent<HTMLImageElement, Event>) => {
+ const target = e.currentTarget;
+ if (target.src !== fallbackUrl) {
+ target.onerror = null;
+ target.src = fallbackUrl;
+ }
+ };
- return imageUrl != undefined ? (
+ return (
<div className="search-result-table-name-col" data-cy="entityIcon">
- {checkEntityImage[entity.guid] !== false ? (
- avatarDisplay == undefined ? (
- <img
- className="search-result-table-img"
- id={entity.guid}
- data-cy={entity.guid}
- src={checkEntityImage[entity.guid]}
- alt="Entity Icon"
- />
- ) : (
- <Avatar
- alt="entityImg"
- src={checkEntityImage[entity.guid]}
- sx={{ width: width, height: height }}
- variant="square"
- ></Avatar>
- )
- ) : avatarDisplay == undefined ? (
+ {avatarDisplay === undefined ? (
Review Comment:
Yes, intentional. The <img> fallback relies strictly on the
.search-result-table-img CSS class (24x24) to maintain uniform alignment in the
Search Results table; applying width/height props directly could override the
CSS and break the table layout.
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -101,61 +101,101 @@ const DrawerHeader = styled("div")(({ theme }) => ({
marginBottom: "1rem",
}));
+
const SideBarBody = (props: {
- loading: boolean;
- handleOpenModal: any;
- handleOpenAboutModal: any;
+ handleOpenModal: () => void;
+ handleOpenAboutModal: () => void;
}) => {
const location = useLocation();
const routes = useRoutes(AppRoutes as RouteObject[]);
const history = useHistory();
const dispatch = useAppDispatch();
- const { loading: loader, handleOpenModal, handleOpenAboutModal } = props;
+ const { handleOpenModal, handleOpenAboutModal } = props;
const navigate = useNavigate();
- const { loading } = useSelector((state: TypeHeaderState) =>
state.typeHeader);
const { relationshipSearch = {} } = globalSessionData || {};
const [open, setOpen] = useState(true);
const [searchTerm, setSearchTerm] = useState<string>("");
+ const { data: versionData } = useAppSelector((state) =>
state.session?.versionData || {});
+ const searchParams = new URLSearchParams(location.search);
+
+ const isCustomFilterActive = searchParams.get("isCF") === "true";
+ const isGlossaryActive = !isCustomFilterActive &&
(location.pathname.includes("/glossary") || !!searchParams.get("gtype") ||
!!searchParams.get("term") || !!searchParams.get("category"));
+ const isBusinessMetadataActive = !isCustomFilterActive &&
location.pathname.includes("/administrator/businessMetadata");
+ const isClassificationActive = !isCustomFilterActive &&
(!!searchParams.get("tag") || location.pathname.includes("/tag/tagAttribute"));
+ const isRelationshipActive = !isCustomFilterActive &&
(!!searchParams.get("relationshipName") ||
location.pathname.includes("/relationshipDetailPage"));
+
+ const isEntitiesActive = !isCustomFilterActive &&
(!!searchParams.get("type") || location.pathname.includes("/detailPage"));
+
+ const modules = [
Review Comment:
Resolved. Wrapped the modules array in useMemo with its dependent states.
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -308,189 +464,121 @@ const SideBarBody = (props: {
data-cy="atlas-logo"
/>
</span>
- <Paper
- sx={{
- width: "100%",
- }}
- className="sidebar-searchbar"
- >
- <InputBase
- fullWidth
- sx={{ color: "rgba(0, 0, 0, 0.7)" }}
- placeholder="Entities, Classifications, Glossaries"
- inputProps={{ "aria-label": "search" }}
- value={searchTerm}
- onChange={(e: ChangeEvent<HTMLInputElement>) => {
- setSearchTerm(e.target.value);
- }}
- data-cy="searchNode"
- />
-
- <IconButton type="submit" size="small" aria-label="search">
- <SearchIcon fontSize="inherit" />
- </IconButton>
- </Paper>
+ <SidebarSearchInput
+ searchTerm={searchTerm}
+ onChange={setSearchTerm}
+ dataCy="searchNode"
+ />
</Stack>
</DrawerHeader>
)}
<Paper
className="sidebar-wrapper"
sx={{
flex: 1,
- overflow: "hidden auto",
- paddingBottom: "0px", // Account for bottom toggle button
- ...(open == false && {
+ overflowX: "hidden",
+ overflowY: "auto",
+ paddingBottom: "48px", // Added space so it doesn't touch the
bottom toggle button
+ ...(!open && {
overflow: "hidden",
+ display: "none",
}),
}}
>
- <div
- className="sidebar-treeview-container"
- data-cy="r_entityTreeRender"
- >
- <Suspense
- fallback={
- <SkeletonLoader
- animation="pulse"
- variant="text"
- width={330}
- count={5}
- />
- // <Stack className="tree-item-loader-box">
- // </Stack>
- }
- >
- <EntitiesTree
- sideBarOpen={open}
- loading={loading}
- searchTerm={searchTerm}
- />
- </Suspense>
- </div>
+ {open && (
+ <>
+ <div
+ className="sidebar-treeview-container"
+ data-cy="r_entityTreeRender"
+ >
+ <Suspense
+ fallback={<TreeSkeletonLoader count={2} />}
+ >
+ <EntitiesTree
+ sideBarOpen={open}
+ searchTerm={searchTerm}
+ />
+ </Suspense>
+ </div>
- <div
- className="sidebar-treeview-container"
- data-cy="r_classificationTreeRender"
- >
- <Suspense
- fallback={
- <SkeletonLoader
- animation="pulse"
- variant="text"
- width={330}
- count={5}
- />
- // <Stack className="tree-item-loader-box">
- // </Stack>
- }
- >
- <ClassificationTree
- sideBarOpen={open}
- loading={loader}
- searchTerm={searchTerm}
- />
- </Suspense>
- </div>
+ <div
+ className="sidebar-treeview-container"
+ data-cy="r_classificationTreeRender"
+ >
+ <Suspense
+ fallback={<TreeSkeletonLoader count={2} />}
+ >
+ <ClassificationTree
+ sideBarOpen={open}
+ searchTerm={searchTerm}
+ />
+ </Suspense>
+ </div>
- <div
- className="sidebar-treeview-container"
- data-cy="r_businessMetadataTreeRender"
- >
- <Suspense
- fallback={
- <SkeletonLoader
- animation="pulse"
- variant="text"
- width={330}
- count={5}
- />
- // <Stack className="tree-item-loader-box">
- // </Stack>
- }
- >
- <BusinessMetadataTree
- sideBarOpen={open}
- searchTerm={searchTerm}
- />
- </Suspense>
- </div>
+ <div
+ className="sidebar-treeview-container"
+ data-cy="r_glossaryTreeRender"
+ >
+ <Suspense
+ fallback={<TreeSkeletonLoader count={2} />}
+ >
+ <GlossaryTree sideBarOpen={open} searchTerm={searchTerm} />
+ </Suspense>
+ </div>
- <div
- className="sidebar-treeview-container"
- data-cy="r_glossaryTreeRender"
- >
- <Suspense
- fallback={
- <SkeletonLoader
- animation="pulse"
- variant="text"
- width={330}
- count={5}
- />
- // <Stack className="tree-item-loader-box">
- // </Stack>
- }
- >
- <GlossaryTree sideBarOpen={open} searchTerm={searchTerm} />
- </Suspense>
- </div>
- {relationshipSearch && (
- <div
- className="sidebar-treeview-container"
- data-cy="r_relationshipTreeRender"
- >
- <Suspense
- fallback={
- <SkeletonLoader
- animation="pulse"
- variant="text"
- width={330}
- count={5}
+ <div
+ className="sidebar-treeview-container"
+ data-cy="r_businessMetadataTreeRender"
+ >
+ <Suspense
+ fallback={<TreeSkeletonLoader count={2} />}
+ >
+ <BusinessMetadataTree
+ sideBarOpen={open}
+ searchTerm={searchTerm}
/>
- // <Stack className="tree-item-loader-box">
- // </Stack>
- }
+ </Suspense>
+ </div>
+ {relationshipSearch && (
+ <div
+ className="sidebar-treeview-container"
+ data-cy="r_relationshipTreeRender"
+ >
+ <Suspense
+ fallback={<TreeSkeletonLoader count={2} />}
+ >
+ <RelationshipsTree
+ sideBarOpen={open}
+ searchTerm={searchTerm}
+ />
+ </Suspense>
+ </div>
+ )}
+
+ <div
+ className="sidebar-treeview-container"
+ data-cy="r_customFilterTreeRender"
>
- <RelationshipsTree
- sideBarOpen={open}
- searchTerm={searchTerm}
- />
- </Suspense>
- </div>
+ <Suspense
+ fallback={<TreeSkeletonLoader count={2} />}
+ >
+ <CustomFiltersTree sideBarOpen={open}
searchTerm={searchTerm} />
+ </Suspense>
+ </div>
+ </>
)}
-
- <div
- className="sidebar-treeview-container"
- data-cy="r_customFilterTreeRender"
- >
- <Suspense
- fallback={
- <SkeletonLoader
- animation="pulse"
- variant="text"
- width={330}
- count={5}
- />
- // <Stack className="tree-item-loader-box">
- // </Stack>
- }
- >
- <CustomFiltersTree sideBarOpen={open} searchTerm={searchTerm}
/>
- </Suspense>
- </div>
</Paper>
<div
- style={{
- width: "100%",
- textAlign: "right",
- padding: "8px",
- position: "sticky",
- bottom: "0px",
- zIndex: "9",
- left: "0",
- background: "#034858",
- }}
+ className={`sidebar-toggle-container ${open ?
'sidebar-toggle-open' : 'sidebar-toggle-closed'}`}
>
+ {open && (
+ <Box display="flex" flexDirection="column" gap="4px"
alignItems="flex-start" pl="4px">
+ <Typography variant="body2" sx={{ color: "rgba(255, 255, 255,
0.6)", pl: '4px' }}>
Review Comment:
Resolved. Added loading and error fallbacks for versionData fetching,
displaying a small spinner while loading and "Version unavailable" on error.
--
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]
