pawarprasad123 commented on code in PR #688:
URL: https://github.com/apache/atlas/pull/688#discussion_r3843262631
##########
dashboard/src/views/SideBar/SideBarTree/__tests__/SideBarTree.test.tsx:
##########
@@ -27,6 +27,7 @@
*/
import React from 'react'
+import '@testing-library/jest-dom'
import { render, screen, waitFor, fireEvent, act, cleanup } from
'@testing-library/react'
Review Comment:
No test for keyboard Escape closing popover (only backdrop click)
No test for version footer in collapsed mode (version only shown when open
=== true, line 583)
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -210,30 +300,32 @@ const SideBarBody = (props: {
<Drawer
sx={{
- width: position,
+ width: open ? defaultDrawerWidth : "60px",
Review Comment:
Still using inline sx heavily
Drawer config (~lines 302–326)
Popover PaperProps.sx (~lines 408–436)
Module icon hover states (~lines 366–387)
Version footer (~lines 585–587)
##########
dashboard/src/views/SideBar/SideBarBody.tsx:
##########
@@ -101,61 +101,116 @@ 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 { relationshipSearch = false } = globalSessionData || {};
const [open, setOpen] = useState(true);
const [searchTerm, setSearchTerm] = useState<string>("");
+ const { data: versionData, loading: isVersionLoading, error: versionError }
= useAppSelector((state) => state.session?.versionData || {});
+ const searchParams = new URLSearchParams(location.search);
+
+ const activeModule = useMemo(() => {
+ if (searchParams.get("isCF") === "true") return "customFilters";
+ if (location.pathname.includes("/glossary") || !!searchParams.get("gtype")
|| !!searchParams.get("term") || !!searchParams.get("category")) return
"glossary";
+ if (location.pathname.includes("/administrator/businessMetadata")) return
"businessMetadata";
+ if (!!searchParams.get("tag") ||
location.pathname.includes("/tag/tagAttribute")) return "classification";
+ if (!!searchParams.get("relationshipName") ||
location.pathname.includes("/relationshipDetailPage")) return "relationships";
+ if (!!searchParams.get("type") ||
location.pathname.includes("/detailPage")) return "entities";
+ return null;
+ }, [location.pathname, location.search]);
Review Comment:
Fix eslint warning: add searchParams to useMemo deps, or derive from
location.search inside the memo.
##########
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, error } = useAppSelector((state:
any) => state.session.versionData);
Review Comment:
Replace (state: any) with typed RootState selector for consistency with
strict typing elsewhere in this PR.
##########
dashboard/src/views/SideBar/SideBarTree/SideBarTree.tsx:
##########
@@ -287,1027 +322,1070 @@ const BarTreeView: FC<{
sideBarOpen,
searchTerm,
loader,
+ isPopover,
}) => {
- const dispatch = useAppDispatch();
- const { savedSearchData }: any = useAppSelector(
- (state: any) => state.savedSearch
- );
- const { bmguid } = useParams();
- const location = useLocation();
- const navigate = useNavigate();
- const searchParams = new URLSearchParams(location.search);
- const [expand, setExpand] = useState<null | HTMLElement>(null);
- const [selectedNode, setSelectedNode] = useState<{
- type: string | null;
- tag: string | null;
- relationship: string | null;
- businessMetadata: string | null;
- }>({
- type: null,
- tag: null,
- relationship: null,
- businessMetadata: null,
- });
-
- const [openModal, setOpenModal] = useState<boolean>(false);
- const toastId: any = useRef(null);
- const open = Boolean(expand);
- const [expandedItems, setExpandedItems] = useState<string[]>([]);
- const [tagModal, setTagModal] = useState<boolean>(false);
- const [glossaryModal, setGlossaryModal] = useState<boolean>(false);
- const { businessMetaData }: any = useAppSelector(
- (state: any) => state.businessMetaData
- );
-
- const filteredData = useMemo(() => {
- return treeData.filter((node) => {
- return (
- node.label?.toLowerCase().includes(searchTerm.toLowerCase()) ||
- (node.children &&
- node.children.some((child) =>
- child.label?.toLowerCase().includes(searchTerm.toLowerCase())
- ))
- );
+ const { savedSearchData }: any = useAppSelector(
+ (state: any) => state.savedSearch
+ );
+ const { bmguid } = useParams();
+ const dispatch = useAppDispatch();
+ const location = useLocation();
+ const navigate = useNavigate();
+ const searchParams = new URLSearchParams(location.search);
+ const [expand, setExpand] = useState<null | HTMLElement>(null);
+ const [selectedNode, setSelectedNode] = useState<SelectedNode>({
+ type: null,
+ tag: null,
+ relationship: null,
+ businessMetadata: null,
+ term: null,
+ customFilter: null,
});
- }, [treeData, searchTerm]);
- const displayTreeName = useMemo(() => {
- return treeName === "CustomFilters" ? "Custom Filters" : treeName
- }, [treeName]);
+ const [openModal, setOpenModal] = useState<boolean>(false);
+ const toastId: any = useRef(null);
+ const open = Boolean(expand);
+ const [expandedItems, setExpandedItems] = useState<string[]>([]);
+ const [tagModal, setTagModal] = useState<boolean>(false);
+ const [glossaryModal, setGlossaryModal] = useState<boolean>(false);
+ const { businessMetaData }: any = useAppSelector(
+ (state: any) => state.businessMetaData
+ );
- const highlightText = useMemo(() => {
- return (text: string) => {
- if (!searchTerm) return text;
+ const filteredData = useMemo(() => {
+ if (!searchTerm) return treeData;
+ const lowerSearch = searchTerm.toLowerCase();
+ return treeData.reduce((acc: any[], node: any) => {
+ const nodeMatches = node.label?.toLowerCase().includes(lowerSearch);
+ let filteredChildren = node.children;
+ if (!nodeMatches && node.children) {
+ filteredChildren = node.children.filter((child: any) =>
+ child.label?.toLowerCase().includes(lowerSearch)
+ );
+ }
+ if (nodeMatches || (filteredChildren && filteredChildren.length > 0)) {
+ acc.push({ ...node, children: filteredChildren });
+ }
+ return acc;
+ }, []);
+ }, [treeData, searchTerm]);
- const parts = text.split(new RegExp(`(${searchTerm})`, "gi"));
- return parts.map((part, index) =>
- part.toLowerCase() === searchTerm.toLowerCase() ? (
- <span key={index} style={{ color: "#D3D3D3", fontWeight: "600" }}>
- {part}
- </span>
- ) : (
- part
- )
- );
- };
- }, [searchTerm]);
-
- const expandedItemsMemo = useMemo(() => {
- const allNodeIds = filteredData.flatMap((node) => {
- return [
- node.id,
- ...(node.children ? node.children.map((child) => child.id) : []),
- ];
- });
- return [...allNodeIds, ...[treeName]];
- }, [filteredData, treeName]);
+ const displayTreeName = useMemo(() => {
+ return treeName === "CustomFilters" ? "Custom Filters" : treeName
+ }, [treeName]);
- useEffect(() => {
- setExpandedItems(expandedItemsMemo);
- }, [expandedItemsMemo]);
+ const highlightText = useMemo(() => {
+ return (text: string) => {
+ if (!searchTerm) return text;
- useEffect(() => {
- const searchParams = new URLSearchParams(location.search);
- const nodeIdFromParamsType = searchParams.get("type");
- const nodeIdFromParamsTag = searchParams.get("tag");
- const nodeIdFromParamsRelationshipName =
- searchParams.get("relationshipName");
- const nodeIdFromBMName = location.pathname.includes(
- "/administrator/businessMetadata"
- );
+ const escapeRegExp = (string: string) => {
+ return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means
the whole matched string
+ };
- const bmObj = !isEmpty(businessMetaData?.businessMetadataDefs)
- ? businessMetaData?.businessMetadataDefs?.find((obj: EnumTypeDefData) =>
{
+ const escapedSearchTerm = escapeRegExp(searchTerm);
+ const parts = text.split(new RegExp(`(${escapedSearchTerm})`, "gi"));
+ return parts.map((part, index) =>
+ part.toLowerCase() === searchTerm.toLowerCase() ? (
+ <span key={index} className="sidebar-tree-highlight">
+ {part}
+ </span>
+ ) : (
+ part
+ )
+ );
+ };
+ }, [searchTerm]);
+
+ const expandedItemsMemo = useMemo(() => {
+ if (!searchTerm) {
+ return [treeName];
+ }
+ const parentNodeIds = filteredData.map((node) => node.id);
+ return [...parentNodeIds, treeName];
+ }, [filteredData, treeName, searchTerm]);
+
+ useEffect(() => {
+ setExpandedItems(expandedItemsMemo);
+ }, [expandedItemsMemo]);
+
+ const getNodeId = (node: TreeNode) => {
+ if (treeName == "Classifications" && node.types == "parent") {
+ return node.label;
+ } else if (treeName == "Classifications" && node.types == "child") {
+ return `${node.id}@${node.label}`;
+ }
+ return !isEmpty(node?.parent) ? `${node.id}@${node?.parent}` : node.id;
+ };
+
+ useEffect(() => {
+ const searchParams = new URLSearchParams(location.search);
+ const nodeIdFromParamsType = searchParams.get("type");
+ const nodeIdFromParamsTag = searchParams.get("tag");
+ const nodeIdFromParamsRelationshipName =
+ searchParams.get("relationshipName");
+ const nodeIdFromBMName = location.pathname.includes(
+ "/administrator/businessMetadata"
+ );
+ const nodeIdFromParamsTerm = searchParams.get("term") ||
searchParams.get("category") || searchParams.get("gtype") ||
location.pathname.split("/glossary/")[1];
+ const nodeIdFromCustomFilter = searchParams.get("customFilter");
+
+ const bmObj = !isEmpty(businessMetaData?.businessMetadataDefs)
+ ? businessMetaData?.businessMetadataDefs?.find((obj: EnumTypeDefData)
=> {
if (bmguid == obj.guid) {
return obj;
}
})
- : {};
- const { name = "" } = bmObj || {};
-
- setSelectedNode({
- type: nodeIdFromParamsType,
- tag: nodeIdFromParamsTag,
- relationship: nodeIdFromParamsRelationshipName,
- businessMetadata: nodeIdFromBMName ? name : null,
- });
+ : {};
+ const { name = "" } = bmObj || {};
- if (
- !nodeIdFromParamsType &&
- !nodeIdFromParamsTag &&
- !nodeIdFromParamsRelationshipName &&
- !nodeIdFromBMName
- ) {
setSelectedNode({
- type: null,
- tag: null,
- relationship: null,
- businessMetadata: null,
+ type: nodeIdFromParamsType,
+ tag: nodeIdFromParamsTag,
+ relationship: nodeIdFromParamsRelationshipName,
+ businessMetadata: nodeIdFromBMName ? name : null,
+ term: nodeIdFromParamsTerm || null,
+ customFilter: nodeIdFromCustomFilter || null,
});
- }
- }, [location.search]);
- const getEmptyTypesTitle = () => {
- switch (treeName) {
- case "Entities":
- return `${isEmptyServicetype ? "Hide" : "Show"} empty service types`;
+ if (nodeIdFromParamsTerm && typeof nodeIdFromParamsTerm === "string" &&
nodeIdFromParamsTerm.includes("@") && treeName === "Glossary") {
+ const glossaryName = nodeIdFromParamsTerm.split("@")[1];
+ if (glossaryName) {
+ const parentNode = treeData.find((n) => n.label === glossaryName);
+ if (parentNode) {
+ const nodeIdToExpand = getNodeId(parentNode);
+ setExpandedItems((prev) => {
+ if (!prev.includes(nodeIdToExpand)) {
+ return [...prev, nodeIdToExpand];
+ }
+ return prev;
+ });
+ }
+ }
+ }
- case "Classifications":
- return `${isEmptyServicetype ? "Show" : "Hide"} unused
classifications`;
+ if (
+ !nodeIdFromParamsType &&
+ !nodeIdFromParamsTag &&
+ !nodeIdFromParamsRelationshipName &&
+ !nodeIdFromBMName &&
+ !nodeIdFromParamsTerm &&
+ !nodeIdFromCustomFilter
+ ) {
+ setSelectedNode({
+ type: null,
+ tag: null,
+ relationship: null,
+ businessMetadata: null,
+ term: null,
+ customFilter: null,
+ });
+ }
+ }, [location.search, treeData, treeName, businessMetaData, bmguid]);
Review Comment:
useEffect missing deps (getNodeId, location.pathname) — may cause stale
selection state edge cases.
--
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]
