pawarprasad123 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3781808034
##########
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 = [
+ { id: "entities", title: "Entities", isActive: isEntitiesActive, iconUrl:
"/img/sidebar-icons/icon-entities.svg", Component: EntitiesTree, isVisible:
true },
+ { id: "classification", title: "Classifications", isActive:
isClassificationActive, iconUrl: "/img/sidebar-icons/icon-classifications.svg",
Component: ClassificationTree, isVisible: true },
+ { id: "glossary", title: "Glossary", isActive: isGlossaryActive, iconUrl:
"/img/sidebar-icons/icon-glossary.svg", Component: GlossaryTree, isVisible:
true },
+ { id: "businessMetadata", title: "Business Metadata", isActive:
isBusinessMetadataActive, iconUrl:
"/img/sidebar-icons/icon-business-metadata.svg", Component:
BusinessMetadataTree, isVisible: true },
+ { id: "relationships", title: "Relationships", isActive:
isRelationshipActive, iconUrl: "/img/sidebar-icons/icon-relationships.svg",
Component: RelationshipsTree, isVisible: !!relationshipSearch },
+ { id: "customFilters", title: "Custom Filters", isActive:
isCustomFilterActive, iconUrl: "/img/sidebar-icons/icon-custom-filters.svg",
Component: CustomFiltersTree, isVisible: true }
+ ];
const handleDrawerOpen = () => {
setOpen(!open);
};
- const [position, setPosition] = useState<string |
number>(defaultDrawerWidth);
- const draggerRef = useRef<HTMLDivElement>(null);
- const headerRef = useRef<HTMLDivElement>(null);
- const windowWidth = window.innerWidth;
- const minPosition = 300;
- const maxPosition = windowWidth * 0.6;
-
- const handleMouseMove = (e: MouseEvent) => {
- let newPosition = e.clientX;
+ const [popoverAnchor, setPopoverAnchor] = useState<HTMLButtonElement |
null>(null);
+ const [activePopover, setActivePopover] = useState<string | null>(null);
+ const [popoverMaxHeight, setPopoverMaxHeight] = useState<string>('calc(100vh
- 100px)');
+ const [isBottomHalf, setIsBottomHalf] = useState<boolean>(false);
+
+ const handlePopoverOpen = (event: React.MouseEvent<HTMLButtonElement>, id:
string) => {
+ const target = event.currentTarget;
+
+ const openNewPopover = () => {
+ setPopoverAnchor(target);
+ setActivePopover(id);
+
+ // Calculate remaining screen height from the anchor to the bottom
+ const rect = target.getBoundingClientRect();
+ const spaceBelow = window.innerHeight - rect.top - 24;
+ const isBottom = spaceBelow < 350;
+ setIsBottomHalf(isBottom);
+
+ if (isBottom) {
+ const spaceAbove = rect.bottom - 24;
+ setPopoverMaxHeight(`${Math.max(250, spaceAbove)}px`);
+ } else {
+ setPopoverMaxHeight(`${Math.max(250, spaceBelow)}px`);
+ }
+ };
- if (newPosition < minPosition) {
- newPosition = minPosition;
- } else if (newPosition > maxPosition) {
- newPosition = maxPosition;
+ // If a different popover is already open, close it first to ensure clean
unmount
+ if (activePopover && activePopover !== id) {
+ handlePopoverClose();
+ setTimeout(openNewPopover, 0);
Review Comment:
line -168-171:
setTimeout(openNewPopover, 0) when switching popovers has no cleanup. Rapid
clicks can leave wrong popover open. Prefer synchronous state update or
store/clear timeout in a ref with cleanup on unmount.
##########
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";
Review Comment:
Multiple modules can appear active simultaneously
line 121-127:
Active checks are independent. A URL with both type and tag can highlight
Entities and Classifications. Consider mutual exclusion or priority (e.g. isCF
already takes precedence).
##########
dashboard/src/components/SidebarSearchInput.tsx:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import React, { ChangeEvent } from "react";
+import { Paper, InputBase, Stack } from "@mui/material";
+import ClearIcon from "@mui/icons-material/Clear";
+import { IconButton } from "@components/muiComponents";
+
+interface SidebarSearchInputProps {
+ searchTerm: string;
+ onChange: (value: string) => void;
+ dataCy?: string;
+}
+
+export const SidebarSearchInput: React.FC<SidebarSearchInputProps> = ({
+ searchTerm,
+ onChange,
+ dataCy
+}) => (
+ <Paper
+ sx={{
+ width: "100%",
+ paddingLeft: "8px",
+ display: "flex",
+ alignItems: "center"
+ }}
+ className="sidebar-searchbar"
+ >
+ <InputBase
+ fullWidth
+ sx={{ color: "rgba(0, 0, 0, 0.7)" }}
+ placeholder="Search"
+ inputProps={{ "aria-label": "search" }}
+ value={searchTerm}
+ onChange={(e: ChangeEvent<HTMLInputElement>) => onChange(e.target.value)}
+ data-cy={dataCy}
+ endAdornment={
+ <Stack direction="row" alignItems="center" gap="4px">
+ {searchTerm.length > 0 && (
+ <IconButton
+ size="small"
+ onClick={() => onChange("")}
+ edge="end"
+ sx={{ padding: "4px" }}
+ >
+ <ClearIcon fontSize="small" sx={{ color: "rgba(0, 0, 0, 0.4)" }}
/>
+ </IconButton>
+ )}
+ <img
+ src="/img/sidebar-icons/icon-search.svg"
+ style={{
Review Comment:
Inline style on search icon — consider moving to SCSS under
.sidebar-searchbar.
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -246,38 +323,117 @@ const SideBarBody = (props: {
backgroundColor: "#034858",
}}
>
- {/* Collapsed sidebar logo */}
+ {/* Collapsed sidebar logo and module icons */}
{!open && (
- <div
- style={{
- width: "100%",
- textAlign: "center",
- paddingLeft: "12px",
- display: "flex",
- alignItems: "center",
- justifyContent: "center",
- minHeight: "64px",
- cursor: "pointer",
- boxSizing: "border-box",
- }}
- role="button"
- tabIndex={0}
- aria-label="Atlas home — refresh dashboard"
- onClick={handleAtlasLogoClick}
- onKeyDown={handleAtlasLogoKeyDown}
- data-cy="apache-atlas-logo-collapsed"
+ <Stack
+ alignItems="center"
+ sx={{ width: "100%", flex: 1, minHeight: 0, overflowY: "auto",
overflowX: "hidden", boxSizing: "border-box", pb: "60px" }}
>
- <img
- src={apacheAtlasLogo}
- alt="Apache Atlas logo"
- style={{
- width: "29px",
- height: "auto",
- maxWidth: "100%",
- display: "block",
+ <div
+ className="collapsed-logo-container"
+ role="button"
+ tabIndex={0}
+ aria-label="Atlas home — refresh dashboard"
+ onClick={handleAtlasLogoClick}
+ onKeyDown={handleAtlasLogoKeyDown}
+ data-cy="apache-atlas-logo-collapsed"
+ >
+ <img
+ src={apacheAtlasLogo}
+ alt="Apache Atlas logo"
+ className="collapsed-logo-img"
+ />
+ </div>
+
+ {/* Module Icons for Mini Drawer */}
+ <Stack alignItems="stretch" gap="1rem" sx={{ width: "100%" }}>
+ {/* Search */}
+ <Box sx={{ display: "flex", justifyContent: "center",
borderLeft: "4px solid transparent", borderRight: "4px solid transparent",
background: "transparent" }}>
+ <Tooltip title="Search" placement="right">
+ <IconButton aria-expanded={open} onClick={() =>
setOpen(true)} sx={{ '&:hover': { background: 'rgba(255, 255, 255, 0.1)' } }}>
+ <img src="/img/sidebar-icons/icon-search.svg"
className="sidebar-module-icon" alt="search" />
+ </IconButton>
+ </Tooltip>
+ </Box>
+
+ {modules.filter(m => m.isVisible).map(m => (
+ <Box
+ key={m.id}
+ className={m.isActive ? "sidebar-icon-active" : ""}
+ sx={{
+ display: "flex",
+ justifyContent: "center",
+ borderLeft: "4px solid transparent",
+ borderRight: "4px solid transparent",
+ background: "transparent"
+ }}
+ >
+ <Tooltip title={m.title} placement="right">
+ <IconButton onClick={(e) => handlePopoverOpen(e, m.id)}
sx={{ color: m.isActive ? "white" : "rgba(255, 255, 255, 0.6)", '&:hover': {
color: 'white', background: 'rgba(255, 255, 255, 0.1)' } }}>
Review Comment:
Accessibility — module icon buttons
line 372–374
Module IconButtons rely on Tooltip only. Add aria-label={m.title} and
aria-expanded={activePopover === m.id} for screen readers and keyboard users.
line 353
Search expand button uses aria-expanded={open} — open is drawer state, not
popover. Misleading for assistive tech; use aria-label="Expand sidebar search".
line54-60
Clear button has no aria-label. Add aria-label="Clear search".
##########
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>) => {
Review Comment:
handleError correctly sets target.onerror = null before fallback. Add test
when fallback also fails (no infinite loop).
##########
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:
low:
Version shows V ${versionData.Version} with no loading/error fallback if
fetchVersionData fails.
##########
dashboard/src/views/DashboardOverview/DashboardOverview.tsx:
##########
@@ -97,11 +97,12 @@ const DashboardOverview = () => {
maxWidth: "100%",
boxSizing: "border-box",
backgroundColor: "#f5f7f9",
- padding: 3,
- borderRadius: 2
+ borderRadius: 2,
+ pb: 3,
+ pr: 3
}}
>
- <Grid container spacing={3} sx={{ width: "100%",
alignItems: "stretch" }}>
+ <Grid container spacing={3} sx={{ m: 0, width: "100%",
alignItems: "stretch" }}>
Review Comment:
Padding/grid margin changes — confirm intentional for this ticket.
verify this.
##########
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 = [
+ { id: "entities", title: "Entities", isActive: isEntitiesActive, iconUrl:
"/img/sidebar-icons/icon-entities.svg", Component: EntitiesTree, isVisible:
true },
+ { id: "classification", title: "Classifications", isActive:
isClassificationActive, iconUrl: "/img/sidebar-icons/icon-classifications.svg",
Component: ClassificationTree, isVisible: true },
+ { id: "glossary", title: "Glossary", isActive: isGlossaryActive, iconUrl:
"/img/sidebar-icons/icon-glossary.svg", Component: GlossaryTree, isVisible:
true },
+ { id: "businessMetadata", title: "Business Metadata", isActive:
isBusinessMetadataActive, iconUrl:
"/img/sidebar-icons/icon-business-metadata.svg", Component:
BusinessMetadataTree, isVisible: true },
+ { id: "relationships", title: "Relationships", isActive:
isRelationshipActive, iconUrl: "/img/sidebar-icons/icon-relationships.svg",
Component: RelationshipsTree, isVisible: !!relationshipSearch },
+ { id: "customFilters", title: "Custom Filters", isActive:
isCustomFilterActive, iconUrl: "/img/sidebar-icons/icon-custom-filters.svg",
Component: CustomFiltersTree, isVisible: true }
+ ];
const handleDrawerOpen = () => {
Review Comment:
Popover state not cleared when drawer expands
line 138–140
handleDrawerOpen() toggles open but does not reset activePopover /
popoverAnchor. If a popover was open, collapsing again can reopen it with a
stale anchor. Call handlePopoverClose() when setOpen(true).
line 353:
Search icon onClick={() => setOpen(true)} has the same issue — expand drawer
should also close any active popover.
##########
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:
mising negative test
When avatarDisplay === undefined, width/height props are ignored on <img>.
Confirm intentional (may affect table layout).
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -246,38 +323,117 @@ const SideBarBody = (props: {
backgroundColor: "#034858",
}}
>
- {/* Collapsed sidebar logo */}
+ {/* Collapsed sidebar logo and module icons */}
{!open && (
- <div
- style={{
- width: "100%",
- textAlign: "center",
- paddingLeft: "12px",
- display: "flex",
- alignItems: "center",
- justifyContent: "center",
- minHeight: "64px",
- cursor: "pointer",
- boxSizing: "border-box",
- }}
- role="button"
- tabIndex={0}
- aria-label="Atlas home — refresh dashboard"
- onClick={handleAtlasLogoClick}
- onKeyDown={handleAtlasLogoKeyDown}
- data-cy="apache-atlas-logo-collapsed"
+ <Stack
+ alignItems="center"
+ sx={{ width: "100%", flex: 1, minHeight: 0, overflowY: "auto",
overflowX: "hidden", boxSizing: "border-box", pb: "60px" }}
>
- <img
- src={apacheAtlasLogo}
- alt="Apache Atlas logo"
- style={{
- width: "29px",
- height: "auto",
- maxWidth: "100%",
- display: "block",
+ <div
+ className="collapsed-logo-container"
+ role="button"
+ tabIndex={0}
+ aria-label="Atlas home — refresh dashboard"
+ onClick={handleAtlasLogoClick}
+ onKeyDown={handleAtlasLogoKeyDown}
+ data-cy="apache-atlas-logo-collapsed"
+ >
+ <img
+ src={apacheAtlasLogo}
+ alt="Apache Atlas logo"
+ className="collapsed-logo-img"
+ />
+ </div>
+
+ {/* Module Icons for Mini Drawer */}
+ <Stack alignItems="stretch" gap="1rem" sx={{ width: "100%" }}>
+ {/* Search */}
+ <Box sx={{ display: "flex", justifyContent: "center",
borderLeft: "4px solid transparent", borderRight: "4px solid transparent",
background: "transparent" }}>
+ <Tooltip title="Search" placement="right">
+ <IconButton aria-expanded={open} onClick={() =>
setOpen(true)} sx={{ '&:hover': { background: 'rgba(255, 255, 255, 0.1)' } }}>
+ <img src="/img/sidebar-icons/icon-search.svg"
className="sidebar-module-icon" alt="search" />
+ </IconButton>
+ </Tooltip>
+ </Box>
+
+ {modules.filter(m => m.isVisible).map(m => (
+ <Box
+ key={m.id}
+ className={m.isActive ? "sidebar-icon-active" : ""}
+ sx={{
+ display: "flex",
+ justifyContent: "center",
+ borderLeft: "4px solid transparent",
+ borderRight: "4px solid transparent",
+ background: "transparent"
+ }}
+ >
+ <Tooltip title={m.title} placement="right">
+ <IconButton onClick={(e) => handlePopoverOpen(e, m.id)}
sx={{ color: m.isActive ? "white" : "rgba(255, 255, 255, 0.6)", '&:hover': {
color: 'white', background: 'rgba(255, 255, 255, 0.1)' } }}>
+ <img src={m.iconUrl} className="sidebar-module-icon"
alt={m.title.toLowerCase()} />
+ </IconButton>
+ </Tooltip>
+ </Box>
+ ))}
+ </Stack>
Review Comment:
Inline { color: "#D3D3D3", fontWeight: "600" } duplicates
.sidebar-tree-highlight in sidebar.scss:329-332. Use the CSS class for
consistency.
##########
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:
low:
modules array recreated every render. Optional useMemo on [location,
relationshipSearch].
--
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]
